Skip to content

Commit b496858

Browse files
authored
fix: improve html escaping of element attributes (#11411)
escape `<` because there's an edge case scenario where a script inside an attribute of a noscript is parsed differently
1 parent a038d49 commit b496858

File tree

13 files changed

+77
-94
lines changed

13 files changed

+77
-94
lines changed

.changeset/young-ads-roll.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: improve html escaping of element attributes

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,7 @@ import {
77
unwrap_optional
88
} from '../../../../utils/ast.js';
99
import { binding_properties } from '../../../bindings.js';
10-
import {
11-
clean_nodes,
12-
determine_namespace_for_children,
13-
escape_html,
14-
infer_namespace
15-
} from '../../utils.js';
10+
import { clean_nodes, determine_namespace_for_children, infer_namespace } from '../../utils.js';
1611
import { DOMProperties, PassiveEvents, VoidElements } from '../../../constants.js';
1712
import { is_custom_element_node, is_element_node } from '../../../nodes.js';
1813
import * as b from '../../../../utils/builders.js';
@@ -38,6 +33,7 @@ import {
3833
TRANSITION_IN,
3934
TRANSITION_OUT
4035
} from '../../../../../constants.js';
36+
import { escape_html } from '../../../../../escaping.js';
4137
import { regex_is_valid_identifier } from '../../../patterns.js';
4238
import { javascript_visitors_runes } from './javascript-runes.js';
4339
import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js';
@@ -1981,7 +1977,7 @@ export const template_visitors = {
19811977
` ${attribute.name}${
19821978
DOMBooleanAttributes.includes(name) && literal_value === true
19831979
? ''
1984-
: `="${literal_value === true ? '' : escape_html(String(literal_value), true)}"`
1980+
: `="${literal_value === true ? '' : escape_html(literal_value, true)}"`
19851981
}`
19861982
);
19871983
continue;

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,14 @@ import {
1717
import {
1818
clean_nodes,
1919
determine_namespace_for_children,
20-
escape_html,
2120
infer_namespace,
2221
transform_inspect_rune
2322
} from '../utils.js';
2423
import { create_attribute, is_custom_element_node, is_element_node } from '../../nodes.js';
2524
import { binding_properties } from '../../bindings.js';
2625
import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js';
27-
import {
28-
DOMBooleanAttributes,
29-
HYDRATION_END,
30-
HYDRATION_END_ELSE,
31-
HYDRATION_START
32-
} from '../../../../constants.js';
26+
import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../../constants.js';
27+
import { escape_html } from '../../../../escaping.js';
3328
import { sanitize_template_string } from '../../../utils/sanitize_template_string.js';
3429
import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js';
3530

@@ -1747,7 +1742,7 @@ const template_visitors = {
17471742
if (attribute.type === 'SpreadAttribute') {
17481743
spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
17491744
} else if (attribute.type === 'Attribute') {
1750-
const value = serialize_attribute_value(attribute.value, context);
1745+
const value = serialize_attribute_value(attribute.value, context, false, true);
17511746
if (attribute.name === 'name') {
17521747
expression = b.member(b.member_id('$$props.$$slots'), value, true, true);
17531748
} else if (attribute.name !== 'slot') {

packages/svelte/src/compiler/phases/3-transform/utils.js

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,55 +6,6 @@ import {
66
import * as b from '../../utils/builders.js';
77
import { walk } from 'zimmerframe';
88

9-
/**
10-
* @param {string} s
11-
* @param {boolean} [attr]
12-
*/
13-
export function escape_html(s, attr) {
14-
if (typeof s !== 'string') return s;
15-
const delimiter = attr ? '"' : '<';
16-
const escaped_delimiter = attr ? '&quot;' : '&lt;';
17-
let i_delimiter = s.indexOf(delimiter);
18-
let i_ampersand = s.indexOf('&');
19-
20-
if (i_delimiter < 0 && i_ampersand < 0) return s;
21-
22-
let left = 0,
23-
out = '';
24-
25-
while (i_delimiter >= 0 && i_ampersand >= 0) {
26-
if (i_delimiter < i_ampersand) {
27-
if (left < i_delimiter) out += s.substring(left, i_delimiter);
28-
out += escaped_delimiter;
29-
left = i_delimiter + 1;
30-
i_delimiter = s.indexOf(delimiter, left);
31-
} else {
32-
if (left < i_ampersand) out += s.substring(left, i_ampersand);
33-
out += '&amp;';
34-
left = i_ampersand + 1;
35-
i_ampersand = s.indexOf('&', left);
36-
}
37-
}
38-
39-
if (i_delimiter >= 0) {
40-
do {
41-
if (left < i_delimiter) out += s.substring(left, i_delimiter);
42-
out += escaped_delimiter;
43-
left = i_delimiter + 1;
44-
i_delimiter = s.indexOf(delimiter, left);
45-
} while (i_delimiter >= 0);
46-
} else if (!attr) {
47-
while (i_ampersand >= 0) {
48-
if (left < i_ampersand) out += s.substring(left, i_ampersand);
49-
out += '&amp;';
50-
left = i_ampersand + 1;
51-
i_ampersand = s.indexOf('&', left);
52-
}
53-
}
54-
55-
return left < s.length ? out + s.substring(left) : out;
56-
}
57-
589
/**
5910
* @param {import('estree').Node} node
6011
* @returns {boolean}

packages/svelte/src/escaping.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
const ATTR_REGEX = /[&"<]/g;
2+
const CONTENT_REGEX = /[&<]/g;
3+
4+
/**
5+
* @template V
6+
* @param {V} value
7+
* @param {boolean} [is_attr]
8+
*/
9+
export function escape_html(value, is_attr) {
10+
const str = String(value ?? '');
11+
12+
const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX;
13+
pattern.lastIndex = 0;
14+
15+
let escaped = '';
16+
let last = 0;
17+
18+
while (pattern.test(str)) {
19+
const i = pattern.lastIndex - 1;
20+
const ch = str[i];
21+
escaped += str.substring(last, i) + (ch === '&' ? '&amp;' : ch === '"' ? '&quot;' : '&lt;');
22+
last = i + 1;
23+
}
24+
25+
return escaped + str.substring(last);
26+
}

packages/svelte/src/internal/server/index.js

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
interactive_elements,
99
is_tag_valid_with_parent
1010
} from '../../constants.js';
11+
import { escape_html } from '../../escaping.js';
1112
import { DEV } from 'esm-env';
1213
import { current_component, pop, push } from './context.js';
1314
import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';
@@ -39,8 +40,6 @@ import { validate_store } from '../shared/validate.js';
3940
* }} Payload
4041
*/
4142

42-
const ATTR_REGEX = /[&"]/g;
43-
const CONTENT_REGEX = /[&<]/g;
4443
// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
4544
// https://infra.spec.whatwg.org/#noncharacter
4645
const INVALID_ATTR_NAME_CHAR_REGEX =
@@ -214,31 +213,6 @@ export function render(component, options) {
214213
};
215214
}
216215

217-
/**
218-
* @template V
219-
* @param {V} value
220-
* @param {any} is_attr
221-
* @returns {string}
222-
*/
223-
export function escape(value, is_attr = false) {
224-
const str = String(value ?? '');
225-
226-
const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX;
227-
pattern.lastIndex = 0;
228-
229-
let escaped = '';
230-
let last = 0;
231-
232-
while (pattern.test(str)) {
233-
const i = pattern.lastIndex - 1;
234-
const ch = str[i];
235-
escaped += str.substring(last, i) + (ch === '&' ? '&amp;' : ch === '"' ? '&quot;' : '&lt;');
236-
last = i + 1;
237-
}
238-
239-
return escaped + str.substring(last);
240-
}
241-
242216
/**
243217
* @param {Payload} payload
244218
* @param {(head_payload: Payload['head']) => void} fn
@@ -260,7 +234,7 @@ export function head(payload, fn) {
260234
*/
261235
export function attr(name, value, boolean) {
262236
if (value == null || (!value && boolean) || (value === '' && name === 'class')) return '';
263-
const assignment = boolean ? '' : `="${escape(value, true)}"`;
237+
const assignment = boolean ? '' : `="${escape_html(value, true)}"`;
264238
return ` ${name}${assignment}`;
265239
}
266240

@@ -381,7 +355,7 @@ export function stringify(value) {
381355
function style_object_to_string(style_object) {
382356
return Object.keys(style_object)
383357
.filter(/** @param {any} key */ (key) => style_object[key])
384-
.map(/** @param {any} key */ (key) => `${key}: ${escape(style_object[key], true)};`)
358+
.map(/** @param {any} key */ (key) => `${key}: ${escape_html(style_object[key], true)};`)
385359
.join(' ');
386360
}
387361

@@ -654,3 +628,5 @@ export {
654628
validate_snippet,
655629
validate_void_dynamic_element
656630
} from '../shared/validate.js';
631+
632+
export { escape_html as escape };
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
test({ assert, logs }) {
5+
assert.deepEqual(logs, []);
6+
}
7+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
const x = `</noscript><script>console.log('should not run')<` + `/script>`
3+
</script>
4+
5+
<noscript>
6+
<a href={x}>test</a>
7+
</noscript>
8+
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
test({ assert, target }) {
5+
assert.htmlEqual(target.innerHTML, '<div title="&amp;<">blah</div>');
6+
}
7+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<div title="&amp;&lt;">blah</div>

0 commit comments

Comments
 (0)