fix(index): escape double quotes correctly (options.interpolate)
#154
Conversation
|
|
Codecov Report
@@ Coverage Diff @@
## master #154 +/- ##
==========================================
+ Coverage 96.96% 97.02% +0.06%
==========================================
Files 2 2
Lines 99 101 +2
Branches 20 20
==========================================
+ Hits 96 98 +2
Misses 3 3
Continue to review full report at Codecov.
|
joshwiens
changed the title
| @@ -129,6 +129,9 @@ module.exports = function(content) { | |||
| } | |||
|
|
|||
| if(config.interpolate && config.interpolate !== 'require') { | |||
| // Double escape quotes so that they are not unescaped completely in the template string | |||
| content = content.replace(/\\"/g, "\\\\\""); | |||
There was a problem hiding this comment.
\\\\\('|") (5) => \\\\('|") (4) ?
There was a problem hiding this comment.
I'm not sure I understand.
The two regexes could be combined into one .replace(/\\("|')/g, "\\\\$1") or what do you mean?
| @@ -71,6 +71,11 @@ describe("loader", function() { | |||
| 'module.exports = "<!-- comment --><h3 customattr=\\"\\">#{number} {customer}</h3><p>{title}</p><!-- comment --><img src=\" + require("./image.png") + \" />";' | |||
| ); | |||
| }); | |||
| it("should preserve escaped quotes", function() { | |||
| loader.call({}, '<script>{"json": "with \\"quotes\\" in value"}</script>').should.be.eql( | |||
| 'module.exports = "<script>{\\\"json\\\": \\\"with \\\\\\\"quotes\\\\\\\" in value\\\"}</script>";' | |||
There was a problem hiding this comment.
This only occurs for { key: "bla \"blub\" bla" }, but doesn't on { key: "bla 'blub' bla" } right ?
There was a problem hiding this comment.
Hi Michael,
sorry for the late reply, but yes: that is correct. I've updated my minimal test case with two more examples: https://github.com/benurb/html-loader-escape-bug
michael-ciniawsky
changed the title
options.interpolate)
|
Is there still anything I can do? @michael-ciniawsky |
|
@benurb - Everyone is a bit buried getting ready for |
michael-ciniawsky
changed the title
options.interpolate)options.interpolate)
michael-ciniawsky
added
status: Approved
and removed
priority: 4 (important)
pr_action: Review
severity: 3 (broken)
labels
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
Currently escaped quotes in inline scripts are unescaped when interpolation is enabled. You can find a minimal test case here: https://github.com/benurb/html-loader-escape-bug
What is the new behavior?
interpolate=true behaves like disabled interpolation and quotes are still escaped.
Does this PR introduce a breaking change?
The documentation states no difference between interpolate=true and false regarding the handling of escaped quotes.
If this PR contains a breaking change, please describe the following...
Other information:
This solution is very hacky, but I couldn't find a cleaner one 😞
Without these changes the second of the two added tests fails.
This PR might also be a fix for #153 though I don't use angular and haven't tested it.