Skip to content

Commit b06eabf

Browse files
authored
Consolidate inline script escaping into shared utility (#16303)
* Consolidate inline script escaping into shared stringifyForScript utility Unify the two separate script-embedding escape approaches (defineScriptVars and safeJsonStringify) into a single stringifyForScript function in escape.ts. Uses a comprehensive < escape strategy rather than pattern-matching specific tag sequences, covering all ETAGO variants in one pass. * Add changeset * Update changeset description * Fix stringifyForScript to handle undefined values * Update astro-directives test assertion for new escape sequence
1 parent 7fe40bc commit b06eabf

6 files changed

Lines changed: 49 additions & 25 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Improves handling of special characters in inline `<script>` content

packages/astro/src/runtime/server/escape.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@ import { streamAsyncIterator } from './util.js';
44
// Leverage the battle-tested `html-escaper` npm package.
55
export const escapeHTML = escape;
66

7+
/**
8+
* Serializes a value to a JSON string that is safe to embed inside a `<script>` tag.
9+
* All `<` characters are escaped to `\u003c` so the browser's HTML parser cannot be
10+
* tricked into closing the script block early via `</script>` variants (case-insensitive,
11+
* whitespace, or self-closing forms) or `<!--` comment injection.
12+
* @see https://mathiasbynens.be/notes/etago
13+
*/
14+
export function stringifyForScript(value: any): string {
15+
return JSON.stringify(value)?.replace(/</g, '\\u003c');
16+
}
17+
718
export class HTMLBytes extends Uint8Array {}
819

920
// TypeScript won't let us define this in the class body so have to do it

packages/astro/src/runtime/server/render/server-islands.ts

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { encryptString, generateCspDigest } from '../../../core/encryption.js';
22
import type { SSRResult } from '../../../types/public/internal.js';
3-
import { markHTMLString } from '../escape.js';
3+
import { markHTMLString, stringifyForScript } from '../escape.js';
44
import { renderChild } from './any.js';
55
import { createThinHead, type ThinHead } from './astro/head-and-content.js';
66
import type { RenderDestination } from './common.js';
@@ -18,20 +18,7 @@ export function containsServerDirective(props: Record<string | number, any>) {
1818
return 'server:component-directive' in props;
1919
}
2020

21-
const SCRIPT_RE = /<\/script/giu;
22-
const COMMENT_RE = /<!--/gu;
23-
const SCRIPT_REPLACER = '<\\/script';
24-
const COMMENT_REPLACER = '\\u003C!--';
2521

26-
/**
27-
* Encodes the script end-tag open (ETAGO) delimiter and opening HTML comment syntax for JSON inside a `<script>` tag.
28-
* @see https://mathiasbynens.be/notes/etago
29-
*/
30-
function safeJsonStringify(obj: any) {
31-
return JSON.stringify(obj)
32-
.replace(SCRIPT_RE, SCRIPT_REPLACER)
33-
.replace(COMMENT_RE, COMMENT_REPLACER);
34-
}
3522

3623
function createSearchParams(
3724
encryptedComponentExport: string,
@@ -214,17 +201,17 @@ export class ServerIslandComponent {
214201

215202
// Get adapter headers for inline script
216203
const adapterHeaders = this.result.internalFetchHeaders || {};
217-
const headersJson = safeJsonStringify(adapterHeaders);
204+
const headersJson = stringifyForScript(adapterHeaders);
218205

219206
const method = useGETRequest
220207
? // GET request
221208
`const headers = new Headers(${headersJson});
222209
let response = await fetch('${serverIslandUrl}', { headers });`
223210
: // POST request
224211
`let data = {
225-
encryptedComponentExport: ${safeJsonStringify(componentExportEncrypted)},
226-
encryptedProps: ${safeJsonStringify(propsEncrypted)},
227-
encryptedSlots: ${safeJsonStringify(slotsEncrypted)},
212+
encryptedComponentExport: ${stringifyForScript(componentExportEncrypted)},
213+
encryptedProps: ${stringifyForScript(propsEncrypted)},
214+
encryptedSlots: ${stringifyForScript(slotsEncrypted)},
228215
};
229216
const headers = new Headers({ 'Content-Type': 'application/json', ...${headersJson} });
230217
let response = await fetch('${serverIslandUrl}', {

packages/astro/src/runtime/server/render/util.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { clsx } from 'clsx';
22
import type { SSRElement } from '../../../types/public/internal.js';
3-
import { HTMLString, markHTMLString } from '../escape.js';
3+
import { HTMLString, markHTMLString, stringifyForScript } from '../escape.js';
44
import { isPromise } from '../util.js';
55
import type { RenderDestination, RenderDestinationChunk, RenderFunction } from './common.js';
66

@@ -44,10 +44,7 @@ export function defineScriptVars(vars: Record<any, any>) {
4444
for (const [key, value] of Object.entries(vars)) {
4545
// Use const instead of let as let global unsupported with Safari
4646
// https://stackoverflow.com/questions/29194024/cant-use-let-keyword-in-safari-javascript
47-
output += `const ${toIdent(key)} = ${JSON.stringify(value)?.replace(
48-
/<\/script>/g,
49-
'\\x3C/script>',
50-
)};\n`;
47+
output += `const ${toIdent(key)} = ${stringifyForScript(value)};\n`;
5148
}
5249
return markHTMLString(output);
5350
}

packages/astro/test/astro-directives.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe('Directives', async () => {
3434
assert.equal($(script).toString().includes('const dashCase = "bar"'), true);
3535
} else if (i < 4) {
3636
// Closing script tags in strings are escaped
37-
assert.equal($(script).toString().includes('const bar = "<script>bar\\x3C/script>"'), true);
37+
assert.equal($(script).toString().includes('const bar = "\\u003cscript>bar\\u003c/script>"'), true);
3838
} else {
3939
// Vars with undefined values are handled
4040
assert.equal($(script).toString().includes('const undef = undefined'), true);

packages/astro/test/units/render/html-primitives.test.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,31 @@ describe('defineScriptVars', () => {
108108
it('sanitizes </script> to prevent XSS injection', () => {
109109
const result = String(defineScriptVars({ evil: '</script>' }));
110110
assert.ok(!result.includes('</script>'), 'should not contain literal </script>');
111-
assert.ok(result.includes('\\x3C/script>'), 'should escape the closing tag');
111+
assert.ok(result.includes('\\u003c/script>'), 'should escape the closing tag');
112+
});
113+
114+
it('sanitizes case-insensitive </script> variants', () => {
115+
for (const tag of ['</Script>', '</SCRIPT>', '</sCrIpT>']) {
116+
const result = String(defineScriptVars({ evil: tag }));
117+
assert.ok(!result.includes(tag), `should not contain literal ${tag}`);
118+
}
119+
});
120+
121+
it('sanitizes </script> with trailing whitespace before >', () => {
122+
for (const tag of ['</script >', '</script\t>', '</script\n>']) {
123+
const result = String(defineScriptVars({ evil: tag }));
124+
assert.ok(!result.includes(tag), `should not contain literal ${JSON.stringify(tag)}`);
125+
}
126+
});
127+
128+
it('sanitizes self-closing </script/>', () => {
129+
const result = String(defineScriptVars({ evil: '</script/>' }));
130+
assert.ok(!result.includes('</script/>'), 'should not contain literal </script/>');
131+
});
132+
133+
it('handles undefined values without throwing', () => {
134+
const result = String(defineScriptVars({ undef: undefined }));
135+
assert.ok(result.includes('const undef = undefined;'));
112136
});
113137

114138
it('converts keys with spaces to valid JS identifiers', () => {

0 commit comments

Comments
 (0)