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

Suggestion: use property-branding to signify a SafeString #980

Open
callumlocke opened this issue Apr 6, 2017 · 4 comments
Open

Suggestion: use property-branding to signify a SafeString #980

callumlocke opened this issue Apr 6, 2017 · 4 comments

Comments

@callumlocke
Copy link

Currently, the Nunjucks runtime does val instanceof SafeString to check if a string is supposed to be safe.

This fails in the (admittedly unusual) situation where there are two independent copies of the Nunjucks runtime in play in the same process.

Another way would be to 'brand' all safe strings with a special non-enumerable property named something like _nunjucksSafeString, and always check for this property to see if a string is safe (instead of using instanceof, which is notoriously unreliable).

A simple boolean property is more robust and would work across VM boundaries. For example, the SafeString prototype could be modified as follows:

SafeString.prototype = Object.create(String.prototype, {
    length: { writable: true, configurable: true, value: 0 },
    _nunjucksSafeString: { value: true }
});
@ArmorDarks
Copy link

Sounds like good idea. But this repo is mostly dead, just for the note...

@niftylettuce
Copy link
Contributor

As I mentioned here #979 (comment) I believe this to be a v2 to v3 upgrade issue.

@niftylettuce
Copy link
Contributor

Also, if you set autoescape: false it fixes the issue.

@fdintino
Copy link
Collaborator

@callumlocke I'm fine with this change. Any chance you could open a pull request? I could copy your changes here but it would be nice to also have a changelog entry and unit test to go along with it.

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

No branches or pull requests

4 participants