Skip to content

JSONPointer: reject array index tokens that RFC 6901 does not allow - #1098

Open
simpleqt wants to merge 1 commit into
stleary:masterfrom
simpleqt:fix/jsonpointer-array-index-grammar
Open

simpleqt wants to merge 1 commit into
stleary:masterfrom
simpleqt:fix/jsonpointer-array-index-grammar

Conversation

@simpleqt

@simpleqt simpleqt commented Nov 4, 2026

Copy link
Copy Markdown

JSONPointer.readByIndexToken (src/main/java/org/json/JSONPointer.java:226) resolved an array element with Integer.parseInt(indexToken). RFC 6901 section 4 restricts an array index much more narrowly:

array-index = %x30 / ( %x31-39 *(%x30-39) )

i.e. "0", or a digit string whose first digit is 1-9. Integer.parseInt is more permissive than that grammar, so tokens that are not array indexes were accepted whenever Java could read them as a number.

Reproducer

JSONArray array = new JSONArray("[10,20,30,40,50,60]");
array.query("/0");    // 10
array.query("/01");   // 20  <- returns a different element instead of failing
array.query("/00");   // 10
array.query("/+1");   // 20
array.optQuery("/01"); // 20

/01 does not address the element at index 1 — it does not address any element, since the token is not an array index. Returning 20 hides a bug in the caller's pointer: /0 and /01 name different tokens yet resolve to the same value, and /00//01 silently select element 0/1. +1 is not matched by the grammar either (it is not even a digit string), and a token longer than Integer.MAX_VALUE reported 2147483648 is not an array index for what is really an out-of-range index.

Fix

Added toArrayIndex(), which parses the token only when it matches the array-index production, and used it in readByIndexToken(). Parsing moved out of the try block whose catch (NumberFormatException) was what made Integer.parseInt the de-facto grammar in the first place; the out of range check and its message are unchanged.

  • /0, /1, ... behave exactly as before.
  • Out of range indexes keep their existing message (index 6 is out of bounds - the array has 6 elements).
  • Tokens that are not array indexes raise JSONPointerException (01 is not an array index), which is what JSONArrayTest/JSONPointerTest already expect for non-index tokens in an array (a is not an array index).
  • Divergence with a readable reference: Jackson's JsonPointer._parseIndex (jackson-core 2.18.2) rejects 01, 00, +1, -1, 1.0, -0, an Arabic-Indic 1 and 0000000000000000001, and accepts 0 and 1. After this change json-java agrees on every one of those tokens.
  • Member names of a JSONObject are not array indexes and are untouched: /01 still addresses the member named "01" (the library has no equivalent of #-escaping a numeric key for arrays).

Testing

  • New regression test JSONPointerTest.arrayIndexTokenMustMatchRfc6901Grammar covers the accepted indexes, the rejected spellings, and the optQuery result.
  • RED on the base commit, with the new test in place but JSONPointer.java reverted:
Tests run: 43, Failures: 1, Errors: 0
JSONPointerTest.arrayIndexTokenMustMatchRfc6901Grammar:325
  Expected JSONPointerException for array index token '01', got 20
  • GREEN after the change: mvn test → Tests run: 840, Failures: 0, Errors: 0, Skipped: 6, BUILD SUCCESS (JDK 17 locally, same version CI uses for build-17).
  • No existing test asserted the old acceptance of "01"/"00"; the full suite is unaffected.

JSONPointer.readByIndexToken resolved array elements with
Integer.parseInt(indexToken), so tokens which are not array indexes per
RFC 6901 section 4 (array-index = %x30 / ( %x31-39 *(%x30-39) )) were
accepted whenever Java's number parsing accepted them.

On a JSONArray, "/01" therefore returned element 1 and "/00" returned
element 0 instead of failing, i.e. a pointer that does not address the
element it names silently selected a different one. The token "+1" was
accepted as well, and a token longer than Integer.MAX_VALUE reported
"2147483648 is not an array index" for what is really an out of range
index.

Parse the token only when it matches the grammar, in a new toArrayIndex()
helper used by readByIndexToken(). Valid indexes such as "/0" and "/1"
behave as before, out of range indexes keep their existing message, and
non-conforming tokens now raise a JSONPointerException. Member names of a
JSONObject are unaffected: they are not array indexes, so "/01" still
addresses the member named "01".

This matches Jackson's JsonPointer (com.fasterxml.jackson.core) and its
JsonPointer._parseIndex(), which rejects exactly the same tokens.
Copilot AI balanced review requested due to automatic review settings November 4, 2026 17:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This branch has not been deployed

No deployments
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.

2 participants