fix(document): HTML-escape NextScript.getInlineScriptSource output - #2727
Conversation
The shim returned raw JSON.stringify(context.__NEXT_DATA__). JSON.stringify does not escape characters the HTML parser treats as significant, so a string value containing "</script>" terminates the inline script it is embedded in and the remainder is parsed as HTML. Next.js returns htmlEscapeJsonString(data) from every path of this method (pages/_document.tsx). A custom _document that calls the helper and inlines the result — the pattern the method exists for — therefore loses escaping it would keep on Next.js, turning page props or query-derived data into script injection. Use safeJsonStringify, the escaper this repo already applies to the framework-generated __NEXT_DATA__ tag in pages-page-response.ts, so both paths produce the same output. The default renderer was unaffected.
commit: |
Performance benchmarksCompared 1 improved · 0 regressed · 5 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
/bigbonk review for issues |
|
UnknownError: ProviderInitError |
|
@james-elicx Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
|
/bigbonk review for issues |
There was a problem hiding this comment.
Review: correct, well-scoped security fix
Verified the fix against the authoritative Next.js source and ran the local checks.
Vulnerability is real. JSON.stringify doesn't escape <, >, &, U+2028, U+2029. A page-data string value containing </script> terminates the inline <script> in a custom _document, and the remainder is parsed as HTML — a stored XSS vector on the app's origin.
Fix matches Next.js exactly. I confirmed against vercel/next.js canary packages/next/src/pages/_document.tsx:858 that getInlineScriptSource returns htmlEscapeJsonString(data) on every return path. vinext's safeJsonStringify (packages/vinext/src/server/html.ts:21) escapes the same character set (<→\u003c, >→\u003e, &→\u0026, \u2028, \u2029).
No double-escaping risk. getInlineScriptSource has no internal callers — it is only invoked by user custom _document code (as in Next.js). The default renderer builds its own __NEXT_DATA__ tag separately via safeJsonStringify (pages-page-response.ts:290), so both paths now emit identical output.
Import is fine. next/document is a server-only shim, so pulling in server/html.js adds no client-bundle weight, and server/html.ts has no imports of its own. 24 existing shims already import from ../server/, so the pattern is established.
Local verification.
vp test run tests/document.test.ts→ 15 passedvp checkon both changed files → format, lint, and type checks clean
The new test correctly asserts no literal </script>, no raw <>&/line separators, the presence of the escaped forms, and a JSON.parse round-trip back to the original value.
LGTM.
|
Review posted successfully as an approval on PR #2727. SummaryI reviewed PR #2727 ( The fix is correct and well-scoped:
Verification I ran locally:
No issues found. The change is minimal, correct, matches Next.js behavior, and is properly tested. |
Problem
NextScript.getInlineScriptSource()in thenext/documentshim returned rawJSON.stringify(context.__NEXT_DATA__):JSON.stringifydoes not escape characters the HTML parser treats as significant. A string value containing</script>terminates the inline script the result is embedded in, and everything after it is parsed as HTML rather than as script content. The same applies to<,>,&, U+2028 and U+2029.Why it matters
This method exists so a custom
_documentcan place__NEXT_DATA__into its own inline<script>. Next.js returnshtmlEscapeJsonString(data)from every return path of the equivalent method (packages/next/src/pages/_document.tsx), so an app that relied on that escaping silently lost it here — page props or query-derived values become script injection on the app's origin.The default renderer was never affected:
pages-page-response.tsalready builds its__NEXT_DATA__tag withsafeJsonStringify. Only the custom-_documentpath diverged.Fix
Call
safeJsonStringify— the escaper already used for the framework-generated tag — so both paths emit identical output.server/html.tshas no imports, so pulling it into the shim adds no dependency weight.Testing
tests/document.test.tsasserts the output contains no literal</script>, contains the escaped form, and still round-trips throughJSON.parseto the original value.tests/document.test.ts+tests/safe-json.test.ts: 45 passing. Repo pre-commit full check, unit/integration suites and knip all passed.No behavior change for apps using the default document.