diff --git a/README.md b/README.md index 8f005c8..34b53ec 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ WinterCMS theme components such as `journalPost` and `journalPosts` are a later ## Features -- **Posts.** `models.Post` maps `golem15_journal_posts`. Translatable fields are title, content, content_html, excerpt, metadata, and slug (indexed). Fillable does not include `redactor_id`, `content_html`, or `user_id`. Attachments `featured_images` and `content_images` use `system_files` under the same MorphName. YAML admin uses `mltext`/`mlmarkdown`. `classes.FormatHTML` regenerates `content_html` on every write path (admin save, `journal:import-posts`, Journal API writes). It renders markdown and passes raw HTML in `content` through the renderer (Winter parity), then sanitizes the result with a `bluemonday` allow-list: bluemonday's UGC policy plus `footer`, `class`, schema.org microdata, the `color`, `font-size`, `font-weight` and `text-align` styles, goldmark's footnote roles, `dir="auto"` and two pasted-transcript `data-message-*` attributes. Like Winter's `Html::clean`, it strips what the policy does not allow instead of refusing the save: script, style, iframe, object, embed, form, base and meta elements, comments, event handler attributes, and URLs other than http, https, mailto and relative ones. HTML the policy keeps whole is stored byte for byte, so imported bodies survive a save. `content` itself is stored as submitted. Translated `content_html` goes through the same policy on the API and `journal:import-posts` write paths (`classes.FormatTranslation`): when the translation carries a non-blank `content`, `content_html` is rendered from it and a supplied `content_html` is replaced, as the host column is always derived; a translation with only `content_html` is sanitized with `classes.SanitizeHTML`. Winter stored a supplied translated `content_html` verbatim. The API response shapes and status codes are unchanged. +- **Posts.** `models.Post` maps `golem15_journal_posts`. Translatable fields are title, content, content_html, excerpt, metadata, and slug (indexed). Fillable does not include `redactor_id`, `content_html`, or `user_id`. Attachments `featured_images` and `content_images` use `system_files` under the same MorphName. YAML admin uses `mltext`/`mlmarkdown`. `classes.FormatHTML` regenerates `content_html` on every write path (admin save, `journal:import-posts`, Journal API writes). It renders markdown and passes raw HTML in `content` through the renderer (Winter parity), then sanitizes the result with a `bluemonday` allow-list: bluemonday's UGC policy plus `footer`, `class`, schema.org microdata, the `color`, `font-size`, `font-weight` and `text-align` styles, goldmark's footnote roles, `dir="auto"` and two pasted-transcript `data-message-*` attributes. Like Winter's `Html::clean`, it strips what the policy does not allow instead of refusing the save: script, style, iframe, object, embed, form, base and meta elements, comments, event handler attributes, and URLs other than http, https, mailto and relative ones. HTML the policy keeps whole is stored byte for byte, so imported bodies survive a save. A tag that repeats an attribute is never kept whole: the sanitized output, which keeps the first value, is stored. `content` itself is stored as submitted. Translated `content_html` goes through the same policy on the API and `journal:import-posts` write paths (`classes.FormatTranslation`): when the translation carries a non-blank `content`, `content_html` is rendered from it and a supplied `content_html` is replaced, as the host column is always derived; a translation with only `content_html` is sanitized with `classes.SanitizeHTML`. Winter stored a supplied translated `content_html` verbatim. The API response shapes and status codes are unchanged. - **Categories and tags.** `models.Category` is translatable (name, description, slug). It also stores `content`, an HTML host column that is not translatable, edited in the admin as a textarea. Fillable is name, slug, code, description, content and parent_id; nest_* columns exist in DDL and are not fillable. `models.Tag` is not translatable. Fillable is name, slug, description. Pivots are `golem15_journal_posts_categories` and `golem15_journal_posts_tags`. - **Settings.** Singleton row ID=1 on `golem15_journal_settings`, not PHP `system_settings`. `search_use_typesense` defaults to false. A fresh save never contacts Typesense. - **Public HTTP.** Anonymous GET `/_journal/api/v1/posts`, `/posts/{slug}`, `/categories`, `/tags`, `/rss`, `/authors/{author}`, and `/authors/{author}/posts`. Writes under the same prefix require a cabana backend JWT (`aud=backend`). @@ -179,7 +179,7 @@ func (Post) MorphName() string { return `Golem15\Journal\Models\Post | `updates.All` | gormigrate steps registered by this plugin. | | `classes.SanitizeHTML` | Strips from HTML (not markdown) what the `FormatHTML` allow-list does not allow; HTML the policy keeps whole is returned byte for byte. | | `classes.FormatTranslation` | Sets `content_html` in one locale's translated attributes: rendered from a non-blank `content` with `FormatHTML`, otherwise a supplied `content_html` sanitized with `SanitizeHTML`. | -| `classes.CheckHTML` | Gate for HTML stored as is, such as category content: refuses HTML that the `FormatHTML` allow-list would strip or change, instead of stripping it. | +| `classes.CheckHTML` | Gate for HTML stored as is, such as category content: refuses HTML that the `FormatHTML` allow-list would strip or change, including any tag that repeats an attribute, instead of stripping it. | ## CLI commands diff --git a/classes/check_html_test.go b/classes/check_html_test.go index 059c3b5..60f9de9 100644 --- a/classes/check_html_test.go +++ b/classes/check_html_test.go @@ -34,6 +34,11 @@ func TestCheckHTML(t *testing.T) { {"form", `
`, false}, {"style element", ``, false}, {"comment", `

