sunng87 / handlebars-rust

Rust templating with Handlebars
MIT License
1.28k stars 138 forks source link

Detecting stack overflows? #386

Open tmpfs opened 4 years ago

tmpfs commented 4 years ago

Hi @sunng87 , I saw this issue from a long time ago but it seems I am still able to create stack overflows pretty easily.

I can certainly create them with helpers that call out to render_template_with_context(). I think what would be useful would be to be able to pass an existing RenderContext into render_template_with_context() in order to be able to deal with this correctly. My helper could then add a stack to the RenderContext data to detect where it has already been called and respond appropriately with a RenderError.

I realize my use case is pretty esoteric (calling render_template_with_context inside a helper) but this is the only way I have been able to structure my program logic and interface exactly the way I want it.

Would you consider modifying render_template_with_context to accept an additional Option<RenderContext> argument? Looking at the source this should be a trivial change; is this something we could integrate for v4?

Thanks :+1:

sunng87 commented 4 years ago

hi @tmpfs , because RenderContext is an internal mutable data to maintain the state during rendering. Exposing it in a render method is breaking my design. Could you please show me your code that produces stack overflows maybe I can offer some help.

tmpfs commented 4 years ago

Hi @sunng87 thanks for taking the time to look into this. Here is a trivial program that will create a stack overflow, it is annotated with some notes of other avenues I have explored for solving the problem:

use handlebars::*;

// NOTE: My program is multi-threaded and if I store the render stack
// NOTE: in an Arc<Mutex<_>> it will slow down the parallel processing
// NOTE: due to contention on the Mutex lock.

#[derive(Clone)]
pub struct Block {}

impl HelperDef for Block {
    fn call<'reg: 'rc, 'rc>(
        // NOTE: Cannot store the call stack on the helper
        // NOTE: because this is not &mut
        &self,
        _h: &Helper<'reg, 'rc>,
        r: &'reg Handlebars<'_>,
        ctx: &'rc Context,
        rc: &mut RenderContext<'reg, 'rc>,
        out: &mut dyn Output,
    ) -> HelperResult {
        // NOTE: In my real program this is a file path and i load
        // NOTE: the file content and split out the template from
        // NOTE: the front matter. It is this file path that i want
        // NOTE: to keep track of in a `stack`.

        let dynamic_template = rc
            .evaluate(ctx, "@root/template")?
            .as_json()
            .as_str()
            .ok_or_else(|| RenderError::new("Type error for `template`, string expected"))?
            .to_string();

        let result = r
            .render_template_with_context(&dynamic_template, ctx)
            .map_err(|_| RenderError::new(format!("Render error {}", dynamic_template)))?;

        out.write(&result)?;
        Ok(())
    }
}

fn main() {
    let mut h = Handlebars::new();
    h.register_helper("block", Box::new(Block {}));

    // Just invoke the helper in our template
    h.register_template_string("main", "{{block}}").unwrap();

    // NOTE: This is an example *naughty* dynamic partial template
    // NOTE: that will create a stack overflow by calling the `block`
    // NOTE: helper
    let bad_data = serde_json::json!({"template": "{{block}}"});

    h.render("main", &bad_data).unwrap();
}

From my point of view the logical place to store this private call stack information would be on the RenderContext but only if I can pass it into the render for the dynamic template.

Thanks for taking a look :+1:

tmpfs commented 4 years ago

Furthermore while I was researching this I read that issue and noticed how you went about detecting stack overflows can easily be defeated with a single indirection. So I think we need to review how stack overflows are detected. Here is an example that will cause a stack overflow:

use handlebars::*;

fn main() {
    let mut h = Handlebars::new();
    h.register_partial("a", "{{> b}}").unwrap();
    h.register_partial("b", "{{> a}}").unwrap();
    h.register_template_string("main", "{{> a}}").unwrap();
    let data = serde_json::json!({});
    h.render("main", &data).unwrap();
}
thread 'main' has overflowed its stack
fatal runtime error: stack overflow
[1]    13987 abort      cargo run

But your stack overflow check will work correctly without the extra indirection:

use handlebars::*;

fn main() {
    let mut h = Handlebars::new();
    h.register_partial("a", "{{> a}}").unwrap();
    h.register_template_string("main", "{{> a}}").unwrap();
    let data = serde_json::json!({});
    h.render("main", &data).unwrap();
}

Yields:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: RenderError { desc: "Cannot include self in >", template_name: Some("a"), line_no: Some(1), column_no: Some(1), cause: None }', src/main.rs:8:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Happy to work with you to try to fix this if you like :+1:

sunng87 commented 4 years ago

Hello @tmpfs

For your first example, I will suggest you to render the dynamic template outside the helper if possible:

let dyn_tpl = data.get("template").as_str().unwrap();
let intermediate_result = hbs.render_template(dyn_tpl, data).unwrap();
let result = hbs.render_template("{{block}}", json!({"block": intermediate_result}).unwrap();

For the second case, a fix is welcomed!