From e2bd987c2d6f0414380affdb6bc161c7dba750e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 21 Jan 2025 10:33:27 +0100 Subject: [PATCH 1/4] Do not warn on potentially unsafe HTML comments when unsafe=false We will still not render these comments, so from a safety perspective this is the same, but HTML comments are very common also inside Markdown and too useful to throw away. Updates #13278 --- markup/goldmark/goldmark_integration_test.go | 51 ++++++++++++++++++++ markup/goldmark/hugocontext/hugocontext.go | 17 +++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/markup/goldmark/goldmark_integration_test.go b/markup/goldmark/goldmark_integration_test.go index 591226dc2fe..23e22b5cafd 100644 --- a/markup/goldmark/goldmark_integration_test.go +++ b/markup/goldmark/goldmark_integration_test.go @@ -851,3 +851,54 @@ title: "p1" b.AssertFileContent("public/p1/index.html", "! ") b.AssertLogContains("! WARN") } + +// See https://github.com/gohugoio/hugo/issues/13278#issuecomment-2603280548 +func TestGoldmarkRawHTMLCommentNoWarning(t *testing.T) { + files := ` +-- hugo.toml -- +disableKinds = ['home','rss','section','sitemap','taxonomy','term'] +markup.goldmark.renderer.unsafe = false +-- content/p1.md -- +--- +title: "p1" +--- +# HTML comments + +## Simple + + + + + **Hello**_world_. +## With HTML + + + +## With HTML and JS + + + +## With Block + + + +XSS + + + +-- layouts/_default/single.html -- +{{ .Content }} +` + + b := hugolib.Test(t, files, hugolib.TestOptWarn()) + + b.AssertFileContent("public/p1/index.html", "! ") + b.AssertLogContains("! Raw HTML omitted") + + b = hugolib.Test(t, strings.ReplaceAll(files, "markup.goldmark.renderer.unsafe = false", "markup.goldmark.renderer.unsafe = true"), hugolib.TestOptWarn()) + b.AssertFileContent("public/p1/index.html", "") + b.AssertLogContains("! WARN") +} diff --git a/markup/goldmark/hugocontext/hugocontext.go b/markup/goldmark/hugocontext/hugocontext.go index 601014b3705..e610bbbebaf 100644 --- a/markup/goldmark/hugocontext/hugocontext.go +++ b/markup/goldmark/hugocontext/hugocontext.go @@ -174,6 +174,9 @@ func (r *hugoContextRenderer) renderHTMLBlock( w util.BufWriter, source []byte, node ast.Node, entering bool, ) (ast.WalkStatus, error) { n := node.(*ast.HTMLBlock) + isHTMLComment := func(b []byte) bool { + return len(b) > 4 && b[0] == '<' && b[1] == '!' && b[2] == '-' && b[3] == '-' + } if entering { if r.Unsafe { l := n.Lines().Len() @@ -188,8 +191,12 @@ func (r *hugoContextRenderer) renderHTMLBlock( r.Writer.SecureWrite(w, linev) } } else { - r.logRawHTMLEmittedWarn(w) - _, _ = w.WriteString("\n") + l := n.Lines().At(0) + v := l.Value(source) + if !isHTMLComment(v) { + r.logRawHTMLEmittedWarn(w) + _, _ = w.WriteString("\n") + } } } else { if n.HasClosure() { @@ -197,7 +204,11 @@ func (r *hugoContextRenderer) renderHTMLBlock( closure := n.ClosureLine r.Writer.SecureWrite(w, closure.Value(source)) } else { - _, _ = w.WriteString("\n") + l := n.Lines().At(0) + v := l.Value(source) + if !isHTMLComment(v) { + _, _ = w.WriteString("\n") + } } } } From 22145b5999d1f1ded65e82ce863ea27fd522de67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 21 Jan 2025 18:52:58 +0100 Subject: [PATCH 2/4] Also handle inline HTML comments --- markup/goldmark/goldmark_integration_test.go | 19 +++++++++++++++- markup/goldmark/hugocontext/hugocontext.go | 24 ++++++++++++-------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/markup/goldmark/goldmark_integration_test.go b/markup/goldmark/goldmark_integration_test.go index 23e22b5cafd..356e601531e 100644 --- a/markup/goldmark/goldmark_integration_test.go +++ b/markup/goldmark/goldmark_integration_test.go @@ -885,10 +885,27 @@ title: "p1" Trulli --> -XSS +## XSS + +## More + +This is a word. + +This is a word. + +This is a word. + +This is a word. + +This is a word. + + -- layouts/_default/single.html -- {{ .Content }} ` diff --git a/markup/goldmark/hugocontext/hugocontext.go b/markup/goldmark/hugocontext/hugocontext.go index e610bbbebaf..e68acb8c31d 100644 --- a/markup/goldmark/hugocontext/hugocontext.go +++ b/markup/goldmark/hugocontext/hugocontext.go @@ -169,14 +169,16 @@ func (r *hugoContextRenderer) getPage(w util.BufWriter) any { return p } +func (r *hugoContextRenderer) isHTMLComment(b []byte) bool { + return len(b) > 4 && b[0] == '<' && b[1] == '!' && b[2] == '-' && b[3] == '-' +} + // HTML rendering based on Goldmark implementation. func (r *hugoContextRenderer) renderHTMLBlock( w util.BufWriter, source []byte, node ast.Node, entering bool, ) (ast.WalkStatus, error) { n := node.(*ast.HTMLBlock) - isHTMLComment := func(b []byte) bool { - return len(b) > 4 && b[0] == '<' && b[1] == '!' && b[2] == '-' && b[3] == '-' - } + if entering { if r.Unsafe { l := n.Lines().Len() @@ -193,7 +195,7 @@ func (r *hugoContextRenderer) renderHTMLBlock( } else { l := n.Lines().At(0) v := l.Value(source) - if !isHTMLComment(v) { + if !r.isHTMLComment(v) { r.logRawHTMLEmittedWarn(w) _, _ = w.WriteString("\n") } @@ -206,7 +208,7 @@ func (r *hugoContextRenderer) renderHTMLBlock( } else { l := n.Lines().At(0) v := l.Value(source) - if !isHTMLComment(v) { + if !r.isHTMLComment(v) { _, _ = w.WriteString("\n") } } @@ -221,17 +223,21 @@ func (r *hugoContextRenderer) renderRawHTML( if !entering { return ast.WalkSkipChildren, nil } + n := node.(*ast.RawHTML) + l := n.Segments.Len() if r.Unsafe { - n := node.(*ast.RawHTML) - l := n.Segments.Len() for i := 0; i < l; i++ { segment := n.Segments.At(i) _, _ = w.Write(segment.Value(source)) } return ast.WalkSkipChildren, nil } - r.logRawHTMLEmittedWarn(w) - _, _ = w.WriteString("") + segment := n.Segments.At(0) + v := segment.Value(source) + if !r.isHTMLComment(v) { + r.logRawHTMLEmittedWarn(w) + _, _ = w.WriteString("") + } return ast.WalkSkipChildren, nil } From 53637f03f4431da76507cbd1314210d4ed402775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 21 Jan 2025 19:19:20 +0100 Subject: [PATCH 3/4] Improve assertions --- markup/goldmark/goldmark_integration_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/markup/goldmark/goldmark_integration_test.go b/markup/goldmark/goldmark_integration_test.go index 356e601531e..faccf15ac4f 100644 --- a/markup/goldmark/goldmark_integration_test.go +++ b/markup/goldmark/goldmark_integration_test.go @@ -912,10 +912,19 @@ hidden b := hugolib.Test(t, files, hugolib.TestOptWarn()) - b.AssertFileContent("public/p1/index.html", "! ") + b.AssertFileContent("public/p1/index.html", + "! ", + "! ", + "! ", + "! script", + ) b.AssertLogContains("! Raw HTML omitted") b = hugolib.Test(t, strings.ReplaceAll(files, "markup.goldmark.renderer.unsafe = false", "markup.goldmark.renderer.unsafe = true"), hugolib.TestOptWarn()) - b.AssertFileContent("public/p1/index.html", "") + b.AssertFileContent("public/p1/index.html", + "! ", + "", + "", + ) b.AssertLogContains("! WARN") } From cd5a8ce1afed1370c2baf77ae15afcb7bf599802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 21 Jan 2025 19:24:51 +0100 Subject: [PATCH 4/4] Improve assert --- markup/goldmark/goldmark_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/markup/goldmark/goldmark_integration_test.go b/markup/goldmark/goldmark_integration_test.go index faccf15ac4f..67348d894d8 100644 --- a/markup/goldmark/goldmark_integration_test.go +++ b/markup/goldmark/goldmark_integration_test.go @@ -918,7 +918,7 @@ hidden "! ", "! script", ) - b.AssertLogContains("! Raw HTML omitted") + b.AssertLogContains("! WARN") b = hugolib.Test(t, strings.ReplaceAll(files, "markup.goldmark.renderer.unsafe = false", "markup.goldmark.renderer.unsafe = true"), hugolib.TestOptWarn()) b.AssertFileContent("public/p1/index.html",