handlebars-lang / handlebars.js

Minimal templating on steroids.
http://handlebarsjs.com
MIT License
17.96k stars 2.04k forks source link

Fix #1748 where allowed prototype methods are not called #1958

Open aalimovs opened 1 year ago

aalimovs commented 1 year ago

Fixes #1748 where allowed prototype methods are not called.

Without this fix the official documentation is incorrect:

const template = handlebars.compile("{{aString.trim}}");
const result = template(
  { aString: "  abc  " },
  {
    allowedProtoMethods: {
      trim: true
    }
  }
);
// result = 'abc'

The result is [object Object], not abc.

jaylinski commented 1 year ago

Note: failing test was not caused by this PR and was fixed in #1963. Needs rebase.

aalimovs commented 1 year ago

@jaylinski rebased, but I've no idea why this PR now picks up changes in .github/workflows/ci.yml that are already in 4.x branch. I've tried rebasing the fork few different ways but still the same.

If you want me to recreate the PR let me know.

jaylinski commented 1 year ago

No problem, I fixed it.

mohd-akram commented 4 weeks ago

This fix isn't right - lookupProperty is only for looking up properties, not calling them. Functions are called at https://github.com/handlebars-lang/handlebars.js/blob/f422bfdf3ed321c32c6eecfaa778de46275fb76d/lib/handlebars/runtime.js#L146-L148

They are called with the Handlebars context, not with the object preceding the dot as in regular JavaScript. Changing this would be a breaking change, and it's not clear for what benefit. You can create a trim helper which achieves the same thing.

jaylinski commented 4 weeks ago

I'm still curious why the documentation states that it should work. :thinking:

mohd-akram commented 4 weeks ago

That was added somewhat recently to the documentation with the security fixes. I think it was just a mistake.

nknapp commented 3 weeks ago

I don't know why I added this example to the docs and did not notice that it does not work. The whole "allowProto..." options were just added to provide a way to get back the old behavior. If it didn't work in pre 4.6 versions (and it doesn't) , I wouldn't fix it.

The Handlebars way to solve the issue is to register a "trim" helper and use "{{trim aString}}" instead.

jaylinski commented 2 weeks ago

I'll keep this issue open until we corrected the wrong documentation and close it afterwards.