From 43611602f049e3920223c6575adeaf4902f0b88f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 1 May 2024 16:24:02 +0100 Subject: [PATCH 1/4] fix: improve html escaping of element attributes --- .changeset/young-ads-roll.md | 5 ++ .../3-transform/client/visitors/template.js | 12 ++--- .../3-transform/server/transform-server.js | 5 +- .../src/compiler/phases/3-transform/utils.js | 49 ------------------- packages/svelte/src/constants.js | 27 ++++++++++ packages/svelte/src/internal/server/index.js | 36 +++----------- .../samples/escaped-attr-2/_config.js | 7 +++ .../samples/escaped-attr-2/main.svelte | 8 +++ .../samples/escaped-attr/_config.js | 7 +++ .../samples/escaped-attr/main.svelte | 4 ++ playgrounds/sandbox/run.js | 2 +- 11 files changed, 71 insertions(+), 91 deletions(-) create mode 100644 .changeset/young-ads-roll.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/main.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr/main.svelte diff --git a/.changeset/young-ads-roll.md b/.changeset/young-ads-roll.md new file mode 100644 index 000000000000..51b7f5b4614a --- /dev/null +++ b/.changeset/young-ads-roll.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: improve html escaping of element attributes diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index c508414f72ce..7233da826992 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -7,12 +7,7 @@ import { unwrap_optional } from '../../../../utils/ast.js'; import { binding_properties } from '../../../bindings.js'; -import { - clean_nodes, - determine_namespace_for_children, - escape_html, - infer_namespace -} from '../../utils.js'; +import { clean_nodes, determine_namespace_for_children, infer_namespace } from '../../utils.js'; import { DOMProperties, PassiveEvents, VoidElements } from '../../../constants.js'; import { is_custom_element_node, is_element_node } from '../../../nodes.js'; import * as b from '../../../../utils/builders.js'; @@ -36,7 +31,8 @@ import { TEMPLATE_USE_IMPORT_NODE, TRANSITION_GLOBAL, TRANSITION_IN, - TRANSITION_OUT + TRANSITION_OUT, + escape_html } from '../../../../../constants.js'; import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; @@ -1984,7 +1980,7 @@ export const template_visitors = { ` ${attribute.name}${ DOMBooleanAttributes.includes(name) && literal_value === true ? '' - : `="${literal_value === true ? '' : escape_html(String(literal_value), true)}"` + : `="${literal_value === true ? '' : escape_html(literal_value, true)}"` }` ); continue; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 0a96310bacf3..6d88acb2f6c7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -17,7 +17,6 @@ import { import { clean_nodes, determine_namespace_for_children, - escape_html, infer_namespace, transform_inspect_rune } from '../utils.js'; @@ -27,8 +26,8 @@ import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patte import { DOMBooleanAttributes, HYDRATION_END, - HYDRATION_END_ELSE, - HYDRATION_START + HYDRATION_START, + escape_html } from '../../../../constants.js'; import { sanitize_template_string } from '../../../utils/sanitize_template_string.js'; import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js'; diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index f7c26eabc950..b7f54a63e23b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -6,55 +6,6 @@ import { import * as b from '../../utils/builders.js'; import { walk } from 'zimmerframe'; -/** - * @param {string} s - * @param {boolean} [attr] - */ -export function escape_html(s, attr) { - if (typeof s !== 'string') return s; - const delimiter = attr ? '"' : '<'; - const escaped_delimiter = attr ? '"' : '<'; - let i_delimiter = s.indexOf(delimiter); - let i_ampersand = s.indexOf('&'); - - if (i_delimiter < 0 && i_ampersand < 0) return s; - - let left = 0, - out = ''; - - while (i_delimiter >= 0 && i_ampersand >= 0) { - if (i_delimiter < i_ampersand) { - if (left < i_delimiter) out += s.substring(left, i_delimiter); - out += escaped_delimiter; - left = i_delimiter + 1; - i_delimiter = s.indexOf(delimiter, left); - } else { - if (left < i_ampersand) out += s.substring(left, i_ampersand); - out += '&'; - left = i_ampersand + 1; - i_ampersand = s.indexOf('&', left); - } - } - - if (i_delimiter >= 0) { - do { - if (left < i_delimiter) out += s.substring(left, i_delimiter); - out += escaped_delimiter; - left = i_delimiter + 1; - i_delimiter = s.indexOf(delimiter, left); - } while (i_delimiter >= 0); - } else if (!attr) { - while (i_ampersand >= 0) { - if (left < i_ampersand) out += s.substring(left, i_ampersand); - out += '&'; - left = i_ampersand + 1; - i_ampersand = s.indexOf('&', left); - } - } - - return left < s.length ? out + s.substring(left) : out; -} - /** * @param {import('estree').Node} node * @returns {boolean} diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index d9fc86d60038..12c768f3ab76 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -273,3 +273,30 @@ export function is_tag_valid_with_parent(tag, parent_tag) { return true; } + +const ATTR_REGEX = /["<]/g; +const CONTENT_REGEX = /[&<]/g; + +/** + * @template V + * @param {V} value + * @param {boolean} [is_attr] + */ +export function escape_html(value, is_attr) { + const str = String(value ?? ''); + + const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX; + pattern.lastIndex = 0; + + let escaped = ''; + let last = 0; + + while (pattern.test(str)) { + const i = pattern.lastIndex - 1; + const ch = str[i]; + escaped += str.substring(last, i) + (ch === '&' ? '&' : ch === '"' ? '"' : '<'); + last = i + 1; + } + + return escaped + str.substring(last); +} diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index f6e68acb1c4f..339f3bf2335f 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -6,7 +6,8 @@ import { RawTextElements, disallowed_paragraph_contents, interactive_elements, - is_tag_valid_with_parent + is_tag_valid_with_parent, + escape_html } from '../../constants.js'; import { DEV } from 'esm-env'; import { current_component, pop, push } from './context.js'; @@ -39,8 +40,6 @@ import { validate_store } from '../shared/validate.js'; * }} Payload */ -const ATTR_REGEX = /[&"]/g; -const CONTENT_REGEX = /[&<]/g; // https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 // https://infra.spec.whatwg.org/#noncharacter const INVALID_ATTR_NAME_CHAR_REGEX = @@ -214,31 +213,6 @@ export function render(component, options) { }; } -/** - * @template V - * @param {V} value - * @param {any} is_attr - * @returns {string} - */ -export function escape(value, is_attr = false) { - const str = String(value ?? ''); - - const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX; - pattern.lastIndex = 0; - - let escaped = ''; - let last = 0; - - while (pattern.test(str)) { - const i = pattern.lastIndex - 1; - const ch = str[i]; - escaped += str.substring(last, i) + (ch === '&' ? '&' : ch === '"' ? '"' : '<'); - last = i + 1; - } - - return escaped + str.substring(last); -} - /** * @param {Payload} payload * @param {(head_payload: Payload['head']) => void} fn @@ -260,7 +234,7 @@ export function head(payload, fn) { */ export function attr(name, value, boolean) { if (value == null || (!value && boolean) || (value === '' && name === 'class')) return ''; - const assignment = boolean ? '' : `="${escape(value, true)}"`; + const assignment = boolean ? '' : `="${escape_html(value, true)}"`; return ` ${name}${assignment}`; } @@ -381,7 +355,7 @@ export function stringify(value) { function style_object_to_string(style_object) { return Object.keys(style_object) .filter(/** @param {any} key */ (key) => style_object[key]) - .map(/** @param {any} key */ (key) => `${key}: ${escape(style_object[key], true)};`) + .map(/** @param {any} key */ (key) => `${key}: ${escape_html(style_object[key], true)};`) .join(' '); } @@ -654,3 +628,5 @@ export { validate_snippet, validate_void_dynamic_element } from '../shared/validate.js'; + +export { escape_html as escape }; diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/_config.js b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/_config.js new file mode 100644 index 000000000000..4841323e4e40 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, []); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/main.svelte b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/main.svelte new file mode 100644 index 000000000000..ec1ecfcac58d --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-2/main.svelte @@ -0,0 +1,8 @@ + + + + diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr/_config.js b/packages/svelte/tests/runtime-legacy/samples/escaped-attr/_config.js new file mode 100644 index 000000000000..4841323e4e40 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, []); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr/main.svelte b/packages/svelte/tests/runtime-legacy/samples/escaped-attr/main.svelte new file mode 100644 index 000000000000..d4852b90c7df --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr/main.svelte @@ -0,0 +1,4 @@ + + diff --git a/playgrounds/sandbox/run.js b/playgrounds/sandbox/run.js index a6589e993d0e..06f4242ae8a5 100644 --- a/playgrounds/sandbox/run.js +++ b/playgrounds/sandbox/run.js @@ -50,7 +50,7 @@ for (const generate of ['client', 'server']) { try { const migrated = migrate(source); - fs.writeFileSync(`${cwd}/output/${file}.migrated.svelte`, migrated); + fs.writeFileSync(`${cwd}/output/${file}.migrated.svelte`, migrated.code); } catch (e) { console.warn(`Error migrating ${file}`, e); } From a061e1f067432ba1adc5c10d0567b22e50833fde Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 1 May 2024 16:57:39 +0100 Subject: [PATCH 2/4] move function --- .../3-transform/client/visitors/template.js | 4 +-- .../3-transform/server/transform-server.js | 8 ++---- packages/svelte/src/constants.js | 27 ------------------- packages/svelte/src/escaping.js | 26 ++++++++++++++++++ packages/svelte/src/internal/server/index.js | 4 +-- 5 files changed, 32 insertions(+), 37 deletions(-) create mode 100644 packages/svelte/src/escaping.js diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 7233da826992..e728e5d4176c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -31,9 +31,9 @@ import { TEMPLATE_USE_IMPORT_NODE, TRANSITION_GLOBAL, TRANSITION_IN, - TRANSITION_OUT, - escape_html + TRANSITION_OUT } from '../../../../../constants.js'; +import { escape_html } from '../../../../../escaping.js'; import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js'; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 6d88acb2f6c7..cc92b0d65aa9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -23,12 +23,8 @@ import { import { create_attribute, is_custom_element_node, is_element_node } from '../../nodes.js'; import { binding_properties } from '../../bindings.js'; import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js'; -import { - DOMBooleanAttributes, - HYDRATION_END, - HYDRATION_START, - escape_html -} from '../../../../constants.js'; +import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../../constants.js'; +import { escape_html } from '../../../../escaping.js'; import { sanitize_template_string } from '../../../utils/sanitize_template_string.js'; import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js'; diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 12c768f3ab76..d9fc86d60038 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -273,30 +273,3 @@ export function is_tag_valid_with_parent(tag, parent_tag) { return true; } - -const ATTR_REGEX = /["<]/g; -const CONTENT_REGEX = /[&<]/g; - -/** - * @template V - * @param {V} value - * @param {boolean} [is_attr] - */ -export function escape_html(value, is_attr) { - const str = String(value ?? ''); - - const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX; - pattern.lastIndex = 0; - - let escaped = ''; - let last = 0; - - while (pattern.test(str)) { - const i = pattern.lastIndex - 1; - const ch = str[i]; - escaped += str.substring(last, i) + (ch === '&' ? '&' : ch === '"' ? '"' : '<'); - last = i + 1; - } - - return escaped + str.substring(last); -} diff --git a/packages/svelte/src/escaping.js b/packages/svelte/src/escaping.js new file mode 100644 index 000000000000..cc3777585e52 --- /dev/null +++ b/packages/svelte/src/escaping.js @@ -0,0 +1,26 @@ +const ATTR_REGEX = /["<]/g; +const CONTENT_REGEX = /[&<]/g; + +/** + * @template V + * @param {V} value + * @param {boolean} [is_attr] + */ +export function escape_html(value, is_attr) { + const str = String(value ?? ''); + + const pattern = is_attr ? ATTR_REGEX : CONTENT_REGEX; + pattern.lastIndex = 0; + + let escaped = ''; + let last = 0; + + while (pattern.test(str)) { + const i = pattern.lastIndex - 1; + const ch = str[i]; + escaped += str.substring(last, i) + (ch === '&' ? '&' : ch === '"' ? '"' : '<'); + last = i + 1; + } + + return escaped + str.substring(last); +} diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 339f3bf2335f..6c2a2b57919c 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -6,9 +6,9 @@ import { RawTextElements, disallowed_paragraph_contents, interactive_elements, - is_tag_valid_with_parent, - escape_html + is_tag_valid_with_parent } from '../../constants.js'; +import { escape_html } from '../../escaping.js'; import { DEV } from 'esm-env'; import { current_component, pop, push } from './context.js'; import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js'; From 7053cb063d565932f87961770047f84b430a491d Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 1 May 2024 18:41:04 +0100 Subject: [PATCH 3/4] fix escaping bug --- .../src/compiler/phases/3-transform/server/transform-server.js | 2 +- packages/svelte/src/escaping.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index cc92b0d65aa9..12e02eb1d4a5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -1742,7 +1742,7 @@ const template_visitors = { if (attribute.type === 'SpreadAttribute') { spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); } else if (attribute.type === 'Attribute') { - const value = serialize_attribute_value(attribute.value, context); + const value = serialize_attribute_value(attribute.value, context, false, true); if (attribute.name === 'name') { expression = b.member(b.member_id('$$props.$$slots'), value, true, true); } else if (attribute.name !== 'slot') { diff --git a/packages/svelte/src/escaping.js b/packages/svelte/src/escaping.js index cc3777585e52..56050e40625d 100644 --- a/packages/svelte/src/escaping.js +++ b/packages/svelte/src/escaping.js @@ -1,4 +1,4 @@ -const ATTR_REGEX = /["<]/g; +const ATTR_REGEX = /[&"<]/g; const CONTENT_REGEX = /[&<]/g; /** From 08c2c253dc851928279a3e35942f8fec118fe191 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 1 May 2024 20:01:42 +0100 Subject: [PATCH 4/4] add test --- .../tests/runtime-legacy/samples/escaped-attr-3/_config.js | 7 +++++++ .../runtime-legacy/samples/escaped-attr-3/main.svelte | 1 + 2 files changed, 8 insertions(+) create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/main.svelte diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/_config.js b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/_config.js new file mode 100644 index 000000000000..ab03ae56d2f7 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + assert.htmlEqual(target.innerHTML, '
blah
'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/main.svelte b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/main.svelte new file mode 100644 index 000000000000..ff33fbf59a73 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/escaped-attr-3/main.svelte @@ -0,0 +1 @@ +
blah