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:

```
<a href="{{ .Destination | safeURL }}" >{{ .Text | markdownify }}</a>
<a href="{{ .Destination | safeURL }}" >{{ .Text | .Page.RenderString }}</a>
```

`.Text` is already rendered to `HTML`. The above needs to be rewritten to:

```
<a href="{{ .Destination | safeURL }}" >{{ .Text | safeHTML }}</a>
<a href="{{ .Destination | safeURL }}" >{{ .Text | safeHTML }}</a>
```

Fixes #8959

diff --git a/common/types/hstring/stringtypes.go b/common/types/hstring/stringtypes.go
new file mode 100644 (file)
index 0000000..601218e
--- /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 (file)
index 0000000..8ff477f
--- /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 d79b152f3b7729cc72ff01499d7aa53c2dbdd6c5..a7ad2a245a4a8161439c6e12024be225b21436a3 100644 (file)
--- 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 54ebf405ec50b0c1b8c1302d9bcc50226f4f1118..2f2d5e6cbedca7b8f68a74121e831017c602c509 100644 (file)
--- 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 d8f218b31ffa7dcc266684974d711dc6ec88a737..cab85cfdd4c6a48a0b6aa29edcb9c864c7feb4a2 100644 (file)
--- 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 --
+<a href="{{ .Destination | safeURL }}">{{ .Text | RENDERFUNC }}</a>    
+-- 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 a22030f545c39ef8c9e7b16581a4d836829a2b4e..6aafc70e170b312341a53ee37fbe100bd87c7b36 100644 (file)
--- 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 48cfaffffe0bb85d4a6ab7ea77303f31db03d4c9..498c0d674c5834976ffba7483560e8ef896c161e 100644 (file)
--- 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
 }