From 4e14cf7607ad3afdbf65272cd5bb61dba4b415da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Thu, 10 Mar 2022 08:19:03 +0100 Subject: Fail with error when double-rendering text in markdownify/RenderString This commit prevents the most commons case of infinite recursion in link render hooks when the `linkify` option is enabled (see below). This is always a user error, but getting a `stack overflow` (the current stack limit in Go is 1 GB on 64-bit, 250 MB on 32-bit) error isn't very helpful. This fix will not prevent all such errors, though, but we may do better once #9570 is in place. So, these will fail: ``` {{ .Text | markdownify }} {{ .Text | .Page.RenderString }} ``` `.Text` is already rendered to `HTML`. The above needs to be rewritten to: ``` {{ .Text | safeHTML }} {{ .Text | safeHTML }} ``` Fixes #8959 --- common/types/hstring/stringtypes.go | 20 ++++++++++++++ common/types/hstring/stringtypes_test.go | 30 +++++++++++++++++++++ hugolib/page__per_output.go | 12 +++++++-- markup/converter/hooks/hooks.go | 5 ++-- markup/goldmark/integration_test.go | 45 ++++++++++++++++++++++++++++++++ markup/goldmark/render_hooks.go | 17 ++++++------ tpl/transform/transform.go | 13 ++++----- 7 files changed, 122 insertions(+), 20 deletions(-) create mode 100644 common/types/hstring/stringtypes.go create mode 100644 common/types/hstring/stringtypes_test.go diff --git a/common/types/hstring/stringtypes.go b/common/types/hstring/stringtypes.go new file mode 100644 index 000000000..601218e0e --- /dev/null +++ b/common/types/hstring/stringtypes.go @@ -0,0 +1,20 @@ +// Copyright 2022 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hstring + +type RenderedString string + +func (s RenderedString) String() string { + return string(s) +} diff --git a/common/types/hstring/stringtypes_test.go b/common/types/hstring/stringtypes_test.go new file mode 100644 index 000000000..8ff477f63 --- /dev/null +++ b/common/types/hstring/stringtypes_test.go @@ -0,0 +1,30 @@ +// Copyright 2022 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hstring + +import ( + "html/template" + "testing" + + qt "github.com/frankban/quicktest" + "github.com/spf13/cast" +) + +func TestStringTypes(t *testing.T) { + c := qt.New(t) + + // Validate that it will behave like a string in Hugo settings. + c.Assert(cast.ToString(RenderedString("Hugo")), qt.Equals, "Hugo") + c.Assert(template.HTML(RenderedString("Hugo")), qt.Equals, template.HTML("Hugo")) +} diff --git a/hugolib/page__per_output.go b/hugolib/page__per_output.go index d79b152f3..a7ad2a245 100644 --- a/hugolib/page__per_output.go +++ b/hugolib/page__per_output.go @@ -24,6 +24,7 @@ import ( "unicode/utf8" "github.com/gohugoio/hugo/common/text" + "github.com/gohugoio/hugo/common/types/hstring" "github.com/gohugoio/hugo/identity" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -351,8 +352,16 @@ func (p *pageContentOutput) RenderString(args ...interface{}) (template.HTML, er } } + contentToRender := args[sidx] + + if _, ok := contentToRender.(hstring.RenderedString); ok { + // This content is already rendered, this is potentially + // a infinite recursion. + return "", errors.New("text is already rendered, repeating it may cause infinite recursion") + } + var err error - s, err = cast.ToStringE(args[sidx]) + s, err = cast.ToStringE(contentToRender) if err != nil { return "", err } @@ -515,7 +524,6 @@ func (p *pageContentOutput) initRenderHooks() error { } } } - if !found1 { if tp == hooks.CodeBlockRendererType { // No user provided tempplate for code blocks, so we use the native Go code version -- which is also faster. diff --git a/markup/converter/hooks/hooks.go b/markup/converter/hooks/hooks.go index 54ebf405e..2f2d5e6cb 100644 --- a/markup/converter/hooks/hooks.go +++ b/markup/converter/hooks/hooks.go @@ -18,6 +18,7 @@ import ( "github.com/gohugoio/hugo/common/hugio" "github.com/gohugoio/hugo/common/text" + "github.com/gohugoio/hugo/common/types/hstring" "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/markup/internal/attributes" ) @@ -32,7 +33,7 @@ type LinkContext interface { Page() interface{} Destination() string Title() string - Text() string + Text() hstring.RenderedString PlainText() string } @@ -75,7 +76,7 @@ type HeadingContext interface { // Anchor is the HTML id assigned to the heading. Anchor() string // Text is the rendered (HTML) heading text, excluding the heading marker. - Text() string + Text() hstring.RenderedString // PlainText is the unrendered version of Text. PlainText() string diff --git a/markup/goldmark/integration_test.go b/markup/goldmark/integration_test.go index d8f218b31..cab85cfdd 100644 --- a/markup/goldmark/integration_test.go +++ b/markup/goldmark/integration_test.go @@ -18,6 +18,8 @@ import ( "strings" "testing" + qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/hugolib" ) @@ -395,6 +397,49 @@ FENCE } } +// Iisse #8959 +func TestHookInfiniteRecursion(t *testing.T) { + t.Parallel() + + for _, renderFunc := range []string{"markdownify", ".Page.RenderString"} { + t.Run(renderFunc, func(t *testing.T) { + + files := ` +-- config.toml -- +-- layouts/_default/_markup/render-link.html -- +{{ .Text | RENDERFUNC }} +-- layouts/_default/single.html -- +{{ .Content }} +-- content/p1.md -- +--- +title: "p1" +--- + +https://example.org + +a@b.com + + + ` + + files = strings.ReplaceAll(files, "RENDERFUNC", renderFunc) + + b, err := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, "text is already rendered, repeating it may cause infinite recursion") + + }) + + } + +} + // Issue 9594 func TestQuotesInImgAltAttr(t *testing.T) { t.Parallel() diff --git a/markup/goldmark/render_hooks.go b/markup/goldmark/render_hooks.go index a22030f54..6aafc70e1 100644 --- a/markup/goldmark/render_hooks.go +++ b/markup/goldmark/render_hooks.go @@ -17,6 +17,7 @@ import ( "bytes" "strings" + "github.com/gohugoio/hugo/common/types/hstring" "github.com/gohugoio/hugo/markup/converter/hooks" "github.com/gohugoio/hugo/markup/goldmark/goldmark_config" "github.com/gohugoio/hugo/markup/goldmark/internal/render" @@ -49,7 +50,7 @@ type linkContext struct { page interface{} destination string title string - text string + text hstring.RenderedString plainText string } @@ -65,7 +66,7 @@ func (ctx linkContext) Page() interface{} { return ctx.page } -func (ctx linkContext) Text() string { +func (ctx linkContext) Text() hstring.RenderedString { return ctx.text } @@ -81,7 +82,7 @@ type headingContext struct { page interface{} level int anchor string - text string + text hstring.RenderedString plainText string *attributes.AttributesHolder } @@ -98,7 +99,7 @@ func (ctx headingContext) Anchor() string { return ctx.anchor } -func (ctx headingContext) Text() string { +func (ctx headingContext) Text() hstring.RenderedString { return ctx.text } @@ -156,7 +157,7 @@ func (r *hookedRenderer) renderImage(w util.BufWriter, source []byte, node ast.N page: ctx.DocumentContext().Document, destination: string(n.Destination), title: string(n.Title), - text: string(text), + text: hstring.RenderedString(text), plainText: string(n.Text(source)), }, ) @@ -226,7 +227,7 @@ func (r *hookedRenderer) renderLink(w util.BufWriter, source []byte, node ast.No page: ctx.DocumentContext().Document, destination: string(n.Destination), title: string(n.Title), - text: string(text), + text: hstring.RenderedString(text), plainText: string(n.Text(source)), }, ) @@ -293,7 +294,7 @@ func (r *hookedRenderer) renderAutoLink(w util.BufWriter, source []byte, node as linkContext{ page: ctx.DocumentContext().Document, destination: url, - text: label, + text: hstring.RenderedString(label), plainText: label, }, ) @@ -381,7 +382,7 @@ func (r *hookedRenderer) renderHeading(w util.BufWriter, source []byte, node ast page: ctx.DocumentContext().Document, level: n.Level, anchor: string(anchor), - text: string(text), + text: hstring.RenderedString(text), plainText: string(n.Text(source)), AttributesHolder: attributes.New(n.Attributes(), attributes.AttributesOwnerGeneral), }, diff --git a/tpl/transform/transform.go b/tpl/transform/transform.go index 48cfaffff..498c0d674 100644 --- a/tpl/transform/transform.go +++ b/tpl/transform/transform.go @@ -20,7 +20,6 @@ import ( "github.com/alecthomas/chroma/lexers" "github.com/gohugoio/hugo/cache/namedmemcache" - "github.com/gohugoio/hugo/common/herrors" "github.com/gohugoio/hugo/markup/converter/hooks" "github.com/gohugoio/hugo/markup/highlight" @@ -119,20 +118,18 @@ func (ns *Namespace) HTMLUnescape(s interface{}) (string, error) { // Markdownify renders a given input from Markdown to HTML. func (ns *Namespace) Markdownify(s interface{}) (template.HTML, error) { - defer herrors.Recover() - ss, err := cast.ToStringE(s) - if err != nil { - return "", err - } home := ns.deps.Site.Home() if home == nil { panic("home must not be nil") } - sss, err := home.RenderString(ss) + ss, err := home.RenderString(s) + if err != nil { + return "", err + } // Strip if this is a short inline type of text. - bb := ns.deps.ContentSpec.TrimShortHTML([]byte(sss)) + bb := ns.deps.ContentSpec.TrimShortHTML([]byte(ss)) return helpers.BytesToHTML(bb), nil } -- cgit v1.2.3