From 334c055f972a820cba9ed2e4a12d2e038829d8f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 30 Oct 2024 18:10:09 +0100 Subject: [PATCH] Fix some RenderShortcodes error cases This issue fixes two cases where `{{__hugo_ctx` artifacts were left in the rendered output: 1. Inclusion when `.RenderShortcodes` is wrapped in HTML. 2. Inclusion of Markdown file without a trailing newline in some cases. Closes #12854 --- common/constants/constants.go | 1 + hugolib/integrationtest_builder.go | 23 +++++ hugolib/rendershortcodes_test.go | 103 ++++++++++++++++++++ markup/goldmark/convert.go | 2 +- markup/goldmark/hugocontext/hugocontext.go | 107 ++++++++++++++++++--- markup/goldmark/internal/render/context.go | 14 +++ 6 files changed, 233 insertions(+), 17 deletions(-) diff --git a/common/constants/constants.go b/common/constants/constants.go index f8f057e053b..752aef72c3c 100644 --- a/common/constants/constants.go +++ b/common/constants/constants.go @@ -21,6 +21,7 @@ const ( ErrRemoteGetCSV = "error-remote-getcsv" WarnFrontMatterParamsOverrides = "warning-frontmatter-params-overrides" + WarnRenderShortcodesInHTML = "warning-rendershortcodes-in-html" ) // Field/method names with special meaning. diff --git a/hugolib/integrationtest_builder.go b/hugolib/integrationtest_builder.go index b806ad7c143..1c5d497582b 100644 --- a/hugolib/integrationtest_builder.go +++ b/hugolib/integrationtest_builder.go @@ -2,6 +2,7 @@ package hugolib import ( "bytes" + "context" "encoding/base64" "errors" "fmt" @@ -32,6 +33,7 @@ import ( "github.com/gohugoio/hugo/htesting" "github.com/gohugoio/hugo/hugofs" "github.com/spf13/afero" + "github.com/spf13/cast" "golang.org/x/text/unicode/norm" "golang.org/x/tools/txtar" ) @@ -294,6 +296,12 @@ func (s *IntegrationTestBuilder) AssertFileContent(filename string, matches ...s } } +func (s *IntegrationTestBuilder) AssertFileContentEquals(filename string, match string) { + s.Helper() + content := s.FileContent(filename) + s.Assert(content, qt.Equals, match, qt.Commentf(match)) +} + func (s *IntegrationTestBuilder) AssertFileContentExact(filename string, matches ...string) { s.Helper() content := s.FileContent(filename) @@ -302,6 +310,16 @@ func (s *IntegrationTestBuilder) AssertFileContentExact(filename string, matches } } +func (s *IntegrationTestBuilder) AssertNoRenderShortcodesArtifacts() { + s.Helper() + for _, p := range s.H.Pages() { + content, err := p.Content(context.Background()) + s.Assert(err, qt.IsNil) + comment := qt.Commentf("Page: %s", p.Path()) + s.Assert(strings.Contains(cast.ToString(content), "__hugo_ctx"), qt.IsFalse, comment) + } +} + func (s *IntegrationTestBuilder) AssertPublishDir(matches ...string) { s.AssertFs(s.fs.PublishDir, matches...) } @@ -835,6 +853,11 @@ type IntegrationTestConfig struct { // The files to use on txtar format, see // https://pkg.go.dev/golang.org/x/exp/cmd/txtar + // There are some conentions used in this test setup. + // - §§§ can be used to wrap code fences. + // - §§ can be used to wrap multiline strings. + // - filenames prefixed with sourcefilename: will be read from the file system relative to the current dir. + // - filenames with a .png or .jpg extension will be treated as binary and base64 decoded. TxtarString string // COnfig to use as the base. We will also read the config from the txtar. diff --git a/hugolib/rendershortcodes_test.go b/hugolib/rendershortcodes_test.go index 313c80a73fc..964d120c36d 100644 --- a/hugolib/rendershortcodes_test.go +++ b/hugolib/rendershortcodes_test.go @@ -69,6 +69,7 @@ Content: {{ .Content }}| b := Test(t, files) + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Fragments: [p1-h1 p2-h1 p2-h2 p2-h3 p2-withmarkdown p3-h1 p3-h2 p3-withmarkdown]|", "HasShortcode Level 1: true|", @@ -115,6 +116,7 @@ JSON: {{ .Content }} b := Test(t, files) + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Myshort HTML") b.AssertFileContent("public/p1/index.json", "Myshort JSON") } @@ -147,9 +149,11 @@ Myshort Original. {{ .Content }} ` b := TestRunning(t, files) + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Myshort Original.") b.EditFileReplaceAll("layouts/shortcodes/myshort.html", "Original", "Edited").Build() + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Myshort Edited.") } @@ -192,12 +196,14 @@ Myshort Original. }, ).Build() + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Original") b.EditFileReplaceFunc("content/p2.md", func(s string) string { return strings.Replace(s, "Original", "Edited", 1) }) b.Build() + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/p1/index.html", "Edited") } @@ -233,8 +239,10 @@ Myshort Original. ` b := TestRunning(t, files) + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/mysection/index.html", "p1-h1") b.EditFileReplaceAll("content/mysection/_index.md", "p1-h1", "p1-h1 Edited").Build() + b.AssertNoRenderShortcodesArtifacts() b.AssertFileContent("public/mysection/index.html", "p1-h1 Edited") } @@ -314,6 +322,8 @@ iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAA b := Test(t, files) + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContent("public/markdown/index.html", // Images. "Image: /posts/p1/pixel1.png|\nImage: /posts/p1/pixel2.png|\n|\nImage: /markdown/pixel3.png|

