cobalt-org / liquid-rust

Liquid templating for Rust
docs.rs/liquid
MIT License
466 stars 78 forks source link

Ensure infinitely recursively includes error out instead of crash #300

Open epage opened 5 years ago

epage commented 5 years ago

Example test case:

#[test]
#[should_panic]
fn test_recursively_included_template_does_not_produce_endless_loop() {
    panic!("We don't check recursion depth");
    /*
    let parser = liquid::ParserBuilder::with_liquid().include_source(Box::new(InfiniteFileSystem)).build();
    parser.parse("{% include 'loop' %}").unwrap();
    */
}

We probably want Parser to have an Option<number> that gets passed to the context. The context would then track the depth and error appropriately.

Resources

epage commented 5 years ago

Note: to get the max recursion from the end-client to the linked Context, it'll need to pass through the following layers

epage commented 5 years ago

Max recursion:

veatnik commented 5 years ago

The bug marks "fn run_in_named_scope()" as being the location to fix. Wouldn't the recursion in "fn run_in_scope()" also be at risk for infinite recursion? Also looking at the function passed in to run_in_named_scope , " | mut scope | " , I'm not sure I see where that closure calls run_in_named_scope again. Any suggestions for understanding how scope and new_scope work? I will just proceed and test based on our conversation, perhaps I will gain an understanding of the rest as I fix various bugs.

epage commented 5 years ago

The bug marks "fn run_in_named_scope()" as being the location to fix. Wouldn't the recursion in "fn run_in_scope()" also be at risk for infinite recursion?

Good question. This requires some background I didn't think to include in the issue.

The difference between the two is use case.

? Also looking at the function passed in to run_in_named_scope , " | mut scope | " , I'm not sure I see where that closure calls run_in_named_scope again. Any suggestions for understanding how scope and new_scope work?

It isn't as obvious because it is hidden behind trait-functions.

If you look at Include::render_to, you'll see that inside of render_to we call run_in_named_scope which calls an included-file's render_to.

ryanyz10 commented 5 years ago

I'd like to try and tackle this one too, if @veatnik isn't working on it anymore.