a

`, false}, + {"duplicate href", `

x

`, false}, + {"duplicate src", ``, false}, + {"duplicate style", `

x

`, false}, + {"duplicate of an allowed attribute", `

x

`, false}, + {"attribute names inside a value", `

x

`, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/classes/format_html.go b/classes/format_html.go index 686c3c6..6d74337 100644 --- a/classes/format_html.go +++ b/classes/format_html.go @@ -136,10 +136,12 @@ func CheckHTML(doc string) error { // sanitizeHTML returns doc unchanged when htmlPolicy keeps all of it, and the // policy's output with changed set otherwise. Keeping all of it means doc and // the policy output tokenize to the same HTML tokens, so an output that only -// re-escapes text or attribute values is not a change. +// re-escapes text or attribute values is not a change. A tag that repeats an +// attribute is always a change: the tokenizer keeps only the first of them, +// so the comparison would not see the others in the stored bytes. func sanitizeHTML(doc string) (string, bool) { clean := htmlPolicy.Sanitize(doc) - if clean == doc || sameTokens(doc, clean) { + if sameTokens(doc, clean) { return doc, false } return clean, true @@ -147,12 +149,19 @@ func sanitizeHTML(doc string) (string, bool) { // sameTokens reports whether a and b tokenize to the same sequence of HTML // tokens: type, tag name or decoded text, and the set of decoded attributes. +// It is false when either repeats an attribute in a tag. func sameTokens(a, b string) bool { ta, oka := htmlTokens(a) + if !oka { + return false + } tb, okb := htmlTokens(b) - return oka && okb && slices.Equal(ta, tb) + return okb && slices.Equal(ta, tb) } +// htmlTokens tokenizes doc. It is not ok when the tokenizer stops on an +// error other than EOF, or when a start tag's raw attributes differ from the +// attributes the tokenizer kept, which happens when a name repeats. func htmlTokens(doc string) ([]string, bool) { var out []string z := xhtml.NewTokenizer(strings.NewReader(doc)) @@ -161,7 +170,21 @@ func htmlTokens(doc string) ([]string, bool) { if tt == xhtml.ErrorToken { return out, errors.Is(z.Err(), io.EOF) } + var raw []string + if tt == xhtml.StartTagToken || tt == xhtml.SelfClosingTagToken { + raw = rawAttrNames(z.Raw()) + } t := z.Token() + if tt == xhtml.StartTagToken || tt == xhtml.SelfClosingTagToken { + if len(raw) != len(t.Attr) { + return out, false + } + for i, a := range t.Attr { + if raw[i] != a.Key { + return out, false + } + } + } attrs := make([]string, 0, len(t.Attr)) for _, a := range t.Attr { attrs = append(attrs, a.Namespace+":"+a.Key+"="+a.Val) @@ -170,3 +193,108 @@ func htmlTokens(doc string) ([]string, bool) { out = append(out, fmt.Sprintf("%d|%s|%q", tt, t.Data, attrs)) } } + +// rawAttrNames returns the ASCII-lowercased name of every attribute in the +// raw start tag, repeats included, in source order. It scans the tag as +// golang.org/x/net/html's tokenizer does (readTagName, readTagAttrKey and +// readTagAttrVal), which drops every repeat of a name it has seen. A scan +// that disagrees with the tokenizer only makes htmlTokens report a change. +func rawAttrNames(raw []byte) []string { + i, n := 2, len(raw) // past '<' and the first byte of the tag name + skipSpace := func() { + for i < n && isHTMLSpace(raw[i]) { + i++ + } + } + for i < n { + c := raw[i] + i++ + if isHTMLSpace(c) { + break + } + if c == '/' || c == '>' { + i-- + break + } + } + var names []string + skipSpace() + for i < n { + if raw[i] == '>' { + break + } + // Attribute name: an '=' that starts it is part of the name. + start, end := i, n + for i < n { + c := raw[i] + i++ + if c == '=' && i == start+1 { + continue + } + if c == '=' || c == '/' || c == '>' || isHTMLSpace(c) { + i-- + end = i + break + } + } + // Attribute value. + skipSpace() + if i < n { + c := raw[i] + i++ + switch { + case c == '/': + case c != '=': + i-- + default: + skipSpace() + if i < n { + q := raw[i] + i++ + switch q { + case '>': + i-- + case '"', '\'': + for i < n { + i++ + if raw[i-1] == q { + break + } + } + default: + for i < n { + c := raw[i] + i++ + if isHTMLSpace(c) { + break + } + if c == '>' { + i-- + break + } + } + } + } + } + } + if end > start { + names = append(names, asciiLower(raw[start:end])) + } + skipSpace() + } + return names +} + +func isHTMLSpace(c byte) bool { + return c == ' ' || c == '\n' || c == '\r' || c == '\t' || c == '\f' +} + +func asciiLower(b []byte) string { + out := []byte(string(b)) + for i, c := range out { + if 'A' <= c && c <= 'Z' { + out[i] = c + 'a' - 'A' + } + } + return string(out) +} diff --git a/classes/format_html_test.go b/classes/format_html_test.go index 93d92a7..2bc3da2 100644 --- a/classes/format_html_test.go +++ b/classes/format_html_test.go @@ -49,6 +49,9 @@ var unsafePayloads = []struct { {"style element", ""}, {"link stylesheet", ``}, {"html comment", "

a

"}, + {"duplicate href", `

