-
-
Notifications
You must be signed in to change notification settings - Fork 225
refactor(plugin-rsc): convert hooks to nested handler form #1043
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: hi-ogawa <[email protected]>
| // during build, load key from an external file to make chunks stable. | ||
| return `export default () => ${KEY_PLACEHOLDER}` | ||
| } | ||
| return `export default () => (${defineEncryptionKey})` |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
General approach: When inserting a stringified value into generated JavaScript source, additionally escape characters that can prematurely terminate a <script> tag or otherwise affect JS parsing. This is done by post-processing the JSON.stringify(...) result before embedding it into source.
Best fix here: Keep defineEncryptionKey as the JSON-stringified key (so functionality and format are unchanged), but sanitize its content before using it in the template literal that returns JS code. We can implement a small, local escaping helper and wrap defineEncryptionKey with it at the sink. This avoids changing the key-generation logic or its storage format and only affects how it is embedded into the virtual module source.
Concretely in packages/plugin-rsc/src/plugin.ts:
-
Add a small
escapeUnsafeCharsForJshelper near the plugin or in a nearby utilities section that replaces<,>,/,\, control whitespace,\0, and\u2028/\u2029with safe escape sequences, following the pattern in the background example. -
Change the
load.handlerbranch at line 1684 from:return `export default () => (${defineEncryptionKey})`
to:
return `export default () => (${escapeUnsafeCharsForJs(defineEncryptionKey)})`
No new imports are required; the helper uses only core JS features. Existing behavior is preserved: we still return a function that, when executed, returns the same string value; only the intermediate source representation is additionally escaped to be safe in all embedding contexts.
-
Copy modified lines R1657-R1677 -
Copy modified lines R1705-R1707
| @@ -1654,6 +1654,27 @@ | ||
| const KEY_PLACEHOLDER = '__vite_rsc_define_encryption_key' | ||
| const KEY_FILE = '__vite_rsc_encryption_key.js' | ||
|
|
||
| function escapeUnsafeCharsForJs(str: string): string { | ||
| const charMap: Record<string, string> = { | ||
| '<': '\\u003C', | ||
| '>': '\\u003E', | ||
| '/': '\\u002F', | ||
| '\\': '\\\\', | ||
| '\b': '\\b', | ||
| '\f': '\\f', | ||
| '\n': '\\n', | ||
| '\r': '\\r', | ||
| '\t': '\\t', | ||
| '\0': '\\0', | ||
| '\u2028': '\\u2028', | ||
| '\u2029': '\\u2029', | ||
| } | ||
| return str.replace( | ||
| /[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, | ||
| (ch) => charMap[ch] ?? ch, | ||
| ) | ||
| } | ||
|
|
||
| const serverEnvironmentName = useServerPluginOptions.environment?.rsc ?? 'rsc' | ||
|
|
||
| return [ | ||
| @@ -1681,7 +1702,9 @@ | ||
| // during build, load key from an external file to make chunks stable. | ||
| return `export default () => ${KEY_PLACEHOLDER}` | ||
| } | ||
| return `export default () => (${defineEncryptionKey})` | ||
| return `export default () => (${escapeUnsafeCharsForJs( | ||
| defineEncryptionKey, | ||
| )})` | ||
| } | ||
| }, | ||
| }, |
Co-authored-by: hi-ogawa <[email protected]>
| this.environment.mode === 'dev' && | ||
| rscPluginOptions.loadModuleDevProxy | ||
| ) { | ||
| replacement = `import("virtual:vite-rsc/rpc-client").then((module) => module.createRpcClient(${JSON.stringify({ environmentName, entryName })}))` |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
In general, to fix this class of problem you should not insert raw JSON.stringify output directly into a JavaScript code string that may be embedded in HTML. Instead, you should further escape characters that can break out of a <script> context or introduce parsing surprises (<, >, /, backslash, control characters, and Unicode line/paragraph separators). The fix is to define a small escapeUnsafeChars helper (like in the background example) and apply it to the serialized data before interpolating it.
Concretely for packages/plugin-rsc/src/plugin.ts, we will:
- Add a
charMapandescapeUnsafeCharsfunction near the top-level of the file (after existing imports, beforeisRolldownVite) so it’s available where needed. - On line 790, wrap the
JSON.stringify({ environmentName, entryName })call withescapeUnsafeChars(...). This preserves the logical content of the JSON while encoding unsafe characters as Unicode escape sequences. - No other behavior changes: the receiving
createRpcClientstill gets a JSON string that, when parsed, yields the same object structure.
This requires no new imports; the helper uses only built-in JavaScript features.
-
Copy modified lines R66-R87 -
Copy modified lines R812-R814
| @@ -63,6 +63,28 @@ | ||
| } from './plugins/shared' | ||
| import { stripLiteral } from 'strip-literal' | ||
|
|
||
| const charMap = { | ||
| '<': '\\u003C', | ||
| '>': '\\u003E', | ||
| '/': '\\u002F', | ||
| '\\': '\\\\', | ||
| '\b': '\\b', | ||
| '\f': '\\f', | ||
| '\n': '\\n', | ||
| '\r': '\\r', | ||
| '\t': '\\t', | ||
| '\0': '\\0', | ||
| '\u2028': '\\u2028', | ||
| '\u2029': '\\u2029', | ||
| } | ||
|
|
||
| function escapeUnsafeChars(str: string): string { | ||
| return str.replace( | ||
| /[<>/\\\b\f\n\r\t\0\u2028\u2029]/g, | ||
| (x) => charMap[x as keyof typeof charMap] ?? x, | ||
| ) | ||
| } | ||
|
|
||
| const isRolldownVite = 'rolldownVersion' in vite | ||
|
|
||
| const BUILD_ASSETS_MANIFEST_NAME = '__vite_rsc_assets_manifest.js' | ||
| @@ -787,7 +809,9 @@ | ||
| this.environment.mode === 'dev' && | ||
| rscPluginOptions.loadModuleDevProxy | ||
| ) { | ||
| replacement = `import("virtual:vite-rsc/rpc-client").then((module) => module.createRpcClient(${JSON.stringify({ environmentName, entryName })}))` | ||
| replacement = `import("virtual:vite-rsc/rpc-client").then((module) => module.createRpcClient(${escapeUnsafeChars( | ||
| JSON.stringify({ environmentName, entryName }), | ||
| )}))` | ||
| } else if (this.environment.mode === 'dev') { | ||
| const environment = server.environments[environmentName]! | ||
| const source = getEntrySource(environment.config, entryName) |
Preparation for #957 by converting all
resolveId,load, andtransformhooks inplugin-rscto nested handler form.Changes
Converted 18 hooks from direct function form to nested handler object:
Affected hooks:
resolveIdhooks (rpc-client, assets-manifest, client-references, encryption-key, css-virtual)loadhooks (rpc-client, assets-manifest, client-references, client-package, encryption-key, css-virtual, importer-resources)transformhooks (load-environment-module, bootstrap-script, use-client, use-server, css-export, importer-resources)Pure refactoring—no functional changes. Enables hook filtering and composition patterns required by #957.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.