From 5af746f31f40de78337343986035ca25ccd1d573 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sat, 10 Oct 2026 16:34:16 +0200 Subject: [PATCH] fix(16.2): WR-04 serve content_html for markdown that starts with a tag LooksLikeHTML decided by the first character alone, so a markdown post opening with a hero , a
or a comment was served as raw markdown and its first edit stored ## and ** markers as HTML in both columns. Load now serves such a value as HTML only when content_html is empty or missing, or holds that same HTML (the value or SanitizeHTML of it, white space at the ends ignored); otherwise it serves content_html. --- classes/blog_editor.go | 77 ++++++++++++++++++-------- classes/blog_editor_test.go | 70 ++++++++++++++++++++++- controllers/posts_editor_smoke_test.go | 25 ++++++--- 3 files changed, 141 insertions(+), 31 deletions(-) diff --git a/classes/blog_editor.go b/classes/blog_editor.go index 35c8b90..68685d0 100644 --- a/classes/blog_editor.go +++ b/classes/blog_editor.go @@ -65,9 +65,10 @@ var ( // one. Both render BlogEditorElement. Decode stores what the HTML policy // keeps (SanitizeHTMLReport) and reports what it removed as notices; it // never refuses a string because of its markup. Load serves HTML content as -// it is and a markdown value (one that does not start with '<') as the -// rendered HTML of the htmlColumn option, and a blank value as the -// htmlColumn attribute when that holds a body. Upload stores an image the editor +// it is and a markdown value as the rendered HTML of the htmlColumn option, +// and a blank value as the htmlColumn attribute when that holds a body; a +// value that starts with '<' counts as HTML only when the htmlColumn +// attribute is empty or holds that same HTML (loadBlogHTML). Upload stores an image the editor // inserts through StoreMedia, in the media bucket the app publishes. app is // asked for the booted application at request time; it may be nil in tests, // where Upload then fails. @@ -107,9 +108,11 @@ func BlogEditorTypes(app func() *backpack.App) []pact.FormFieldType { } } -// LooksLikeHTML reports whether s is stored HTML rather than markdown: its -// first character that is not white space is '<' (an HTML comment -// included). Phase 16.2 assumption A1. +// LooksLikeHTML reports whether s may be stored HTML rather than markdown: +// its first character that is not white space is '<' (an HTML comment +// included). Phase 16.2 assumption A1. Markdown can start with a tag too, +// so Load also compares the value with its htmlColumn attribute before it +// serves the value as HTML. func LooksLikeHTML(s string) bool { t := strings.TrimLeftFunc(s, unicode.IsSpace) return strings.HasPrefix(t, "<") @@ -198,38 +201,68 @@ func removalNames(names []string, cut bool) string { return joined } -// loadBlogHTML serves a stored value to the editor (D-07). HTML content is -// served as it is. A markdown value is served as its rendered HTML: the -// htmlColumn attribute in the same locale, or FormatHTML of the value when -// that is empty or cannot be read (the value itself when rendering fails). -// A blank value is served as the htmlColumn attribute in the same locale -// when that is not empty, so a locale that carries only content_html (an -// API-written translation, FormatTranslation) opens with the body the site -// renders instead of an empty editor whose first edit would replace it; it -// is served as stored otherwise. +// loadBlogHTML serves a stored value to the editor (D-07). Without the +// htmlColumn option every value is served as stored. With it: +// +// - A blank value is served as the htmlColumn attribute in the same locale +// when that is not empty, so a locale that carries only content_html (an +// API-written translation, FormatTranslation) opens with the body the +// site renders instead of an empty editor whose first edit would replace +// it; it is served as stored otherwise. +// - A value that looks like HTML (LooksLikeHTML) is served as it is when +// the htmlColumn attribute is empty or missing, or holds the same HTML: +// the value itself or the policy's output for it (SanitizeHTML), white +// space at either end ignored. Otherwise the value is markdown that +// merely starts with a tag (a hero , a leading
or comment), +// and it is served as the htmlColumn attribute, so the first edit does +// not store raw markdown as HTML. +// - Any other value is markdown, served as the htmlColumn attribute, or as +// FormatHTML of the value when that is empty or cannot be read (the +// value itself when rendering fails). func loadBlogHTML(_ context.Context, in pact.FormFieldValue) (any, error) { text := storedText(in.Value) column, _ := in.Options["htmlColumn"].(string) - if column == "" || LooksLikeHTML(text) { + if column == "" { return text, nil } + html := "" if in.Translated != nil { - html, found, err := in.Translated(column) + value, found, err := in.Translated(column) if err != nil { return nil, err } - if found && html != "" { - return html, nil + if found { + html = value } } - if strings.TrimSpace(text) == "" { + switch { + case strings.TrimSpace(text) == "": + if html != "" { + return html, nil + } return text, nil + case LooksLikeHTML(text): + if html == "" || sameStoredHTML(text, html) { + return text, nil + } + return html, nil + case html != "": + return html, nil } - html, err := FormatHTML(text) + rendered, err := FormatHTML(text) if err != nil { return text, nil } - return html, nil + return rendered, nil +} + +// sameStoredHTML reports whether rendered, a stored htmlColumn value, is the +// HTML text itself: equal to text or to the policy's output for it, with +// white space at either end ignored. It is false when rendered came from +// rendering text as markdown in a way that changed it. +func sameStoredHTML(text, rendered string) bool { + rendered = strings.TrimSpace(rendered) + return rendered == strings.TrimSpace(text) || rendered == strings.TrimSpace(SanitizeHTML(text)) } // storedText is a stored column value as text: a string, a *string (nil is diff --git a/classes/blog_editor_test.go b/classes/blog_editor_test.go index 34f5486..65f6d7f 100644 --- a/classes/blog_editor_test.go +++ b/classes/blog_editor_test.go @@ -57,15 +57,81 @@ func TestBlogEditorLoad(t *testing.T) { "\n\n