x

`}, + {"duplicate src", ``}, + {"duplicate style", `

x

`}, } // forbiddenElements must never appear in sanitized journal HTML. @@ -159,6 +162,72 @@ func TestFormatHTMLSanitizesUnsafeHTML(t *testing.T) { }) } +// TestFormatHTMLDuplicateAttributes pins W-2: the tokenizer keeps only the +// first of repeated attributes, so input that repeats one is never stored +// byte for byte. The sanitized output keeps the first value, as browsers do. +func TestFormatHTMLDuplicateAttributes(t *testing.T) { + cases := []struct { + name, src, want string + }{ + {"duplicate href", `

x

`, `

x

`}, + {"duplicate href in other case", `

x

`, `

x

`}, + {"duplicate src", ``, ``}, + {"duplicate style", `

x

`, `

x

`}, + {"duplicate class", `

x

`, `

x

`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, err := FormatHTML(tc.src) + if err != nil { + t.Fatalf("FormatHTML(%q): %v", tc.src, err) + } + if out != tc.want { + t.Fatalf("got %q, want %q", out, tc.want) + } + }) + } +} + +// TestRawAttrNames checks the raw attribute scan against the tokenizer: for +// tags without repeats both list the same names in the same order. +func TestRawAttrNames(t *testing.T) { + tags := []string{ + `

`, + `
`, + `b`, + ``, + ``, + ``, + `

`, + "", + `

`, + ``, + `

`, + `