fix: enforce catalog integrity for order item creation#59
fix: enforce catalog integrity for order item creation#59dharapandya85 wants to merge 1 commit intofuzziecoder:mainfrom
Conversation
|
@dharapandya85 is attempting to deploy a commit to the Revon Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/db.js`:
- Around line 244-247: The map callback used to build parsedItems currently uses
the 'in' operator on item which can throw a TypeError for
null/undefined/primitives and also checks prototype properties; before accessing
'in' replace that with a type guard that ensures item is a non-null object
(e.g., typeof item === 'object' && item !== null) and then use
Object.prototype.hasOwnProperty.call(item, 'name') / 'unitPrice' / 'total' to
check only own properties; update the items.map callback to validate and throw
the existing Error when those own properties are present, and otherwise continue
parsing so malformed payloads produce a controlled validation error instead of a
runtime TypeError.
| const parsedItems = items.map((item) => { | ||
| if ('name' in item || 'unitPrice' in item || 'total' in item) { | ||
| throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JavaScript in operator behavior with null undefined primitives TypeError
💡 Result:
In JavaScript, the in operator requires its right-hand operand to be an Object. Per the ECMAScript spec, evaluating prop in rhs does:
- evaluate
rhsto a valuerVal - if
rValis not an Object, throwTypeError - otherwise return
HasProperty(rVal, ToPropertyKey(prop))(checks own + prototype chain) [1]
So these all throw, because the RHS is not an Object (including null, undefined, and all primitives like strings/numbers/booleans/bigints/symbols):
"x" in null // TypeError
"x" in undefined // TypeError
"length" in "abc" // TypeErrorThis is the same error described by MDN (“right-hand side of 'in' should be an object”) and it explicitly calls out null/undefined and primitives as causes [2].
How to avoid the TypeError
1) Guard against nullish values (most common):
if (obj != null && ("x" in obj)) { ... } // obj is not null/undefined2) If you intentionally want “wrap primitives” behavior:
"x" in Object(maybeValue) // never throws for null/undefined; primitives get boxed(Be aware this changes semantics vs throwing—for null/undefined it becomes {}, for strings it becomes a String object wrapper.)
3) If you just want a safe property read (not in):
Use optional chaining for access:
const v = obj?.x; // undefined if obj is null/undefined[3]
Sources: ECMAScript spec (RelationalExpression in) [1], MDN error reference for in with non-objects [2], MDN optional chaining [3].
🌐 Web query:
JavaScript in operator prototype chain inheritance check
💡 Result:
In JavaScript, the in operator returns true if a property exists either as an own property or anywhere on the object’s prototype chain:
const proto = { x: 1 };
const obj = Object.create(proto);
"x" in obj; // true (found on prototype)
Object.hasOwn(obj,"x"); // false (not an own property)Key points:
prop in objchecks existence, not value—so it can betrueeven ifobj[prop] === undefined. [1]- It includes inherited properties from prototypes, which is why it’s often not the right choice for “own property only” checks; use
Object.hasOwn(obj, prop)(orhasOwnProperty.call) for that. [1] - If what you really want is an inheritance/prototype-chain relationship check (not a property check), use:
someProto.isPrototypeOf(obj)to test whethersomeProtoappears inobj’s prototype chain, orobj instanceof Ctor(which checks againstCtor.prototype). [2]
Sources:
[1] MDN — in operator (prototype chain behavior, Object.hasOwn recommendation) (developer.mozilla.org)
[2] MDN — Object.prototype.isPrototypeOf() (prototype chain relationship; note about instanceof) (developer.mozilla.org)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/isPrototypeOf?utm_source=openai
🏁 Script executed:
# Find and examine the backend/db.js file around the changed lines
find . -name "db.js" -path "*/backend/*" | head -5Repository: fuzziecoder/Brocode-Party-Update-App
Length of output: 95
🏁 Script executed:
# Read the broader context around lines 244-247 in backend/db.js
fd "db.js" backend | xargs wc -lRepository: fuzziecoder/Brocode-Party-Update-App
Length of output: 97
🏁 Script executed:
# Read the backend/db.js file with line numbers, focusing on the function context around lines 244-247
sed -n '230,260p' backend/db.js | cat -nRepository: fuzziecoder/Brocode-Party-Update-App
Length of output: 1364
🏁 Script executed:
# Read more context above createOrder to see if there's any upstream validation
sed -n '200,260p' backend/db.js | cat -nRepository: fuzziecoder/Brocode-Party-Update-App
Length of output: 2620
🏁 Script executed:
# Check the broader function to see if items is validated anywhere before createOrder is called
sed -n '1,100p' backend/db.js | grep -A5 -B5 "createOrder\|items" | head -30Repository: fuzziecoder/Brocode-Party-Update-App
Length of output: 997
Add type guard before using in operator on item; can throw TypeError on malformed payloads.
At line 246, the in operator throws a TypeError if item is null, undefined, or a primitive value. This can occur with malformed client input and will cause an unhandled runtime error instead of a proper validation error. Additionally, in checks the prototype chain; use Object.prototype.hasOwnProperty.call() to validate only own properties.
Suggested fix
const parsedItems = items.map((item) => {
+ if (!item || typeof item !== 'object' || Array.isArray(item)) {
+ throw new Error('Each order item must be an object with productId and a positive integer quantity');
+ }
- if ('name' in item || 'unitPrice' in item || 'total' in item) {
- throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ')
- }
+ if (
+ Object.prototype.hasOwnProperty.call(item, 'name') ||
+ Object.prototype.hasOwnProperty.call(item, 'unitPrice') ||
+ Object.prototype.hasOwnProperty.call(item, 'total')
+ ) {
+ throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog.');
+ }📝 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.
| const parsedItems = items.map((item) => { | |
| if ('name' in item || 'unitPrice' in item || 'total' in item) { | |
| throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ') | |
| } | |
| const parsedItems = items.map((item) => { | |
| if (!item || typeof item !== 'object' || Array.isArray(item)) { | |
| throw new Error('Each order item must be an object with productId and a positive integer quantity'); | |
| } | |
| if ( | |
| Object.prototype.hasOwnProperty.call(item, 'name') || | |
| Object.prototype.hasOwnProperty.call(item, 'unitPrice') || | |
| Object.prototype.hasOwnProperty.call(item, 'total') | |
| ) { | |
| throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog.'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/db.js` around lines 244 - 247, The map callback used to build
parsedItems currently uses the 'in' operator on item which can throw a TypeError
for null/undefined/primitives and also checks prototype properties; before
accessing 'in' replace that with a type guard that ensures item is a non-null
object (e.g., typeof item === 'object' && item !== null) and then use
Object.prototype.hasOwnProperty.call(item, 'name') / 'unitPrice' / 'total' to
check only own properties; update the items.map callback to validate and throw
the existing Error when those own properties are present, and otherwise continue
parsing so malformed payloads produce a controlled validation error instead of a
runtime TypeError.
|
@dharapandya85 there are some issues please fix it then let me know |
2 participants
Fixes #18
Changes:
Summary by CodeRabbit