From 5643e663e7e60e48f50d65a28d15ff4c11718427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sun, 20 Apr 2025 18:20:17 +0200 Subject: [PATCH] tpl: Detect and fail on infinite template recursion Fixes #13627 --- config/allconfig/load.go | 2 +- hugolib/page__content.go | 2 +- tpl/partials/partials.go | 65 ++++++++++--------- tpl/partials/partials_integration_test.go | 7 +- tpl/template.go | 1 + tpl/tplimpl/templatestore.go | 13 +++- tpl/tplimpl/templatestore_integration_test.go | 16 +++++ 7 files changed, 68 insertions(+), 38 deletions(-) diff --git a/config/allconfig/load.go b/config/allconfig/load.go index f224009ac..9289cf294 100644 --- a/config/allconfig/load.go +++ b/config/allconfig/load.go @@ -213,7 +213,7 @@ func (l configLoader) applyDefaultConfig() error { "disableAliases": false, "debug": false, "disableFastRender": false, - "timeout": "30s", + "timeout": "60s", "timeZone": "", "enableInlineShortcodes": false, } diff --git a/hugolib/page__content.go b/hugolib/page__content.go index 5f7d6f930..20abb7884 100644 --- a/hugolib/page__content.go +++ b/hugolib/page__content.go @@ -850,7 +850,7 @@ func (c *cachedContentScope) contentPlain(ctx context.Context) (contentPlainPlai }) if err != nil { if herrors.IsTimeoutError(err) { - err = fmt.Errorf("timed out rendering the page content. You may have a circular loop in a shortcode, or your site may have resources that take longer to build than the `timeout` limit in your Hugo config file: %w", err) + err = fmt.Errorf("timed out rendering the page content. Extend the `timeout` limit in your Hugo config file: %w", err) } return contentPlainPlainWords{}, err } diff --git a/tpl/partials/partials.go b/tpl/partials/partials.go index b9ef4b244..19882e36a 100644 --- a/tpl/partials/partials.go +++ b/tpl/partials/partials.go @@ -24,13 +24,13 @@ import ( "time" "github.com/bep/lazycache" - "github.com/gohugoio/hugo/common/constants" "github.com/gohugoio/hugo/common/hashing" "github.com/gohugoio/hugo/identity" + texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" "github.com/gohugoio/hugo/tpl" - texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" + "github.com/gohugoio/hugo/tpl/tplimpl" bp "github.com/gohugoio/hugo/bufferpool" "github.com/gohugoio/hugo/deps" @@ -109,7 +109,7 @@ func (c *contextWrapper) Set(in any) string { // A string if the partial is a text/template, or template.HTML when html/template. // Note that ctx is provided by Hugo, not the end user. func (ns *Namespace) Include(ctx context.Context, name string, contextList ...any) (any, error) { - res := ns.includWithTimeout(ctx, name, contextList...) + res := ns.include(ctx, name, contextList...) if res.err != nil { return nil, res.err } @@ -121,49 +121,36 @@ func (ns *Namespace) Include(ctx context.Context, name string, contextList ...an return res.result, nil } -func (ns *Namespace) includWithTimeout(ctx context.Context, name string, dataList ...any) includeResult { +func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) includeResult { + v, err := ns.lookup(name) + if err != nil { + return includeResult{err: err} + } + return ns.doInclude(ctx, v, dataList...) +} + +func (ns *Namespace) lookup(name string) (*tplimpl.TemplInfo, error) { if strings.HasPrefix(name, "partials/") { // This is most likely not what the user intended. // This worked before Hugo 0.146.0. ns.deps.Log.Warnidf(constants.WarnPartialSuperfluousPrefix, "Partial name %q starting with 'partials/' (as in {{ partial \"%s\"}}) is most likely not what you want. Before 0.146.0 we did a double lookup in this situation.", name, name) } - // Create a new context with a timeout not connected to the incoming context. - timeoutCtx, cancel := context.WithTimeout(context.Background(), ns.deps.Conf.Timeout()) - defer cancel() - - res := make(chan includeResult, 1) - - go func() { - res <- ns.include(ctx, name, dataList...) - }() - - select { - case r := <-res: - return r - case <-timeoutCtx.Done(): - err := timeoutCtx.Err() - if err == context.DeadlineExceeded { - //lint:ignore ST1005 end user message. - err = fmt.Errorf("partial %q timed out after %s. This is most likely due to infinite recursion. If this is just a slow template, you can try to increase the 'timeout' config setting.", name, ns.deps.Conf.Timeout()) - } - return includeResult{err: err} + v := ns.deps.TemplateStore.LookupPartial(name) + if v == nil { + return nil, fmt.Errorf("partial %q not found", name) } + return v, nil } // include is a helper function that lookups and executes the named partial. // Returns the final template name and the rendered output. -func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) includeResult { +func (ns *Namespace) doInclude(ctx context.Context, templ *tplimpl.TemplInfo, dataList ...any) includeResult { var data any if len(dataList) > 0 { data = dataList[0] } - v := ns.deps.TemplateStore.LookupPartial(name) - if v == nil { - return includeResult{err: fmt.Errorf("partial %q not found", name)} - } - templ := v - info := v.ParseInfo + info := templ.ParseInfo var w io.Writer @@ -212,6 +199,20 @@ func (ns *Namespace) IncludeCached(ctx context.Context, name string, context any Variants: variants, } depsManagerIn := tpl.Context.GetDependencyManagerInCurrentScope(ctx) + ti, err := ns.lookup(name) + if err != nil { + return nil, err + } + + if parent := tpl.Context.CurrentTemplate.Get(ctx); parent != nil { + for parent != nil { + if parent.CurrentTemplateInfoOps == ti { + // This will deadlock if we continue. + return nil, fmt.Errorf("circular call stack detected in partial %q", ti.Filename()) + } + parent = parent.Parent + } + } r, found, err := ns.cachedPartials.cache.GetOrCreate(key.Key(), func(string) (includeResult, error) { var depsManagerShared identity.Manager @@ -221,7 +222,7 @@ func (ns *Namespace) IncludeCached(ctx context.Context, name string, context any depsManagerShared = identity.NewManager("partials") ctx = tpl.Context.DependencyManagerScopedProvider.Set(ctx, depsManagerShared.(identity.DependencyManagerScopedProvider)) } - r := ns.includWithTimeout(ctx, key.Name, context) + r := ns.doInclude(ctx, ti, context) if ns.deps.Conf.Watching() { r.mangager = depsManagerShared } diff --git a/tpl/partials/partials_integration_test.go b/tpl/partials/partials_integration_test.go index 6fab3abd8..0fa47104d 100644 --- a/tpl/partials/partials_integration_test.go +++ b/tpl/partials/partials_integration_test.go @@ -256,7 +256,6 @@ func TestIncludeTimeout(t *testing.T) { files := ` -- config.toml -- baseURL = 'http://example.com/' -timeout = '200ms' -- layouts/index.html -- {{ partials.Include "foo.html" . }} -- layouts/partials/foo.html -- @@ -271,7 +270,7 @@ timeout = '200ms' ).BuildE() b.Assert(err, qt.Not(qt.IsNil)) - b.Assert(err.Error(), qt.Contains, "timed out") + b.Assert(err.Error(), qt.Contains, "maximum template call stack size exceeded") } func TestIncludeCachedTimeout(t *testing.T) { @@ -284,6 +283,8 @@ timeout = '200ms' -- layouts/index.html -- {{ partials.IncludeCached "foo.html" . }} -- layouts/partials/foo.html -- +{{ partialCached "bar.html" . }} +-- layouts/partials/bar.html -- {{ partialCached "foo.html" . }} ` @@ -295,7 +296,7 @@ timeout = '200ms' ).BuildE() b.Assert(err, qt.Not(qt.IsNil)) - b.Assert(err.Error(), qt.Contains, "timed out") + b.Assert(err.Error(), qt.Contains, `error calling partialCached: circular call stack detected in partial`) } // See Issue #10789 diff --git a/tpl/template.go b/tpl/template.go index f69ae2210..877422123 100644 --- a/tpl/template.go +++ b/tpl/template.go @@ -160,6 +160,7 @@ type CurrentTemplateInfoCommonOps interface { // CurrentTemplateInfo as returned in templates.Current. type CurrentTemplateInfo struct { Parent *CurrentTemplateInfo + Level int CurrentTemplateInfoOps } diff --git a/tpl/tplimpl/templatestore.go b/tpl/tplimpl/templatestore.go index 4266e274a..7770d053b 100644 --- a/tpl/tplimpl/templatestore.go +++ b/tpl/tplimpl/templatestore.go @@ -484,13 +484,24 @@ func (t *TemplateStore) ExecuteWithContext(ctx context.Context, ti *TemplInfo, w templ := ti.Template + parent := tpl.Context.CurrentTemplate.Get(ctx) + var level int + if parent != nil { + level = parent.Level + 1 + } currentTi := &tpl.CurrentTemplateInfo{ - Parent: tpl.Context.CurrentTemplate.Get(ctx), + Parent: parent, + Level: level, CurrentTemplateInfoOps: ti, } ctx = tpl.Context.CurrentTemplate.Set(ctx, currentTi) + const levelThreshold = 999 + if level > levelThreshold { + return fmt.Errorf("maximum template call stack size exceeded in %q", ti.Filename()) + } + if t.opts.Metrics != nil { defer t.opts.Metrics.MeasureSince(templ.Name(), time.Now()) } diff --git a/tpl/tplimpl/templatestore_integration_test.go b/tpl/tplimpl/templatestore_integration_test.go index e59dad33a..375813c31 100644 --- a/tpl/tplimpl/templatestore_integration_test.go +++ b/tpl/tplimpl/templatestore_integration_test.go @@ -1229,3 +1229,19 @@ layouts/list.html b.AssertFileContent("public/p1/index.html", "layouts/single.html") } } + +func TestTemplateLoop(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +-- layouts/_partials/p.html -- +p: {{ partial "p.html" . }} +-- layouts/all.html -- +{{ partial "p.html" . }} + +` + b, err := hugolib.TestE(t, files) + b.Assert(err, qt.IsNotNil) + b.Assert(err.Error(), qt.Contains, "error calling partial: maximum template call stack size exceeded") +}