Orchardcore: Liquid Template: Exception of type 'System.StackOverflowException' was thrown.

[ns8482e]

Liquid Templates throws 'System.StackOverflowException' and application crashes.

{{ Model | shape_render }}
should not be allowed within liquid template and should not be saved.

As OC shape that defines above syntax tries to call itself.

[hishamco]

@jptissot I think we could eliminate this to avoid crash the app

[jptissot]

Thing is, you might want to run {{ Model | shape_render }} after modifying the shape metadata (shape morphing)
Maybe we should have a MaxStackDepth setting somewhere that could detect this and at least not crash the application?

[jptissot]

Another option maybe would be to check the current shape and target shape in the shape_render filter and throw an error if the shapes are equivalent. However, I am not sure if we have access to the current shape in a filter (maybe via context ?)

[hishamco]

I like the second approach

[sebastienros]

There is already a feature like this in Fluid. Maybe I missed this case.

[jtkech]

Hmm, most of the time the current model in registered in the ambient values, so if it is a shape we could check its alternates / bindings, maybe there is a way to avoid it to render itself, i'm finishing something then i will take a look.

[sebastienros]

As @jptissot mentioned sometimes we really want to call render on the current shape, again. Which is why I think this is an issue in Fluid instead.

[jtkech]

Yes, when we do morphing, oh yes and then it will also change the registered model (same reference), so we would need a flag saying that it has been morphed allowing to be rendered again.

Just an idea, but okay right now i let it here as it is.

[jtkech]

I tried different workarounds but more or less hacky ;)

One solution that works is to add a Recursion integer to the ShapeMetadata and to do the following in the liquid ShapeRender (maybe also in ShapeStringify).

Note: In the blog theme, it works with maxRecursion = 2 without breaking the menu because each menu item morphs itself once. So, maybe a quite low value without having to make it configurable.

var model = context.LocalScope.GetValue("Model")?.ToObjectValue();
if (model == shape)
{
    shape.Metadata.Recursion++;

    if (shape.Metadata.Recursion > maxRecursion)
    {
        return ThrowArgumentException<FluidValue>(
            "A shape is rendering itself too many time while invoking 'shape_render'");
    }
}

Maybe we could do this at a lower level, e.g in ShapeExecuteAsync so that it would also work for any kind of shapes, e.g razor shapes.

[agriffard]

Fixed by #4394

[jtkech]

The little "problem" is that #4394 has never been merged ;)

But if it may be useful i could reopen it, retry it and merge it

[sebastienros]

Maybe a scoped counter in the liquid template manager render method would be easier to implement.

[jtkech]

Okay i'll take a look

[jtkech]

Hmm, what i did in #4394 is not so complex

When we render a liquid file we don't pass through the liquid template manager

[Piedone]

Shouldn't we consider this P1? Being able to bring the app down with a user feature is pretty serious.

[sebastienros]

@Piedone for RC2 or 1.0 ;)

[Piedone]

I'm glad we agree :D.