Limit nested tags to 100 by default

[edgurgel]

We want to limit how deep a liquid template can get to just like the Liquid gem does: https://github.com/Shopify/liquid/blob/efef03d944157db323f1aed5e19861bf66fe256f/test/integration/security_test.rb#L82-L88

[bluzky]

@edgurgel do you have any idea on this?

When using render tag, it's hard to track nested tag. And we cannot use Context to carry nested tag level because some tag render does not pass context to inner scope.

Another thought is put nested level value in current process, but I think it's kind of hack

[edgurgel]

🤔 I'm wondering if something like this would work:

def eval(tag, context, options) do
   current_stack_level = context.stack_level

   context = %{context | stack_level: current_stack_level + 1}

    {result, new_context} = case do_eval(tag, context, options) do
      {text, context} -> 
        {text, context}

      text when is_binary(text) -> 
         {[text: text], context}

      text -> 
        {text, context}
    end

    new_context = %{new_context | stack_level: current_stack_level}

    {result, new_context}
end

And we just trust that tags are passing Context around?

If people add custom tags that don't pass the Context around then there is not much we can do to protect them?

And we change the render tag to pass the stack_level maybe through options? Solid.render(..., stack_level: context.stack_level)

What do you think?

The process solution could work if we maybe used the Process dictionary or something else like an ets table? But it feels hacky as you said 🤔

[Jcambass]

If people add custom tags that don't pass the Context around then there is not much we can do to protect them?

Can't the Solig.Tag behaviour be used to at least clarify this expectation?

[edgurgel]

@Jcambass yeah definitely worth documenting that!

[bluzky]

@edgurgel I think passing stack_level along with context is good idea. Regarding custom tag, user should protect themselves 😈 . We provide document as @Jcambass said and that's enough, I think.