Rewrite stale charset declarations to UTF-8 - #81
Conversation
|
@tamnd This charset split is ready for review. A final correctness pass now ensures declarations inside inert |
Co-authored-by: SihanTeng <SihanTeng@users.noreply.github.com>
f62500c to
5ca4167
Compare
|
Rewriting stale declarations instead of only inserting a missing one is the right fix for #16, and keeping CharsetRewritten as a separate field rather than renaming CharsetAdded was a good call. I squashed onto main to clear the changelog conflicts. I am merging this and sending a follow up straight after for one thing I found while probing it: fixCharsetMetas sets declared from a meta anywhere in the tree, so a page whose only charset meta sits in body comes out with nothing in head, which puts the declaration past the 1024 byte prescan window and mojibakes exactly the pages #16 is about. The rewrite half should stay tree wide, only the declared question needs to go back to head. |
fixCharsetMetas set declared from a <meta charset> anywhere in the tree, so a page whose only charset meta sits in <body>, which is the malformed markup case the walk exists to handle, came out of sanitize with no declaration in <head> at all. Readers pre-scan the first 1024 bytes for the encoding, so the declaration was never found and every multibyte character mojibaked, which is the failure issue #16 reports. Rewriting stale values stays whole-document. Only the declared question moves back to <head>, so a stray body meta is rewritten to utf-8 and <head> still gets its own declaration first. rewriteContentTypeCharset also starts at the first field rather than the second, so content="charset=iso-8859-1" with no media type is rewritten instead of being left to contradict the injected declaration. A real media type has no equals sign, so the existing Cut rejects it. Follow-up to #81.
2 participants
Summary
<meta charset>and legacy Content-Type declarations to match the UTF-8 bytes kage writes.<head>.<template>content without treating them as the document declaration.Report.CharsetAddedmeaning and addCharsetRewritteninstead of renaming the field.Why
This is the charset contribution split out of #74 as requested in review. It builds on the doctype fix already landed in #78 and keeps the public report API backward compatible.
Test plan
go test -count=1 ./sanitize/Related: #16