\n|", @@ -333,3 +343,96 @@ iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAA b.AssertFileContent("public/html/index.html", "! hugo_ctx") } + +// Issue 12854. +func TestRenderShortcodesWithHTML(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +disableLiveReload = true +disableKinds = ["home", "taxonomy", "term"] +markup.goldmark.renderer.unsafe = true +-- content/p1.md -- +--- +title: "p1" +--- +{{% include "p2" %}} +-- content/p2.md -- +--- +title: "p2" +--- +Hello world. Some **bold** text. Some Unicode: 神真美好. +-- layouts/shortcodes/include.html -- +{{ with site.GetPage (.Get 0) }} +
{{ .RenderShortcodes }}
+{{ end }} +-- layouts/_default/single.html -- +{{ .Content }} +` + + b := TestRunning(t, files, TestOptWarn()) + + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContent("public/p1/index.html", "
Hello world. Some **bold** text. Some Unicode: 神真美好.
") + b.EditFileReplaceAll("content/p2.md", "Hello", "Hello Edited").Build() + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContent("public/p1/index.html", "
Hello Edited world. Some **bold** text. Some Unicode: 神真美好.
") +} + +func TestRenderShortcodesIncludeMarkdownFileWithoutTrailingNewline(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +disableLiveReload = true +disableKinds = ["home", "taxonomy", "term"] +markup.goldmark.renderer.unsafe = true +-- content/p1.md -- +--- +title: "p1" +--- +Content p1 id-1000.{{% include "p2" %}}{{% include "p3" %}} + +§§§ go +code_p1 +§§§ +-- content/p2.md -- +--- +title: "p2" +--- +§§§ go +code_p2 +§§§ +Foo. +-- content/p3.md -- +--- +title: "p3" +--- +§§§ go +code_p3 +§§§ +-- layouts/shortcodes/include.html -- +{{ with site.GetPage (.Get 0) -}} +{{ .RenderShortcodes -}} +{{ end -}} +-- layouts/_default/single.html -- +{{ .Content }} +-- layouts/_default/_markup/render-codeblock.html -- +{{ .Inner | safeHTML }} +` + + b := TestRunning(t, files, TestOptWarn()) + + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContentEquals("public/p1/index.html", "

Content p1 id-1000.

\ncode_p2

Foo.\n

\ncode_p3code_p1") + b.EditFileReplaceAll("content/p1.md", "id-1000.", "id-100.").Build() + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContentEquals("public/p1/index.html", "

Content p1 id-100.

\ncode_p2

Foo.\n

\ncode_p3code_p1") + b.EditFileReplaceAll("content/p2.md", "code_p2", "codep2").Build() + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContentEquals("public/p1/index.html", "

Content p1 id-100.

\ncodep2

Foo.\n

\ncode_p3code_p1") + b.EditFileReplaceAll("content/p3.md", "code_p3", "code_p3_edited").Build() + b.AssertNoRenderShortcodesArtifacts() + b.AssertFileContentEquals("public/p1/index.html", "

Content p1 id-100.

\ncodep2

Foo.\n

\ncode_p3_editedcode_p1") +} diff --git a/markup/goldmark/convert.go b/markup/goldmark/convert.go index 5c31eee40d9..ea3bbc4ae9b 100644 --- a/markup/goldmark/convert.go +++ b/markup/goldmark/convert.go @@ -106,7 +106,7 @@ func newMarkdown(pcfg converter.ProviderConfig) goldmark.Markdown { renderer.WithNodeRenderers(util.Prioritized(emoji.NewHTMLRenderer(), 200))) var ( extensions = []goldmark.Extender{ - hugocontext.New(), + hugocontext.New(pcfg.Logger), newLinks(cfg), newTocExtension(tocRendererOptions), blockquotes.New(), diff --git a/markup/goldmark/hugocontext/hugocontext.go b/markup/goldmark/hugocontext/hugocontext.go index b9c548dac5e..7d0dcaa850f 100644 --- a/markup/goldmark/hugocontext/hugocontext.go +++ b/markup/goldmark/hugocontext/hugocontext.go @@ -16,20 +16,24 @@ package hugocontext import ( "bytes" "fmt" + "regexp" "strconv" "github.com/gohugoio/hugo/bufferpool" + "github.com/gohugoio/hugo/common/constants" + "github.com/gohugoio/hugo/common/loggers" "github.com/gohugoio/hugo/markup/goldmark/internal/render" "github.com/yuin/goldmark" "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/parser" "github.com/yuin/goldmark/renderer" + "github.com/yuin/goldmark/renderer/html" "github.com/yuin/goldmark/text" "github.com/yuin/goldmark/util" ) -func New() goldmark.Extender { - return &hugoContextExtension{} +func New(logger loggers.Logger) goldmark.Extender { + return &hugoContextExtension{logger: logger} } // Wrap wraps the given byte slice in a Hugo context that used to determine the correct Page @@ -37,14 +41,15 @@ func New() goldmark.Extender { func Wrap(b []byte, pid uint64) string { buf := bufferpool.GetBuffer() defer bufferpool.PutBuffer(buf) - buf.Write(prefix) + buf.Write(hugoCtxPrefix) buf.WriteString(" pid=") buf.WriteString(strconv.FormatUint(pid, 10)) - buf.Write(endDelim) + buf.Write(hugoCtxEndDelim) buf.WriteByte('\n') buf.Write(b) - buf.Write(prefix) - buf.Write(closingDelimAndNewline) + buf.WriteByte('\n') + buf.Write(hugoCtxPrefix) + buf.Write(hugoCtxClosingDelimAndNewline) return buf.String() } @@ -89,9 +94,10 @@ func (h *HugoContext) Kind() ast.NodeKind { } var ( - prefix = []byte("{{__hugo_ctx") - endDelim = []byte("}}") - closingDelimAndNewline = []byte("/}}\n") + hugoCtxPrefix = []byte("{{__hugo_ctx") + hugoCtxEndDelim = []byte("}}") + hugoCtxClosingDelimAndNewline = []byte("/}}\n") + hugoCtxRe = regexp.MustCompile(`{{__hugo_ctx( pid=\d+)?/?}}\n?`) ) var _ parser.InlineParser = (*hugoContextParser)(nil) @@ -100,21 +106,26 @@ type hugoContextParser struct{} func (s *hugoContextParser) Parse(parent ast.Node, block text.Reader, pc parser.Context) ast.Node { line, _ := block.PeekLine() - if !bytes.HasPrefix(line, prefix) { + + if !bytes.HasPrefix(line, hugoCtxPrefix) { return nil } - end := bytes.Index(line, endDelim) + end := bytes.Index(line, hugoCtxEndDelim) if end == -1 { return nil } - block.Advance(end + len(endDelim) + 1) // +1 for the newline + block.Advance(end + len(hugoCtxEndDelim) + 1) // +1 for the newline if line[end-1] == '/' { + if parent.Kind() == ast.KindParagraph && !parent.HasChildren() { + // Remove empty paragraph created by the {{__hugo_ctx/}}. + parent.Parent().RemoveChild(parent.Parent(), parent) + } return &HugoContext{Closing: true} } - attrBytes := line[len(prefix)+1 : end] + attrBytes := line[len(hugoCtxPrefix)+1 : end] h := &HugoContext{} h.parseAttrs(attrBytes) return h @@ -124,10 +135,67 @@ func (a *hugoContextParser) Trigger() []byte { return []byte{'{'} } -type hugoContextRenderer struct{} +type hugoContextRenderer struct { + logger loggers.Logger + html.Config +} + +func (r *hugoContextRenderer) SetOption(name renderer.OptionName, value any) { + r.Config.SetOption(name, value) +} func (r *hugoContextRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) { reg.Register(kindHugoContext, r.handleHugoContext) + reg.Register(ast.KindHTMLBlock, r.renderHTMLBlock) +} + +func (r *hugoContextRenderer) stripHugoCtx(b []byte) ([]byte, bool) { + if !bytes.Contains(b, hugoCtxPrefix) { + return b, false + } + return hugoCtxRe.ReplaceAll(b, nil), true +} + +func (r *hugoContextRenderer) renderHTMLBlock( + w util.BufWriter, source []byte, node ast.Node, entering bool, +) (ast.WalkStatus, error) { + n := node.(*ast.HTMLBlock) + if entering { + if r.Unsafe { + l := n.Lines().Len() + for i := 0; i < l; i++ { + line := n.Lines().At(i) + linev := line.Value(source) + // TODO1 add warn with ID. + var stripped bool + linev, stripped = r.stripHugoCtx(linev) + if stripped { + var pageContext string + ctx, ok := w.(*render.Context) + if ok { + p, _ := render.GetPageAndPageInnerAsPage(ctx) + if p != nil && p.PathInfo() != nil { + pageContext = p.PathInfo().Path() + } + } + r.logger.Warnidf(constants.WarnRenderShortcodesInHTML, ".RenderShortcodes detected inside HTML block in %q; this may not be what you intended, see https://gohugo.io/methods/page/rendershortcodes/#limitations", pageContext) + } + r.Writer.SecureWrite(w, linev) + } + } else { + _, _ = w.WriteString("\n") + } + } else { + if n.HasClosure() { + if r.Unsafe { + closure := n.ClosureLine + r.Writer.SecureWrite(w, closure.Value(source)) + } else { + _, _ = w.WriteString("\n") + } + } + } + return ast.WalkContinue, nil } func (r *hugoContextRenderer) handleHugoContext(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) { @@ -148,7 +216,9 @@ func (r *hugoContextRenderer) handleHugoContext(w util.BufWriter, source []byte, return ast.WalkContinue, nil } -type hugoContextExtension struct{} +type hugoContextExtension struct { + logger loggers.Logger +} func (a *hugoContextExtension) Extend(m goldmark.Markdown) { m.Parser().AddOptions( @@ -159,7 +229,12 @@ func (a *hugoContextExtension) Extend(m goldmark.Markdown) { m.Renderer().AddOptions( renderer.WithNodeRenderers( - util.Prioritized(&hugoContextRenderer{}, 50), + util.Prioritized(&hugoContextRenderer{ + logger: a.logger, + Config: html.Config{ + Writer: html.DefaultWriter, + }, + }, 50), ), ) } diff --git a/markup/goldmark/internal/render/context.go b/markup/goldmark/internal/render/context.go index b8cf9ba54ef..feab7c535d6 100644 --- a/markup/goldmark/internal/render/context.go +++ b/markup/goldmark/internal/render/context.go @@ -18,6 +18,7 @@ import ( "math/bits" "sync" + "github.com/gohugoio/hugo/common/paths" htext "github.com/gohugoio/hugo/common/text" "github.com/gohugoio/hugo/markup/converter" @@ -175,6 +176,11 @@ func extractSourceSample(n ast.Node, src []byte) []byte { return sample } +// A sub set of page.Page. +type Page interface { + PathInfo() *paths.Path +} + // GetPageAndPageInner returns the current page and the inner page for the given context. func GetPageAndPageInner(rctx *Context) (any, any) { p := rctx.DocumentContext().Document @@ -189,6 +195,14 @@ func GetPageAndPageInner(rctx *Context) (any, any) { return p, p } +// GetPageAndPageInnerAsPage returns the current page and the inner page for the given context. +func GetPageAndPageInnerAsPage(rctx *Context) (Page, Page) { + v1, v2 := GetPageAndPageInner(rctx) + p1, _ := v1.(Page) + p2, _ := v2.(Page) + return p1, p2 +} + // NewBaseContext creates a new BaseContext. func NewBaseContext(rctx *Context, renderer any, n ast.Node, src []byte, getSourceSample func() []byte, ordinal int) hooks.BaseContext { if getSourceSample == nil {