Skip to content

Commit

Permalink
Make sure to also escape quotes
Browse files Browse the repository at this point in the history 
Imagine a case where a stringified json is inlined as a data attribute, in this case the " must be escaped, too, in order to maintain valid html
  • Loading branch information
@timse
timse authored Sep 18, 2019
1 parent 47d5a98 commit 1cbea0f
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/compile/references.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ module.exports = function(Velocity, utils) {
if (c === '&') {
cstr = "&"
escape = true
} else if (c === '"') {
cstr = """
escape = true
} else if (c === '<') {
cstr = "&lt;"
escape = true
Expand Down

4 comments on commit 1cbea0f

@rudnitskih
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timse @shepherdwind Hey, just let you know that this commit broke us:

We have the following index.vm:

<body>
  <script>
     window.foo = ${foo};
  </script>
</body>

And the following data.mock.json:

{
  "foo": "{\"prop\": \"value\"}"
}

Before this commit it worked fine, but after it started to be escaped.

In our case we just disabled escaping. But it can brake other users.
Thanks for your contribution, guys!

@shepherdwind
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you feedback. I rollback the latest version to 1.1.4 now.

This change is break change, I should publish a new major version.

@shepherdwind
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.0.0 version published, https://github.com/shepherdwind/velocity.js/blob/master/History.md#200-2019-09-26

@timse You can upgrade to the new version, and set escape option to true. https://github.com/shepherdwind/velocity.js/blob/master/index.d.ts#L39

@rudnitskih
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shepherdwind Thank you man! You rock 🤘

Please sign in to comment.