Conversation
Extended the nested object reconstruction to handle array indices in multipart
form keys (e.g., `images.create[0]`, `items[1].name`).
Changes:
- Updated setNestedValue() to parse and handle array index notation [N]
- Arrays are automatically initialized when indices are detected
- Code generation in web-standard adapter updated for array support
- Added comprehensive test for complex nested arrays with files
This enables schemas like:
t.Object({
images: t.Object({
create: t.Files(),
update: t.Array(t.Object({
id: t.String(),
img: t.File()
}))
})
})
All 49 body validator tests pass.
📝 WalkthroughWalkthroughThe changes extend form data handling and nested value assignment to support array notation (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/adapter/web-standard/index.ts">
<violation number="1" location="src/adapter/web-standard/index.ts:64">
P1: Potential prototype pollution vulnerability: The array key handling doesn't sanitize dangerous property names like `__proto__`, `constructor`, or `prototype`. A malicious form key like `__proto__[0]` or `items.__proto__[0]` could manipulate object prototypes. Consider adding a blocklist check before processing keys.</violation>
</file>
<file name="src/dynamic-handle.ts">
<violation number="1" location="src/dynamic-handle.ts:43">
P2: Missing bounds check on array index from user input. Consider adding a maximum index limit (e.g., `if (index > 10000) return` or throw an error) to prevent potential DoS via sparse arrays with extremely large indices like `items[999999999999]`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| `if(arrayMatch){` + | ||
| `const arrayKey=arrayMatch[1]\n` + | ||
| `const index=parseInt(arrayMatch[2],10)\n` + | ||
| `if(!(arrayKey in current))current[arrayKey]=[]\n` + |
There was a problem hiding this comment.
P1: Potential prototype pollution vulnerability: The array key handling doesn't sanitize dangerous property names like __proto__, constructor, or prototype. A malicious form key like __proto__[0] or items.__proto__[0] could manipulate object prototypes. Consider adding a blocklist check before processing keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/adapter/web-standard/index.ts, line 64:
<comment>Potential prototype pollution vulnerability: The array key handling doesn't sanitize dangerous property names like `__proto__`, `constructor`, or `prototype`. A malicious form key like `__proto__[0]` or `items.__proto__[0]` could manipulate object prototypes. Consider adding a blocklist check before processing keys.</comment>
<file context>
@@ -52,16 +52,35 @@ export const WebStandardAdapter: ElysiaAdapter = {
+ `if(arrayMatch){` +
+ `const arrayKey=arrayMatch[1]\n` +
+ `const index=parseInt(arrayMatch[2],10)\n` +
+ `if(!(arrayKey in current))current[arrayKey]=[]\n` +
+ `if(!Array.isArray(current[arrayKey]))current[arrayKey]=[]\n` +
+ `if(!current[arrayKey][index])current[arrayKey][index]={}\n` +
</file context>
|
|
||
| if (arrayMatch) { | ||
| const [, arrayKey, indexStr] = arrayMatch | ||
| const index = parseInt(indexStr, 10) |
There was a problem hiding this comment.
P2: Missing bounds check on array index from user input. Consider adding a maximum index limit (e.g., if (index > 10000) return or throw an error) to prevent potential DoS via sparse arrays with extremely large indices like items[999999999999].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/dynamic-handle.ts, line 43:
<comment>Missing bounds check on array index from user input. Consider adding a maximum index limit (e.g., `if (index > 10000) return` or throw an error) to prevent potential DoS via sparse arrays with extremely large indices like `items[999999999999]`.</comment>
<file context>
@@ -29,17 +29,50 @@ const setNestedValue = (
+
+ if (arrayMatch) {
+ const [, arrayKey, indexStr] = arrayMatch
+ const index = parseInt(indexStr, 10)
+
+ // Initialize array if needed
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/adapter/web-standard/index.ts`:
- Around line 55-83: The traversal that builds nested objects/arrays (using
variables keys, lastKey, current, arrayMatch/lastArrayMatch, arrayKey, index,
finalValue) can break when an existing array element is a primitive; before
descending into current[arrayKey][index] or assigning current[k] as the new
current, ensure the target is an object (typeof === 'object' && current !==
null) and if it is missing or not an object replace it with {} (e.g., when
handling arrayMatch and lastArrayMatch check if current[arrayKey][index] exists
and is an object, otherwise set current[arrayKey][index]={}; similarly when
handling non-array keys ensure current[k] is an object before assigning
current=current[k]). This prevents primitive values from being treated as
containers during later dot/array-index traversals.
In `@src/dynamic-handle.ts`:
- Around line 32-75: The code crashes when an array slot contains a primitive
and later code tries to descend into it (e.g., items[0] = 1 then items[0].id);
in the keys loop and before descending into an array element you must coerce
non-objects to {}. Update the array branch where you do if
(!current[arrayKey][index]) current[arrayKey][index] = {} to instead check
whether typeof current[arrayKey][index] !== 'object' || current[arrayKey][index]
=== null (and set it to {} in that case), and apply the same defensive
coerce-before-descend logic for any place that assumes current[arrayKey][index]
is an object (the array handling in the main keys loop and before descending
into lastKey when arrayMatch is present).
Comment on lines
+55
to
+83
| `if(key.includes('.')||key.includes('[')){` + | ||
| `const keys=key.split('.')\n` + | ||
| `const lastKey=keys.pop()\n` + | ||
| `let current=c.body\n` + | ||
| `for(const k of keys){` + | ||
| `const arrayMatch=k.match(/^(.+)\\[(\\d+)\\]$/)\n` + | ||
| `if(arrayMatch){` + | ||
| `const arrayKey=arrayMatch[1]\n` + | ||
| `const index=parseInt(arrayMatch[2],10)\n` + | ||
| `if(!(arrayKey in current))current[arrayKey]=[]\n` + | ||
| `if(!Array.isArray(current[arrayKey]))current[arrayKey]=[]\n` + | ||
| `if(!current[arrayKey][index])current[arrayKey][index]={}\n` + | ||
| `current=current[arrayKey][index]` + | ||
| `}else{` + | ||
| `if(!(k in current)||typeof current[k]!=='object'||current[k]===null)` + | ||
| `current[k]={}\n` + | ||
| `current=current[k]` + | ||
| `}` + | ||
| `}\n` + | ||
| `const lastArrayMatch=lastKey.match(/^(.+)\\[(\\d+)\\]$/)\n` + | ||
| `if(lastArrayMatch){` + | ||
| `const arrayKey=lastArrayMatch[1]\n` + | ||
| `const index=parseInt(lastArrayMatch[2],10)\n` + | ||
| `if(!(arrayKey in current))current[arrayKey]=[]\n` + | ||
| `if(!Array.isArray(current[arrayKey]))current[arrayKey]=[]\n` + | ||
| `current[arrayKey][index]=finalValue` + | ||
| `}else{` + | ||
| `current[lastKey]=finalValue` + | ||
| `}` + |
There was a problem hiding this comment.
Harden array-index traversal against primitives.
If a prior field sets arrayKey[index] to a primitive (e.g., items[0]=foo) and a later field uses items[0].id, current becomes a primitive and the next assignment throws. Consider ensuring the element is an object before descending.
🐛 Proposed fix
- `if(!current[arrayKey][index])current[arrayKey][index]={}\n` +
+ `if(!current[arrayKey][index]||typeof current[arrayKey][index]!=='object'||current[arrayKey][index]===null)current[arrayKey][index]={}\n` +🤖 Prompt for AI Agents
In `@src/adapter/web-standard/index.ts` around lines 55 - 83, The traversal that
builds nested objects/arrays (using variables keys, lastKey, current,
arrayMatch/lastArrayMatch, arrayKey, index, finalValue) can break when an
existing array element is a primitive; before descending into
current[arrayKey][index] or assigning current[k] as the new current, ensure the
target is an object (typeof === 'object' && current !== null) and if it is
missing or not an object replace it with {} (e.g., when handling arrayMatch and
lastArrayMatch check if current[arrayKey][index] exists and is an object,
otherwise set current[arrayKey][index]={}; similarly when handling non-array
keys ensure current[k] is an object before assigning current=current[k]). This
prevents primitive values from being treated as containers during later
dot/array-index traversals.
Comment on lines
+32
to
+75
| // Split by dots, but preserve array indices | ||
| const keys = path.split('.') | ||
| const lastKey = keys.pop()! | ||
|
|
||
| let current = obj | ||
| for (const key of keys) { | ||
| if (!(key in current) || typeof current[key] !== 'object' || current[key] === null) | ||
| current[key] = {} | ||
| current = current[key] | ||
| // Check if key has array index notation: key[0], key[1], etc. | ||
| const arrayMatch = key.match(/^(.+)\[(\d+)\]$/) | ||
|
|
||
| if (arrayMatch) { | ||
| const [, arrayKey, indexStr] = arrayMatch | ||
| const index = parseInt(indexStr, 10) | ||
|
|
||
| // Initialize array if needed | ||
| if (!(arrayKey in current)) current[arrayKey] = [] | ||
|
|
||
| // Ensure it's an array | ||
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | ||
|
|
||
| // Initialize object at index if needed | ||
| if (!current[arrayKey][index]) current[arrayKey][index] = {} | ||
|
|
||
| current = current[arrayKey][index] | ||
| } else { | ||
| // Regular object property | ||
| if (!(key in current) || typeof current[key] !== 'object' || current[key] === null) | ||
| current[key] = {} | ||
| current = current[key] | ||
| } | ||
| } | ||
|
|
||
| current[lastKey] = value | ||
| // Handle array index in last key | ||
| const arrayMatch = lastKey.match(/^(.+)\[(\d+)\]$/) | ||
| if (arrayMatch) { | ||
| const [, arrayKey, indexStr] = arrayMatch | ||
| const index = parseInt(indexStr, 10) | ||
|
|
||
| if (!(arrayKey in current)) current[arrayKey] = [] | ||
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | ||
|
|
||
| current[arrayKey][index] = value | ||
| } else { | ||
| current[lastKey] = value | ||
| } |
There was a problem hiding this comment.
Prevent crashes when array elements are non-objects.
If an array index already contains a primitive and a later path descends further (e.g., items[0]=1 then items[0].id), this will throw at assignment. Guard by coercing non-objects to {} before descending.
🐛 Proposed fix
- if (!current[arrayKey][index]) current[arrayKey][index] = {}
+ if (
+ !current[arrayKey][index] ||
+ typeof current[arrayKey][index] !== 'object' ||
+ current[arrayKey][index] === null
+ )
+ current[arrayKey][index] = {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Split by dots, but preserve array indices | |
| const keys = path.split('.') | |
| const lastKey = keys.pop()! | |
| let current = obj | |
| for (const key of keys) { | |
| if (!(key in current) || typeof current[key] !== 'object' || current[key] === null) | |
| current[key] = {} | |
| current = current[key] | |
| // Check if key has array index notation: key[0], key[1], etc. | |
| const arrayMatch = key.match(/^(.+)\[(\d+)\]$/) | |
| if (arrayMatch) { | |
| const [, arrayKey, indexStr] = arrayMatch | |
| const index = parseInt(indexStr, 10) | |
| // Initialize array if needed | |
| if (!(arrayKey in current)) current[arrayKey] = [] | |
| // Ensure it's an array | |
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | |
| // Initialize object at index if needed | |
| if (!current[arrayKey][index]) current[arrayKey][index] = {} | |
| current = current[arrayKey][index] | |
| } else { | |
| // Regular object property | |
| if (!(key in current) || typeof current[key] !== 'object' || current[key] === null) | |
| current[key] = {} | |
| current = current[key] | |
| } | |
| } | |
| current[lastKey] = value | |
| // Handle array index in last key | |
| const arrayMatch = lastKey.match(/^(.+)\[(\d+)\]$/) | |
| if (arrayMatch) { | |
| const [, arrayKey, indexStr] = arrayMatch | |
| const index = parseInt(indexStr, 10) | |
| if (!(arrayKey in current)) current[arrayKey] = [] | |
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | |
| current[arrayKey][index] = value | |
| } else { | |
| current[lastKey] = value | |
| } | |
| // Split by dots, but preserve array indices | |
| const keys = path.split('.') | |
| const lastKey = keys.pop()! | |
| let current = obj | |
| for (const key of keys) { | |
| // Check if key has array index notation: key[0], key[1], etc. | |
| const arrayMatch = key.match(/^(.+)\[(\d+)\]$/) | |
| if (arrayMatch) { | |
| const [, arrayKey, indexStr] = arrayMatch | |
| const index = parseInt(indexStr, 10) | |
| // Initialize array if needed | |
| if (!(arrayKey in current)) current[arrayKey] = [] | |
| // Ensure it's an array | |
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | |
| // Initialize object at index if needed | |
| if ( | |
| !current[arrayKey][index] || | |
| typeof current[arrayKey][index] !== 'object' || | |
| current[arrayKey][index] === null | |
| ) | |
| current[arrayKey][index] = {} | |
| current = current[arrayKey][index] | |
| } else { | |
| // Regular object property | |
| if (!(key in current) || typeof current[key] !== 'object' || current[key] === null) | |
| current[key] = {} | |
| current = current[key] | |
| } | |
| } | |
| // Handle array index in last key | |
| const arrayMatch = lastKey.match(/^(.+)\[(\d+)\]$/) | |
| if (arrayMatch) { | |
| const [, arrayKey, indexStr] = arrayMatch | |
| const index = parseInt(indexStr, 10) | |
| if (!(arrayKey in current)) current[arrayKey] = [] | |
| if (!Array.isArray(current[arrayKey])) current[arrayKey] = [] | |
| current[arrayKey][index] = value | |
| } else { | |
| current[lastKey] = value | |
| } |
🤖 Prompt for AI Agents
In `@src/dynamic-handle.ts` around lines 32 - 75, The code crashes when an array
slot contains a primitive and later code tries to descend into it (e.g.,
items[0] = 1 then items[0].id); in the keys loop and before descending into an
array element you must coerce non-objects to {}. Update the array branch where
you do if (!current[arrayKey][index]) current[arrayKey][index] = {} to instead
check whether typeof current[arrayKey][index] !== 'object' ||
current[arrayKey][index] === null (and set it to {} in that case), and apply the
same defensive coerce-before-descend logic for any place that assumes
current[arrayKey][index] is an object (the array handling in the main keys loop
and before descending into lastKey when arrayMatch is present).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extended the nested object reconstruction to handle array indices in multipart form keys (e.g.,
images.create[0],items[1].name).Changes:
This enables schemas like:
t.Object({ images: t.Object({ create: t.Files(), update: t.Array(t.Object({ id: t.String(), img: t.File() })) }) })
All 49 body validator tests pass.
Summary by cubic
Adds support for array indices in multipart form fields when rebuilding nested bodies (e.g., images.create[0], items[1].name). This unlocks arrays and arrays of objects with files in body schemas without custom parsing.
Written for commit c816f5c. Summary will update on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.