feat(nuxt): experimental page component name normalisation #33513
Conversation
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (isSyncImport) { | ||
| metaRoute.component = `Object.assign(${pageImportName}, { __name: ${metaRoute.name} })` | ||
| } else { | ||
| metaRoute.component = `${metaRoute.component}.then((m) => Object.assign(m.default, { __name: ${metaRoute.name} }))` |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
To prevent code injection vulnerabilities, we should escape unsafe characters in serialized values before interpolating them in generated JavaScript code. The best way is to follow the CodeQL-provided template: create a function that escapes potentially dangerous characters (<, >, /, \, and certain whitespace/unicode), and apply this to the output of JSON.stringify(value). Thus, edit the serializeRouteValue function (lines 516–519) in packages/nuxt/src/pages/utils.ts to use this sanitization.
- Add a static map of risky character replacements (
charMap) and an escaping function (escapeUnsafeChars) before/at the top of the relevant code region. - Within
serializeRouteValue, after JSON.stringify, apply the escaping function to the string:return escapeUnsafeChars(JSON.stringify(value)). - You only need to change code inside the shown code region (lines 516–520 or nearby for helpers) in the file. Do not affect other code outside.
- No new dependencies are necessary, as this uses pure JS.
| @@ -513,9 +513,28 @@ | ||
| return routes | ||
| } | ||
|
|
||
| const charMap: 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, x => charMap[x] || x) | ||
| } | ||
|
|
||
| function serializeRouteValue (value: any, skipSerialisation = false) { | ||
| if (skipSerialisation || value === undefined) { return undefined } | ||
| return JSON.stringify(value) | ||
| return escapeUnsafeChars(JSON.stringify(value)) | ||
| } | ||
|
|
||
| type NormalizedRoute = Partial<Record<Exclude<keyof NuxtPage, 'file'>, string>> & { component?: string } |
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33513 will not alter performanceComparing Summary
|
WalkthroughThis pull request introduces a new experimental feature Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple packages with consistent patterns: a new experimental configuration option with schema definitions, conditional logic to augment component metadata, and dedicated tests for the new feature. The implementation logic is relatively straightforward (wrapping imports with Object.assign or promise chains), but review requires verification that the conditional logic correctly handles both sync and async import paths, proper integration with the experimental flag, and validation that test coverage adequately reflects the new behaviour. The heterogeneity is moderate—whilst most changes follow the same pattern (adding the flag and implementation), the different import method handling adds some complexity. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (1)📚 Learning: 2024-12-12T12:36:34.871ZApplied to files:
🧬 Code graph analysis (2)packages/nuxt/src/pages/utils.ts (2)
packages/nuxt/test/pages.test.ts (3)
🪛 GitHub Check: CodeQLpackages/nuxt/src/pages/utils.ts[warning] 598-598: Improper code sanitization 🔇 Additional comments (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| : pageImport, | ||
| } | ||
|
|
||
| if (nuxt.options.experimental.normalizePageNames) { |
There was a problem hiding this comment.
could we maybe split this into normalizeComponent and normalizeComponentWithName functions, so we only assign it once, rather than manipulating an existing string?
There was a problem hiding this comment.
Do you mean a normalizeComponentName at runtime ?
There was a problem hiding this comment.
no. I mean, at the moment it's created a few lines above and then changed here
we can instead create it just once, which will tidy up the logic
📚 Description
resolves #33254
this PR adds an experimental feature to normalize page components names using the route name.