Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper arguments starting with a number are split in two parts #450

Closed
jdesgats opened this issue Jun 30, 2021 · 2 comments · Fixed by #479
Closed

Helper arguments starting with a number are split in two parts #450

jdesgats opened this issue Jun 30, 2021 · 2 comments · Fixed by #479

Comments

@jdesgats
Copy link

When a helper parameter starts with a number, but contain something else after, it will be split at the first non-digit character.

For example {{helper 100_000}} will result in an agument sequence of [PathAndJson { relative_path: None, value: Constant(Number(100)) }, PathAndJson { relative_path: Some("_000"), value: Missing }]

I would expect one of:

  • String "100_000"
  • Number 100000
  • Syntax error

But truncating here seems like a bug.

Full example:

use std::collections::BTreeMap;
use handlebars::{Handlebars, RenderContext, Helper, Context, JsonRender, HelperResult, Output};

fn echo_helper (h: &Helper, _: &Handlebars, _: &Context, rc: &mut RenderContext, out: &mut dyn Output) -> HelperResult {
    println!("params={:?}", h.params());
    let param = h.param(0).unwrap();
    out.write(param.value().render().as_ref())?;
    Ok(())
}

fn main() {
  // create the handlebars registry
  let mut handlebars = Handlebars::new();
  handlebars.register_helper("echo", Box::new(echo_helper));

  // register the template. The template string will be verified and compiled.
  let source = "amount: {{echo 100_000}}";
  assert!(handlebars.register_template_string("t1", source).is_ok());

  // Prepare some data.
  //
  // The data type should implements `serde::Serialize`
  let data: BTreeMap<String, String> = BTreeMap::new();
  assert_eq!(handlebars.render("t1", &data).unwrap(), "amount: 100_000");
}

Output with handlebars-rust 4.0.1:

params=[PathAndJson { relative_path: None, value: Constant(Number(100)) }, PathAndJson { relative_path: Some("_000"), value: Missing }]
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"amount: 100"`,
 right: `"amount: 100_000"`', src/main.rs:24:3
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@jdesgats jdesgats changed the title Arguments starting with a number are split in two parts Helper arguments starting with a number are split in two parts Jun 30, 2021
@sunng87
Copy link
Owner

sunng87 commented Jul 2, 2021

it seems the underscore separator in number literal is a new feature introduced recently and is not supported by some json parsers and handlebarsjs language. We will hold the support for now.

However as you found out, there is issue with our parser when parsing parameters of helpers. This is also related to #443 . I will attempt to fix it in future.

@jdesgats
Copy link
Author

jdesgats commented Jul 5, 2021

Thank yo for your reply. To be clear, I am not expecting the number to be parsed correctly (although in my case of rendering TOML, it would make my life easier 😃). I managed to work around the problem by passing a constant string {{helper "1_000"}} and checking in the helper I have a string parameter (to catch cases where I forget quotes), it works well and I'm happy keeping it that way.

From what I tested with HandlebarsJS it would render the context variable if it exists, which is also what handlebars-rust would do without invoking a helper.

I tested the following template rendered with a context {"1_000": "value"}:

existing:     direct='{{1_000}}' echo='{{echo 1_000}}'
non existing: direct='{{2_000}}' echo='{{echo 2_000}}'

With the original JS handlebars, I get:

existing:     direct='value' echo='value'
non existing: direct='' echo=''

With handlebars-rust, I get:

existing:     direct='value' echo='1'
non existing: direct='' echo='2'

So I guess the right thing to in this case is to try to resolve the "number" as a variable name (even if having such variable names is a bit silly), and fail or not depending on the strict mode.

mkantor added a commit to mkantor/handlebars-rust that referenced this issue Dec 5, 2021
These test cases capture the issue called out in sunng87#450.

The handling of identifiers which start with numbers diverges from the
official JavaScript implementation of Handlebars. It's especially bad
for cases like `{{eq 1a}}`, which validly parses as `{{eq 1 a}}`!
mkantor added a commit to mkantor/handlebars-rust that referenced this issue Dec 5, 2021
mkantor added a commit to mkantor/handlebars-rust that referenced this issue Dec 5, 2021
These test cases capture the issue called out in sunng87#450.

The handling of identifiers which start with numbers diverges from the
official JavaScript implementation of Handlebars. It's especially bad
for cases like `{{eq 1a}}`, which validly parses as `{{eq 1 a}}`!
mkantor added a commit to mkantor/handlebars-rust that referenced this issue Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants