-
Notifications
You must be signed in to change notification settings - Fork 656
fix: escape grave character in template literals #2867
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@marko/runtime-tags": patch | ||
--- | ||
|
||
Escape grave (`) characters in template literals |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"vars": { | ||
"props": {} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# Render | ||
```html | ||
<div> | ||
1` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# Render | ||
```html | ||
<div> | ||
1` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
``` | ||
|
||
# Mutations | ||
``` | ||
INSERT div | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export const $template = "<span>child`\"'</span><span>${value}</span>"; | ||
export const $walks = /* over(2) */"c"; | ||
export const $setup = () => {}; | ||
const value = "No!!"; | ||
import * as _ from "@marko/runtime-tags/debug/dom"; | ||
export default /* @__PURE__ */_._template("__tests__/tags/child.marko", $template, $walks, $setup); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
// size: 0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
export const $template = `<div><!>\` ${_child_template}</div>`; | ||
export const $walks = /* next(1), replace, over(2), beginChild, _child_walks, endChild, out(1) */`D%c/${_child_walks}&l`; | ||
import * as _ from "@marko/runtime-tags/debug/dom"; | ||
import { $setup as _child, $template as _child_template, $walks as _child_walks } from "./tags/child.marko"; | ||
const $count = /* @__PURE__ */_._let("count/2", ($scope, count) => _._text($scope["#text/0"], count)); | ||
export function $setup($scope) { | ||
_child($scope["#childScope/1"]); | ||
$count($scope, 1); | ||
} | ||
export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
const value = "No!!"; | ||
import * as _ from "@marko/runtime-tags/debug/html"; | ||
export default _._template("__tests__/tags/child.marko", input => { | ||
const $scope0_id = _._scope_id(); | ||
_._html("<span>child`\"'</span><span>${value}</span>"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import * as _ from "@marko/runtime-tags/debug/html"; | ||
import _child from "./tags/child.marko"; | ||
export default _._template("__tests__/template.marko", input => { | ||
const $scope0_id = _._scope_id(); | ||
let count = 1; | ||
_._html(`<div>${_._escape(count)}${_._el_resume($scope0_id, "#text/0")}\` `); | ||
_child({}); | ||
_._html("</div>"); | ||
_._scope($scope0_id, {}, "__tests__/template.marko", 0); | ||
_._resume_branch($scope0_id); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# Render | ||
```html | ||
<div> | ||
1` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Render | ||
```html | ||
<html> | ||
<head /> | ||
<body> | ||
<div> | ||
1 | ||
<!--M_*1 #text/0--> | ||
` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
<script> | ||
WALKER_RUNTIME("M")("_"); | ||
M._.r = [_ => (_.a = [0, | ||
{}])] | ||
</script> | ||
</body> | ||
</html> | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# Render End | ||
```html | ||
<div> | ||
1` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Write | ||
```html | ||
<div>1<!--M_*1 #text/0-->` <span>child`"'</span><span>${value}</span></div><script>WALKER_RUNTIME("M")("_");M._.r=[_=>(_.a=[0,{}])]</script> | ||
``` | ||
|
||
# Render End | ||
```html | ||
<html> | ||
<head /> | ||
<body> | ||
<div> | ||
1 | ||
<!--M_*1 #text/0--> | ||
` | ||
<span> | ||
child`"' | ||
</span> | ||
<span> | ||
${value} | ||
</span> | ||
</div> | ||
<script> | ||
WALKER_RUNTIME("M")("_"); | ||
M._.r = [_ => (_.a = [0, | ||
{}])] | ||
</script> | ||
</body> | ||
</html> | ||
``` | ||
|
||
# Mutations | ||
``` | ||
INSERT html | ||
INSERT html/head | ||
INSERT html/body | ||
INSERT html/body/div | ||
INSERT html/body/div/#text0 | ||
INSERT html/body/div/#comment | ||
INSERT html/body/div/#text1 | ||
INSERT html/body/div/span0 | ||
INSERT html/body/div/span0/#text | ||
INSERT html/body/div/span1 | ||
INSERT html/body/div/span1/#text | ||
INSERT html/body/script | ||
INSERT html/body/script/#text | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
static const value = "No!!" | ||
<span>child`"'</span> | ||
<span>\${value}</span> | ||
<script>"`"</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<let/count=1> | ||
<div> | ||
${count}` | ||
<child /> | ||
</div> |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -12,6 +12,7 @@ import { | |||||
|
||||||
import { getEventHandlerName, isEventHandler } from "../../common/helpers"; | ||||||
import { WalkCode } from "../../common/types"; | ||||||
import { bodyToTextLiteral } from "../util/body-to-text-literal"; | ||||||
import evaluate from "../util/evaluate"; | ||||||
import { generateUidIdentifier } from "../util/generate-uid"; | ||||||
import isInvokedFunction from "../util/is-invoked-function"; | ||||||
|
@@ -405,29 +406,15 @@ export default { | |||||
} | ||||||
} | ||||||
} else { | ||||||
const templateQuasis: t.TemplateElement[] = []; | ||||||
const templateExpressions: t.Expression[] = []; | ||||||
let currentQuasi = ""; | ||||||
let referencePlaceholder: t.MarkoPlaceholder | undefined; | ||||||
for (const child of tag.node.body.body) { | ||||||
if (t.isMarkoText(child)) { | ||||||
currentQuasi += child.value; | ||||||
} else if (t.isMarkoPlaceholder(child)) { | ||||||
referencePlaceholder ||= child; | ||||||
templateQuasis.push(t.templateElement({ raw: currentQuasi })); | ||||||
templateExpressions.push(child.value); | ||||||
currentQuasi = ""; | ||||||
} | ||||||
} | ||||||
const textLiteral = bodyToTextLiteral(tag.node.body); | ||||||
|
||||||
if (!referencePlaceholder) { | ||||||
write`${currentQuasi}`; | ||||||
if (t.isStringLiteral(textLiteral)) { | ||||||
write`${textLiteral}`; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Access When Apply this diff: - write`${textLiteral}`;
+ write`${textLiteral.value}`; 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||
} else { | ||||||
templateQuasis.push(t.templateElement({ raw: currentQuasi })); | ||||||
addStatement( | ||||||
"render", | ||||||
getSection(tag), | ||||||
referencePlaceholder.value.extra?.referencedBindings, | ||||||
textLiteral.extra?.referencedBindings, | ||||||
t.expressionStatement( | ||||||
callRuntime( | ||||||
"_text_content", | ||||||
|
@@ -436,7 +423,7 @@ export default { | |||||
getScopeAccessorLiteral(nodeBinding!), | ||||||
true, | ||||||
), | ||||||
t.templateLiteral(templateQuasis, templateExpressions), | ||||||
textLiteral, | ||||||
), | ||||||
), | ||||||
); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { types as t } from "@marko/compiler"; | ||
|
||
export function bodyToTextLiteral(body: t.MarkoTagBody) { | ||
const templateQuasis: t.TemplateElement[] = []; | ||
const templateExpressions: t.Expression[] = []; | ||
let currentQuasi = ""; | ||
let placeholderExtra: t.MarkoPlaceholder["extra"]; | ||
for (const child of body.body) { | ||
if (t.isMarkoText(child)) { | ||
currentQuasi += child.value; | ||
} else if (t.isMarkoPlaceholder(child)) { | ||
placeholderExtra ||= child.value.extra; | ||
templateQuasis.push(templateElement(currentQuasi, false)); | ||
templateExpressions.push(child.value); | ||
currentQuasi = ""; | ||
} | ||
} | ||
if (templateExpressions.length) { | ||
templateQuasis.push(templateElement(currentQuasi, true)); | ||
const literal = t.templateLiteral(templateQuasis, templateExpressions); | ||
literal.extra = placeholderExtra; | ||
return literal; | ||
} | ||
return t.stringLiteral(currentQuasi); | ||
} | ||
|
||
function templateElement(value: string, tail: boolean) { | ||
return t.templateElement( | ||
{ | ||
raw: value.replace(/`/g, "\\`"), | ||
|
||
cooked: value, | ||
}, | ||
tail, | ||
); | ||
} | ||
Comment on lines
+27
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete escaping for template literal quasis (risk of invalid/broken output). When building TemplateElement.raw, only backticks are escaped. You must also escape backslashes and the sequence "${" in quasis; otherwise text like "${x}" will be parsed as an expression, and backslashes may alter escapes. Apply: function templateElement(value: string, tail: boolean) {
return t.templateElement(
{
- raw: value.replace(/`/g, "\\`"),
+ raw: value
+ .replace(/\\/g, "\\\\") // escape backslash first
+ .replace(/`/g, "\\`") // escape backtick
+ .replace(/\$\{/g, "\\${"), // escape "${"
cooked: value,
},
tail,
);
} Also consider reusing a shared escape helper (eg, from normalize-string-expression) to avoid divergence. 🧰 Tools🪛 GitHub Check: CodeQL[failure] 30-30: Incomplete string escaping or encoding 🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -44,7 +44,7 @@ export default function normalizeStringExpression( | |||||||||||||
strs.push(curStr); | ||||||||||||||
|
||||||||||||||
return t.templateLiteral( | ||||||||||||||
strs.map((raw) => t.templateElement({ raw })), | ||||||||||||||
strs.map((raw) => t.templateElement({ raw: raw.replace(/`/g, "\\`") })), | ||||||||||||||
|
strs.map((raw) => t.templateElement({ raw: raw.replace(/`/g, "\\`") })), | |
strs.map((raw) => | |
t.templateElement({ | |
raw: raw.replace(/\\/g, "\\\\").replace(/`/g, "\\`"), | |
}) | |
), |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 47-47: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
🤖 Prompt for AI Agents
In packages/runtime-tags/src/translator/util/normalize-string-expression.ts
around line 47, the code only escapes backticks which leaves backslashes
unescaped and can produce invalid template raw values; update the transformation
to first escape backslashes and then escape backticks (e.g., apply
raw.replace(/\\/g, "\\\\").replace(/`/g, "\\`")) so backslashes are doubled
before backticks are escaped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Access
.value
property for StringLiteral nodes.When
textLiteral
is aStringLiteral
, you should access its.value
property to write the actual string content. This matches the correct pattern used inhtml-script.ts
at line 412.Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents