Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ All notable changes to kage are recorded here. The format follows
directly on its article and keeps that article's title metadata ([#62](https://github.com/tamnd/kage/issues/62)).
- Non-UTF-8 `<meta charset>` 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)).
Rewriting covers the whole document, but whether the page *declares* an
encoding is answered from `<head>` alone. A charset meta that Chrome left in
`<body>` was otherwise treated as the document's declaration, so nothing was
inserted into `<head>` and the only declaration sat past the 1024 bytes a
reader pre-scans, which is the mojibake #16 is about.
A legacy `content="charset=iso-8859-1"` with no media type is now rewritten
too, instead of being left to contradict the injected UTF-8 declaration.
- Relative links on redirected pages resolve against the browser's final URL
and the document's first `<base href>`, while the page remains saved under
the URL that was originally discovered. Consumed `href` attributes are
Expand Down
42 changes: 40 additions & 2 deletions sanitize/sanitize.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,14 @@ func ensureCharset(root *html.Node) (added, rewritten bool) {
if head == nil {
return false, false
}
// Rewriting stale values is a whole-document job: a declaration Chrome moved
// into <body> still contradicts the UTF-8 bytes kage writes.
fix := fixCharsetMetas(root)
if fix.declared {
// Whether the document *declares* an encoding is a <head> question, though.
// Readers pre-scan only the first 1024 bytes, so a meta stranded in <body>
// is never found; treating it as a declaration left the saved page with
// nothing a reader could act on, which is the mojibake of issue #16.
if headDeclaresCharset(head) {
return false, fix.rewritten
}
meta := &html.Node{
Expand Down Expand Up @@ -332,10 +338,42 @@ func fixCharsetMetas(n *html.Node) charsetMetaFix {

// rewriteContentTypeCharset rewrites a charset parameter while preserving the
// media type and other parameters. It accepts optional whitespace around '='.
// headDeclaresCharset reports whether <head> itself carries an encoding
// declaration, in either the <meta charset> or the legacy Content-Type form.
// Content inside <template> is inert and does not count.
func headDeclaresCharset(head *html.Node) bool {
var walk func(*html.Node) bool
walk = func(n *html.Node) bool {
if n.Type == html.ElementNode && n.DataAtom == atom.Meta {
if strings.TrimSpace(attr(n, "charset")) != "" {
return true
}
if strings.EqualFold(attr(n, "http-equiv"), "content-type") &&
strings.Contains(strings.ToLower(attr(n, "content")), "charset=") {
return true
}
}
if n.Type == html.ElementNode && n.DataAtom == atom.Template {
return false
}
for c := n.FirstChild; c != nil; c = c.NextSibling {
if walk(c) {
return true
}
}
return false
}
return walk(head)
}

func rewriteContentTypeCharset(content string) (string, charsetMetaFix) {
var fix charsetMetaFix
parts := strings.Split(content, ";")
for i := 1; i < len(parts); i++ {
// From 0, not 1: malformed markup writes content="charset=iso-8859-1" with
// no media type, and skipping the first field left that declaration in the
// saved page contradicting the UTF-8 bytes. A real media type has no "=",
// so the Cut below rejects it on its own.
for i := range parts {
key, value, ok := strings.Cut(parts[i], "=")
if !ok || !strings.EqualFold(strings.TrimSpace(key), "charset") {
continue
Expand Down
33 changes: 32 additions & 1 deletion sanitize/sanitize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ func TestCharsetRewritesNonUTF8(t *testing.T) {
cases := []string{
`<html><head><meta charset="ISO-8859-1"><title>x</title></head><body></body></html>`,
`<html><head><meta http-equiv="Content-Type" content="text/html; charset = windows-1252; foo=bar"><title>x</title></head><body></body></html>`,
`<html><head><title>x</title></head><body><meta charset="shift_jis"></body></html>`,
// A Content-Type with no media type at all, which is malformed but
// appears in the wild.
`<html><head><meta http-equiv="Content-Type" content="charset=iso-8859-1"><title>x</title></head><body></body></html>`,
}
for _, in := range cases {
out, rep, err := Strip([]byte(in), Options{})
Expand All @@ -348,6 +350,35 @@ func TestCharsetRewritesNonUTF8(t *testing.T) {
}
}

// A charset meta that ended up in <body> is rewritten, but it does not answer
// the question <head> has to answer: readers pre-scan only the first 1024 bytes,
// so a declaration stranded further down is never seen (issue #16).
func TestCharsetInBodyStillDeclaresInHead(t *testing.T) {
in := `<html><head><title>x</title></head><body><meta charset="shift_jis"><p>` +
strings.Repeat("padding ", 400) + `</p></body></html>`
out, rep, err := Strip([]byte(in), Options{})
if err != nil {
t.Fatal(err)
}
if !rep.CharsetAdded {
t.Errorf("CharsetAdded = false, so <head> declares nothing: %+v", rep)
}
if !rep.CharsetRewritten {
t.Errorf("CharsetRewritten = false, so the body declaration is still stale: %+v", rep)
}
s := strings.ToLower(string(out))
if strings.Contains(s, "shift_jis") {
t.Errorf("stale charset survived:\n%s", out)
}
if n := strings.Count(s, "charset"); n != 2 {
t.Errorf("charset count = %d, want 2 (head declaration plus the rewritten body one):\n%s", n, out)
}
// The declaration has to land inside the window a reader actually scans.
if i := strings.Index(s, "charset"); i < 0 || i >= 1024 {
t.Errorf("first charset at byte %d, outside the 1024-byte prescan window:\n%s", i, out)
}
}

func TestCharsetInTemplateDoesNotDeclareDocumentEncoding(t *testing.T) {
in := `<html><head><template><meta charset="shift_jis"></template><title>x</title></head><body></body></html>`
out, rep, err := Strip([]byte(in), Options{})
Expand Down