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

runtime: use custom escape function #1753

Closed
wants to merge 1 commit into from
Closed

Conversation

Cireo
Copy link

@Cireo Cireo commented May 25, 2021

Without this change, the line

hb.escapeExpression = Utils.escapeExpression;

in top-level create() is unused.

Users who use a custom escape can only do so by modifying/corrupting
the global Utils.escapeExpression, or using deep copies.

Without this change, the line

    hb.escapeExpression = Utils.escapeExpression;

in top-level `create()` is unused.

Users who use a custom escape can only do so by modifying the global Utils.escapeExpression.
@Cireo
Copy link
Author

Cireo commented Nov 24, 2021

@jaylinski is there something I should have done to request a review of this?

@jaylinski
Copy link
Member

@Cireo It's okay, but we have to take a closer look at it.

Adding an automated test for this is probably hard, right?

@Cireo
Copy link
Author

Cireo commented Dec 1, 2021

No, it only takes a few lines to demonstrate the existing bug.

This doesn't add any new functionality, it repairs a slightly broken feature that was never properly implemented.

> const clobberEscape = (val) => "escaped"
> const test = (hb) => hb.compile("x {{y}} z")({y: "y"})

> const Handlebars = require('handlebars');

// Pass in escapeExpression
> const hb1 = Handlebars.create({escapeExpression: clobberEscape});
> test(hb1)
'x y z'  // BUG: expect "x escaped z"

> const hb2 = Handlebars.create()  // workaround by modifying global escape
> hb2.Utils.escapeExpression = clobberEscape;
> test(hb2)
'x escaped z'

@Cireo
Copy link
Author

Cireo commented Dec 1, 2021

It's been months since I did the initial debugging, so I don't recall if you're supposed to be able to pass in the escapeExpression to create or what the exact syntax is.

@jaylinski jaylinski added bug and removed unverified labels Dec 1, 2021
@jaylinski
Copy link
Member

Duplicate of #1523.

@jaylinski jaylinski closed this Dec 23, 2021
@jaylinski jaylinski removed the bug label Dec 23, 2021
@Cireo Cireo deleted the 4.x branch December 28, 2021 00:05
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 this pull request may close these issues.

None yet

2 participants