both

\n", "

unsafe markup is served too; Decode strips it on save

", } { + // content_html missing, empty, the stored HTML itself (an admin + // save), the policy's output for it, or either with other white + // space at the ends (FormatHTML trims and goldmark appends "\n"). + for _, tc := range []struct { + name string + html string + found bool + }{ + {"missing", "", false}, + {"empty", "", true}, + {"equal", stored, true}, + {"sanitized", SanitizeHTML(stored), true}, + {"trimmed", strings.TrimSpace(stored) + "\n", true}, + {"sanitized and trimmed", "\n" + strings.TrimSpace(SanitizeHTML(stored)), true}, + } { + got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{ + Options: blogOptions, Locale: "pl", Default: true, Value: stored, + Translated: translatedReader(new([]string), tc.html, tc.found, nil), + }) + if err != nil || got != stored { + t.Fatalf("Load(%q) with content_html %s = %q, %v; want the stored bytes", stored, tc.name, got, err) + } + } got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{ - Options: blogOptions, Locale: "pl", Default: true, Value: stored, Translated: noTranslated(t), + Options: blogOptions, Locale: "pl", Default: true, Value: stored, }) if err != nil || got != stored { - t.Fatalf("Load(%q) = %q, %v; want the stored bytes", stored, got, err) + t.Fatalf("Load(%q) without a reader = %q, %v; want the stored bytes", stored, got, err) } } }) + // WR-04: markdown whose first character is '<' is not HTML when its + // content_html is a different rendering of it; serving the raw markdown + // would make the first edit store "##" and "**" as HTML. + t.Run("markdown that starts with a tag is served as content_html", func(t *testing.T) { + for _, source := range []string{ + "\"Hero\"\n\n## Nagłówek\n\nTekst z **pogrubieniem**.", + "
Wstęp
\n\n## Nagłówek\n\n*kursywa*", + "\n\n## Heading\n\n**bold**", + } { + stored, err := FormatHTML(source) + if err != nil { + t.Fatal(err) + } + if !LooksLikeHTML(source) || sameStoredHTML(source, stored) { + t.Fatalf("fixture %q does not exercise WR-04 (content_html %q)", source, stored) + } + var asked []string + got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{ + Options: blogOptions, Locale: "en", Value: source, + Translated: translatedReader(&asked, stored, true, nil), + }) + if err != nil || got != stored { + t.Fatalf("Load(%q) = %q, %v; want content_html %q", source, got, err, stored) + } + if strings.Contains(got.(string), "**") || strings.Contains(got.(string), "## ") { + t.Fatalf("Load(%q) served markdown markers: %q", source, got) + } + if len(asked) != 1 || asked[0] != "content_html" { + t.Fatalf("asked %v, want [content_html]", asked) + } + } + }) + + t.Run("an HTML value with a failing content_html read is the Load error", func(t *testing.T) { + boom := errors.New("read failed") + _, err := loadBlogHTML(context.Background(), pact.FormFieldValue{ + Options: blogOptions, Value: "

x

", Translated: translatedReader(new([]string), "", false, boom), + }) + if !errors.Is(err, boom) { + t.Fatalf("err = %v, want %v", err, boom) + } + }) + t.Run("markdown content is served as that locale's content_html", func(t *testing.T) { var asked []string got, err := loadBlogHTML(context.Background(), pact.FormFieldValue{ diff --git a/controllers/posts_editor_smoke_test.go b/controllers/posts_editor_smoke_test.go index 3f9f763..585b5d6 100644 --- a/controllers/posts_editor_smoke_test.go +++ b/controllers/posts_editor_smoke_test.go @@ -202,15 +202,26 @@ func TestBlogEditorTracerSmoke(t *testing.T) { } return got } - noRead := func(string) (string, bool, error) { - t.Fatal("Translated read for an HTML value") - return "", false, nil - } - t.Run("load serves HTML content as it is", func(t *testing.T) { const html = " \n

Stored HTML

" - if got := load(t, html, noRead); got != html { - t.Fatalf("Load(HTML) = %q", got) + for _, stored := range []string{"", html, html + "\n"} { + if got := load(t, html, func(string) (string, bool, error) { return stored, true, nil }); got != html { + t.Fatalf("Load(HTML) with content_html %q = %q", stored, got) + } + } + if got := load(t, html, nil); got != html { + t.Fatalf("Load(HTML, no reader) = %q", got) + } + }) + + t.Run("load serves markdown that starts with a tag as content_html", func(t *testing.T) { + const source = "\"Hero\"\n\n## Heading\n\n**bold**" + rendered, err := classes.FormatHTML(source) + if err != nil { + t.Fatal(err) + } + if got := load(t, source, func(string) (string, bool, error) { return rendered, true, nil }); got != rendered { + t.Fatalf("Load(markdown starting with ) = %q, want content_html %q", got, rendered) } })