express-hbs 2.0.2 causes infinite loops and crashes Ghost

[allouis]

#10642 reverts the update to express 2.0.2 to avoid this bug.

Issue summary

When using the {{#get}} helper inside of a helper's template file, Ghost seems to end up in an infinite loop and crashes.

Technical details

In an effort to narrow the cause of the problem the following scenarios have been tested (✅=working, ❌=infinite loop):

• ✅ {{#get}} inside a regular partial
• ❌ {{#get}} inside the original navigation.hbs used by {{navigation}}
• ❌ {{#get}} inside of a theme's partials/navigation.hbs override
• ❌ {{#get}} inside the pagination.hbs file or a theme override

Our template-driven helpers all use templates.execute() which compiles a hbs template string if necessary then calls the resulting function. These are non-async helpers, whereas {{get}} is an async helper (returns promise).

I've generated a flamegraph which highlights the calls within express-hbs here and here that appear to be looping.

[allouis]

Update

This issue can now be traced back to here: https://github.com/barc/express-hbs/blob/v2.1.2/lib/hbs.js#L364-L366

In some circumstances, the value of this.resolverCache is not the same as context...resolverCache or options...resolverCache.

In this case, the promise will be added to a "resolverCache" which is not shared between helpers, so when express-hbs attempts to re-hydrate the template output with promise data, it will not be found - resulting in an endless loop when trying to replace the __aSyNc_ string.

Fix (possible)

This can be fixed by replacing the highlighted lines from above with:

    var resolverCache = _.get(context, 'data.root.resolverCache') ||
		_.get(options, 'data.root.resolverCache') ||
		this.resolverCache;

This changes forces express-hbs to prefer a resolverCache from either the context or the options, which reads from the root data - meaning that it is shared between all helpers when rendering a template. (I would like to know if it's possible for neither options nor context to have a resolverCache but this does - I suspect this is not a valid case)

Status

I have still not managed to replicate this outside of Ghost, but it seems to be occurring when a block helper is used like {{#helper "smth" key=val}} - Which causes the context to equal "smth" (An example of usage in Ghost is {{#get "posts" include="tags"}}

Handlebars documentation states that a helper will always be called with a this value of context - however when context is a string, this is not the case.

I'm going to continue to try to produce a breaking test for this, though if we can't, I would suggest moving forward with the patch detailed above.