Skip to content

fix: reject array indices with a leading zero (RFC 6901)#331

Open
spokodev wants to merge 1 commit into
Starcounter-Jack:masterfrom
spokodev:fix-leading-zero-array-index
Open

fix: reject array indices with a leading zero (RFC 6901)#331
spokodev wants to merge 1 commit into
Starcounter-Jack:masterfrom
spokodev:fix-leading-zero-array-index

Conversation

@spokodev

Copy link
Copy Markdown

Problem

isInteger() only checks that every character is a digit, so it accepts malformed array indices with a leading zero such as 01 or 007. Those are not valid JSON Pointer array indices — RFC 6901 §4 defines an array index as "0", or a digit 1-9 followed by digits.

Because of this, add (and move/copy destinations) accept /01 and coerce it to index 1, while replace / remove / move-from reject it. Under validate: true (the spec-safe mode) validate() and applyOperation() therefore disagree about the same path depending on the operation:

const { applyOperation, validate } = require('fast-json-patch');

applyOperation([10,20,30], {op:'add',     path:'/01', value:9}, true).newDocument; // [10,9,20,30]  (mutated at index 1)
applyOperation([10,20,30], {op:'replace', path:'/01', value:9}, true);             // throws OPERATION_PATH_UNRESOLVABLE

validate([{op:'add',     path:'/01', value:9}], [10,20,30]); // undefined  (accepted)
validate([{op:'replace', path:'/01', value:9}], [10,20,30]); // JsonPatchError (rejected)

The library already intends to validate this — the dedicated error OPERATION_PATH_ILLEGAL_ARRAY_INDEX is defined as "Expected an unsigned base-10 integer value…" and /+1 is already rejected — so this is an incomplete check, not intended leniency.

Fix

Reject a leading zero in isInteger, so /01 consistently raises OPERATION_PATH_ILLEGAL_ARRAY_INDEX. "0" and ordinary indices are unaffected.

Test

Added a validate assertion that add /01 against an array is rejected with OPERATION_PATH_ILLEGAL_ARRAY_INDEX. It fails before the change and passes after. The full suite is green, including the standard json-patch-tests conformance suite (216 core specs, 83 duplex, 0 failures).

isInteger() only checked that every character is a digit, so it accepted
malformed array indices with a leading zero such as "01" or "007". Those
are not valid JSON Pointer array indices (RFC 6901 section 4: "0", or a
digit 1-9 followed by digits). As a result `add` (and move/copy
destinations) accepted them and coerced "01" to index 1, while
replace/remove/move-from rejected them, so under `validate: true`
validate() and applyOperation() disagreed about the same path depending
on the operation:

  applyOperation([10,20,30], {op:'add',     path:'/01', value:9}, true) // mutated at index 1
  applyOperation([10,20,30], {op:'replace', path:'/01', value:9}, true) // threw

Reject a leading zero in isInteger so `/01` consistently raises
OPERATION_PATH_ILLEGAL_ARRAY_INDEX, matching that error's own definition
("an unsigned base-10 integer value"). "0" and ordinary indices are
unaffected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant