refactor: extract generateRscEntry, generateServerEntry, generateClientEntry into entries/ (#253 batch 2)#378
Conversation
…ntEntry into entries/ Move the three remaining large code generators out of app-dev-server.ts and index.ts into dedicated modules under src/entries/, matching the pattern established by PR #377 for generateSsrEntry and generateBrowserEntry. - entries/app-rsc-entry.ts — generateRscEntry() (~2400 lines) - entries/pages-server-entry.ts — generateServerEntry() (~985 lines) - entries/pages-client-entry.ts — generateClientEntry() (~85 lines) - entries/pages-entry-helpers.ts — shared findFileWithExts() helper app-dev-server.ts and index.ts become thin re-export/delegation wrappers. All 10 entry-template snapshot tests pass unchanged.
commit: |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // JSON.stringify safely escapes quotes, backslashes, and special chars in | ||
| // both the route pattern and the absolute file path. | ||
| // lgtm[js/bad-code-sanitization] | ||
| return ` ${JSON.stringify(nextFormatPattern)}: () => import(${JSON.stringify(absPath)})`; |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, when constructing JavaScript source code by concatenating/interpolating strings that may contain arbitrary characters, you should escape additional characters beyond what JSON.stringify handles for JavaScript string literals, particularly characters that can break out of <script> tags or cause parsing surprises (like <, >, /, \u2028, \u2029, etc.). This is done by passing the string through an “unsafe character” escaper before injecting it into code.
For this specific code, the best fix with minimal behavior change is:
- Introduce a small helper
escapeUnsafeCharsin this file that replaces problematic characters with safe Unicode escape sequences, using acharMapsimilar to the background example. - Use this helper to post-process the result of
JSON.stringify(absPath)when building the dynamic import expression inloaderEntries. That is, change the line returning the loader entry from
import(${JSON.stringify(absPath)})
to
import(${escapeUnsafeChars(JSON.stringify(absPath))}). - Do not alter the runtime behavior of which modules are imported; we only change how the generated source text is encoded.
- The helper can be defined near the top of the file (after imports) and does not require new external dependencies or new imports, since it only uses built-in string/regex functionality.
Lines to change:
- Add
charMapandescapeUnsafeCharsafter the existing imports (around lines 16–17). - Update line 36 to wrap
JSON.stringify(absPath)withescapeUnsafeChars(...).
| @@ -14,6 +14,25 @@ | ||
| import { type ResolvedNextConfig } from "../config/next-config.js"; | ||
| import { findFileWithExts } from "./pages-entry-helpers.js"; | ||
|
|
||
| const unsafeCharMap: Record<string, string> = { | ||
| "<": "\\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, (ch) => unsafeCharMap[ch] ?? ch); | ||
| } | ||
|
|
||
| export async function generateClientEntry( | ||
| pagesDir: string, | ||
| nextConfig: ResolvedNextConfig, | ||
| @@ -31,9 +50,10 @@ | ||
| const absPath = r.filePath.replace(/\\/g, "/"); | ||
| const nextFormatPattern = pagesPatternToNextFormat(r.pattern); | ||
| // JSON.stringify safely escapes quotes, backslashes, and special chars in | ||
| // both the route pattern and the absolute file path. | ||
| // both the route pattern and the absolute file path. We additionally | ||
| // escape characters that could break out of a <script> context. | ||
| // lgtm[js/bad-code-sanitization] | ||
| return ` ${JSON.stringify(nextFormatPattern)}: () => import(${JSON.stringify(absPath)})`; | ||
| return ` ${JSON.stringify(nextFormatPattern)}: () => import(${escapeUnsafeChars(JSON.stringify(absPath))})`; | ||
| }); | ||
|
|
||
| const appFileBase = appFilePath?.replace(/\\/g, "/"); |
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean mechanical extraction — all import paths correctly updated, delegation wrappers pass the right parameters, and all 10 entry-template snapshot tests pass unchanged (confirmed locally). Typecheck is clean.
CodeQL alert on pages-client-entry.ts:36: False positive. Both interpolated values use JSON.stringify() which safely escapes all special characters. The // lgtm[js/bad-code-sanitization] suppression was carried over from the original code in index.ts.
Minor note (non-blocking): findFileWithExts is now duplicated — the new shared helper in pages-entry-helpers.ts and the original private copy still in index.ts:3217 (used for finding global-error). Could be consolidated in a follow-up.
LGTM.
|
PR #378 has been approved. The extraction is correct — all import paths resolve properly, the delegation wrappers pass the right parameters, snapshot tests and typecheck both pass clean. The CodeQL alert is a false positive (values are safely escaped via |
1 participant
Summary
Continues #253 (second batch, following #377).
Extracts three large code-generator functions out of
app-dev-server.tsandindex.tsinto dedicated modules undersrc/entries/:entries/app-rsc-entry.tsgenerateRscEntry()entries/pages-server-entry.tsgenerateServerEntry()entries/pages-client-entry.tsgenerateClientEntry()entries/pages-entry-helpers.tsfindFileWithExts()(shared helper)app-dev-server.tsandindex.tsbecome thin delegation wrappers (re-exports / single-line delegates).Verification
pnpm test tests/entry-templates.test.ts)pnpm run typecheckcleanNotes
./foo→../server/foo,config/config-matchers.js→../config/config-matchers.js)generateServerEntrywas kept with its original 4-space indentation to preserve template-string content exactly