Skip to content

fix: don't panic on empty key when path enters an array - #284

Closed
c-tonneslan wants to merge 1 commit into
buger:masterfrom
c-tonneslan:fix/empty-key-array-panic
Closed

fix: don't panic on empty key when path enters an array#284
c-tonneslan wants to merge 1 commit into
buger:masterfrom
c-tonneslan:fix/empty-key-array-panic

Conversation

@c-tonneslan

Copy link
Copy Markdown

Description: For a Get call like jsonparser.Get([]byte("[]"), "") the searchKeys '[' case reached keys[level][0] == '[' with an empty keys[level] and indexed into a zero-length string, panicking with index out of range [0] with length 0. The reproducer is exactly the one in #247:

jsonparser.Get([]byte(`[]`), "")
jsonparser.Get([]byte(`[`), "")

Both used to panic where the object equivalents ({} and {) correctly return KeyPathNotFoundError. After this change they fall through to the same array-skip branch and surface KeyPathNotFoundError instead.

Two regression cases added to getArrayTests.

Benchmark before change: this is a guard on an exceptional input. The hot path is untouched and the new check is a length comparison on a string already in cache, so I didn't expect (or measure) a perceivable benchmark delta. Happy to run make bench here if you'd like the numbers.

Closes #247.

…array

For a Get call like jsonparser.Get([]byte("[]"), "") the searchKeys
'[' case reached `keys[level][0] == '['` with an empty `keys[level]`
and indexed into a zero-length string, panicking with index out of
range. Guard the index access so an empty key falls through to the
array-skip branch and surfaces as KeyPathNotFoundError, matching the
behaviour for the object case (`Get([]byte("{}"), "")`).

Closes buger#247

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
@buger

buger commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Closing as already-fixed — thank you for the report and the fix, @c-tonneslan.

The same bug class (unguarded keys[i][0] dereference on empty-string key components) was found by a structure-aware hazard sweep and fixed comprehensively in #283, which guards all 8 sites (your fix covered the searchKeys site; #283 also covers EachKey, createInsertComponent × 3, calcAllocateSpace × 2, plus the slice-expression variant keys[depth:][0][0] in Set's depth≠0 branch that a pattern-based sweep initially missed).

Your two regression fixtures ([] and [) are functionally covered by empty_key_path_test.go in #283, which tests the empty-key path across Get, GetString, GetInt, GetFloat, GetBoolean, GetUnsafeString, EachKey, Set, and Delete — 248 lines of coverage.

The fix ships in #283 with DEFECT-260726-QS2V / KI-1. Appreciate the contribution — if you spot more parser hazards, the security and hazard-analysis proof roles are the right place to start.

@buger buger closed this Jul 27, 2026
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.

Get call panics on empty json array or invalid json with an empty path - expecting an error to be returned

2 participants