diff --git a/CHANGELOG.md b/CHANGELOG.md index 281306c..cee3d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ All notable changes to kage are recorded here. The format follows - Packing a mirror with multiple HTML pages but no root `index.html` creates a bounded, title-sorted landing page, while a single-page archive still opens directly on its article and keeps that article's title metadata ([#62](https://github.com/tamnd/kage/issues/62)). +- Non-UTF-8 `` and Content-Type charset declarations are + rewritten to `utf-8`, matching the encoding kage writes to disk ([#16](https://github.com/tamnd/kage/issues/16)). - `--resume` picks an interrupted crawl back up instead of doing nothing ([#36](https://github.com/tamnd/kage/issues/36)). `state.json` persisted only the visited set, and the frontier was rebuilt purely by re-rendering pages and following their links, which resume exists to avoid. So a resumed run found its seed already visited, `enqueuePage` turned it down, nothing was queued, and the run printed `pages 0` and exited successfully with most of the site still missing. diff --git a/docs/content/reference/release-notes.md b/docs/content/reference/release-notes.md index 0f7c122..f408934 100644 --- a/docs/content/reference/release-notes.md +++ b/docs/content/reference/release-notes.md @@ -16,6 +16,8 @@ The authoritative, commit-level history lives in [`CHANGELOG.md`](https://github - **Multi-page ZIM packs get a usable landing page.** Mirrors without a root `index.html` list pages by title instead of opening an arbitrary first page; single-page archives still open directly on their article ([#62](https://github.com/tamnd/kage/issues/62)). +- **Saved pages declare their real encoding.** Non-UTF-8 charset metadata is + rewritten to UTF-8, matching the bytes kage writes to disk ([#16](https://github.com/tamnd/kage/issues/16)). ## v0.3.11 diff --git a/sanitize/sanitize.go b/sanitize/sanitize.go index 2b1f1bc..de6adb7 100644 --- a/sanitize/sanitize.go +++ b/sanitize/sanitize.go @@ -50,6 +50,7 @@ type Report struct { DeadLinksRemoved int CondCommentsRemoved int CharsetAdded bool + CharsetRewritten bool } // jsURLAttrs are attributes whose value may be a javascript: URL. @@ -79,7 +80,7 @@ func Strip(doc []byte, opts Options) ([]byte, Report, error) { func CleanTree(root *html.Node, opts Options) Report { var rep Report clean(root, opts, &rep) - rep.CharsetAdded = ensureCharset(root) + rep.CharsetAdded, rep.CharsetRewritten = ensureCharset(root) if opts.MobileReadable { ensureViewport(root) injectMobileCSS(root) @@ -241,22 +242,20 @@ func unwrapNoscript(parent, ns *html.Node) { parent.RemoveChild(ns) } -// ensureCharset guarantees the document declares UTF-8, inserting a -// at the top of when none is present, and reports -// whether it added one. kage renders every saved page as UTF-8, but a source -// that set its charset only in the HTTP Content-Type header, with no -// charset in the markup, loses that signal once the page is a standalone file. -// A reader then serving the bytes without a charset falls back to its locale -// encoding and mojibakes every multibyte character (curly quotes, dashes, a -// non-breaking space). Declaring the charset in the markup makes the page -// self-describing in any reader, kage's own viewer and Kiwix alike. -func ensureCharset(root *html.Node) bool { +// ensureCharset guarantees the document declares UTF-8. kage serialises every +// saved page as UTF-8, so a stale source declaration must be rewritten or it +// can make a standalone reader mojibake the output (issue #16). Missing +// declarations are inserted at the start of . The return values report +// insertion and rewriting separately so the exported Report keeps the existing +// meaning of CharsetAdded. +func ensureCharset(root *html.Node) (added, rewritten bool) { head := findElement(root, atom.Head) if head == nil { - return false + return false, false } - if hasCharsetMeta(head) { - return false + fix := fixCharsetMetas(root) + if fix.declared { + return false, fix.rewritten } meta := &html.Node{ Type: html.ElementNode, @@ -267,26 +266,63 @@ func ensureCharset(root *html.Node) bool { // The declaration must precede any content for a reader to honour it, so it // goes first in . head.InsertBefore(meta, head.FirstChild) - return true + return true, fix.rewritten } -// hasCharsetMeta reports whether head already declares a character encoding, -// either as or the older . -func hasCharsetMeta(head *html.Node) bool { - for c := head.FirstChild; c != nil; c = c.NextSibling { - if c.Type != html.ElementNode || c.DataAtom != atom.Meta { - continue +type charsetMetaFix struct { + declared bool + rewritten bool +} + +// fixCharsetMetas finds charset declarations anywhere in the parsed document +// and rewrites non-UTF-8 values. Searching the whole tree also handles malformed +// input whose meta node Chrome serialised outside without adding a +// second, contradictory declaration. Declarations inside template content are +// rewritten but do not count as declarations for the containing document. +func fixCharsetMetas(n *html.Node) charsetMetaFix { + var fix charsetMetaFix + if n.Type == html.ElementNode && n.DataAtom == atom.Meta { + if charset := strings.TrimSpace(attr(n, "charset")); charset != "" { + fix.declared = true + if !strings.EqualFold(charset, "utf-8") { + setAttr(n, "charset", "utf-8") + fix.rewritten = true + } + } else if strings.EqualFold(attr(n, "http-equiv"), "content-type") { + content, contentFix := rewriteContentTypeCharset(attr(n, "content")) + fix = contentFix + if contentFix.rewritten { + setAttr(n, "content", content) + } } - if attr(c, "charset") != "" { - return true + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + childFix := fixCharsetMetas(c) + fix.rewritten = fix.rewritten || childFix.rewritten + if n.Type != html.ElementNode || n.DataAtom != atom.Template { + fix.declared = fix.declared || childFix.declared } - if strings.EqualFold(attr(c, "http-equiv"), "content-type") && - strings.Contains(strings.ToLower(attr(c, "content")), "charset=") { - return true + } + return fix +} + +// rewriteContentTypeCharset rewrites a charset parameter while preserving the +// media type and other parameters. It accepts optional whitespace around '='. +func rewriteContentTypeCharset(content string) (string, charsetMetaFix) { + var fix charsetMetaFix + parts := strings.Split(content, ";") + for i := 1; i < len(parts); i++ { + key, value, ok := strings.Cut(parts[i], "=") + if !ok || !strings.EqualFold(strings.TrimSpace(key), "charset") { + continue + } + fix.declared = true + if !strings.EqualFold(strings.Trim(strings.TrimSpace(value), `"'`), "utf-8") { + parts[i] = " charset=utf-8" + fix.rewritten = true } } - return false + return strings.Join(parts, ";"), fix } // findElement returns the first element node of the given atom in document @@ -407,3 +443,13 @@ func attr(n *html.Node, key string) string { } return "" } + +func setAttr(n *html.Node, key, value string) { + for i := range n.Attr { + if strings.EqualFold(n.Attr[i].Key, key) { + n.Attr[i].Val = value + return + } + } + n.Attr = append(n.Attr, html.Attribute{Key: key, Val: value}) +} diff --git a/sanitize/sanitize_test.go b/sanitize/sanitize_test.go index 8f86d1b..d618eaf 100644 --- a/sanitize/sanitize_test.go +++ b/sanitize/sanitize_test.go @@ -180,6 +180,9 @@ func TestCharsetAddedWhenMissing(t *testing.T) { if !rep.CharsetAdded { t.Error("CharsetAdded = false, want true") } + if rep.CharsetRewritten { + t.Error("CharsetRewritten = true for a missing declaration") + } s := string(out) if !strings.Contains(strings.ToLower(s), ``) { t.Errorf("expected an injected meta charset:\n%s", s) @@ -266,10 +269,24 @@ func TestMobileReadableSkipsExistingViewport(t *testing.T) { } func TestCharsetNotDuplicated(t *testing.T) { - // A page that already declares a charset, in either form, is left alone. + in := `x` + out, rep, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + if rep.CharsetAdded || rep.CharsetRewritten { + t.Errorf("unchanged UTF-8 declaration reported a change: %+v", rep) + } + if n := strings.Count(strings.ToLower(string(out)), "charset"); n != 1 { + t.Errorf("charset count = %d, want 1:\n%s", n, out) + } +} + +func TestCharsetRewritesNonUTF8(t *testing.T) { cases := []string{ - `x`, - `x`, + `x`, + `x`, + `x`, } for _, in := range cases { out, rep, err := Strip([]byte(in), Options{}) @@ -277,14 +294,44 @@ func TestCharsetNotDuplicated(t *testing.T) { t.Fatal(err) } if rep.CharsetAdded { - t.Errorf("CharsetAdded = true for a page that already declares one:\n%s", in) + t.Errorf("CharsetAdded = true for an existing declaration:\n%s", in) + } + if !rep.CharsetRewritten { + t.Errorf("CharsetRewritten = false for:\n%s", in) + } + s := strings.ToLower(string(out)) + for _, stale := range []string{"iso-8859-1", "windows-1252", "shift_jis"} { + if strings.Contains(s, stale) { + t.Errorf("stale charset %q survived:\n%s", stale, out) + } } - if n := strings.Count(strings.ToLower(string(out)), "charset"); n != 1 { + if n := strings.Count(s, "charset"); n != 1 { t.Errorf("charset count = %d, want 1:\n%s", n, out) } } } +func TestCharsetInTemplateDoesNotDeclareDocumentEncoding(t *testing.T) { + in := `x` + out, rep, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + if !rep.CharsetAdded || !rep.CharsetRewritten { + t.Errorf("template declaration should be rewritten without satisfying the document: %+v", rep) + } + s := strings.ToLower(string(out)) + if strings.Contains(s, "shift_jis") { + t.Errorf("stale template charset survived:\n%s", out) + } + if n := strings.Count(s, `charset="utf-8"`); n != 2 { + t.Errorf("UTF-8 charset count = %d, want document and template declarations:\n%s", n, out) + } + if head, meta, template := strings.Index(s, ""), strings.Index(s, ""); head >= meta || meta >= template { + t.Errorf("document charset must be inserted before template content (head=%d meta=%d template=%d)", head, meta, template) + } +} + func TestDoctypePreservedAndBannerFollowsIt(t *testing.T) { // The browser hands the doctype back with the page, and everything sanitize // does has to leave it at the top of the file. A doctype anywhere but first