Skip to content

Commit 753cda0

Browse files
bugerbuger
committed
fix: guard empty-string key path components; remove synthetic proof scaffolding
Hazard-sweep finding: Get/Set/Delete/EachKey panicked with index-out-of-range when a caller passed an empty-string path component (""). 7 unguarded keys[i][0] dereference sites in searchKeys, EachKey, createInsertComponent, calcAllocateSpace. Same bug class as the OSS-Fuzz Delete panic, on the path side. Fixed by applying the existing len(...) > 0 guard pattern (already at parser.go:835 in Delete) to all 7 sites. Regression tests in empty_key_path_test.go. DEFECT-260726-QS2V + KI-1 filed. Also removed 5 synthetic proof-scaffolding functions that existed only to game the audit (deleteCleanupBuggyDereferenceObligation, deleteCleanupFixedDereferenceObligation, deleteCleanupBuggyFalsifyingWitness, keysCount, isUTF16EncodedRuneNot): no production caller, hosted reqproof:lemma directives on synthetic stand-ins rather than real code. Obligations routed honestly to DEFECT-260726-QS2V / KI-1 / real-function annotations.
1 parent 7169e68 commit 753cda0

6 files changed

Lines changed: 597 additions & 76 deletions

File tree

‎empty_key_path_test.go‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
package jsonparser
2+
3+
import (
4+
"errors"
5+
"testing"
6+
)
7+
8+
// Regression coverage for the empty-string key path component hazard
9+
// (hazard-sweep finding). A caller passing "" as a path component used to
10+
// trigger `runtime error: index out of range [0] with length 0` at the
11+
// unguarded `keys[i][0]` / `p[level][0]` dereference sites in searchKeys,
12+
// EachKey, createInsertComponent, and calcAllocateSpace. The fix adds the
13+
// same `len(...) > 0` guard that already existed in Delete (parser.go:835),
14+
// routing an empty key component through the existing not-found / no-callback
15+
// / unchanged-payload path instead of panicking.
16+
//
17+
// These cases assert panic-free degradation: each entry MUST surface a typed
18+
// not-found outcome (or, for Set, produce a defined document) rather than
19+
// crashing the goroutine.
20+
21+
// runNoPanic executes fn and fails the test if it panics, returning the
22+
// recovered value so callers can also assert on the post-fix result.
23+
// reqproof:proptest:skip test-helper that asserts a callback does not panic; assertion utility with no return value to compare against a reference
24+
func runNoPanic(t *testing.T, name string, fn func()) {
25+
t.Helper()
26+
defer func() {
27+
if r := recover(); r != nil {
28+
t.Fatalf("%s panicked (empty-key regression): %v", name, r)
29+
}
30+
}()
31+
fn()
32+
}
33+
34+
// =============================================================================
35+
// Get family — empty key component resolves to KeyPathNotFoundError (SYS-REQ-016)
36+
// =============================================================================
37+
38+
// Verifies: SYS-REQ-016 [boundary]
39+
// An empty-string path component is not a resolvable object key or array index;
40+
// Get must surface KeyPathNotFoundError rather than panicking.
41+
// reqproof:proptest:skip test-case harness function; is itself a unit/integration test, not a pure function amenable to property-based testing
42+
func TestGetEmptyKeyPathComponent(t *testing.T) {
43+
cases := []struct {
44+
name string
45+
data string
46+
keys []string
47+
}{
48+
{name: "empty component on array root", data: `[1,2,3]`, keys: []string{""}},
49+
{name: "empty component on object root", data: `{"a":1}`, keys: []string{""}},
50+
{name: "empty component after valid key", data: `{"a":[1]}`, keys: []string{"a", ""}},
51+
{name: "empty component before valid key", data: `{"a":1}`, keys: []string{"", "a"}},
52+
{name: "two empty components", data: `{"a":1}`, keys: []string{"", ""}},
53+
}
54+
for _, tc := range cases {
55+
t.Run(tc.name, func(t *testing.T) {
56+
var (
57+
val []byte
58+
dt ValueType
59+
off int
60+
err error
61+
)
62+
runNoPanic(t, tc.name, func() {
63+
val, dt, off, err = Get([]byte(tc.data), tc.keys...)
64+
})
65+
if !errors.Is(err, KeyPathNotFoundError) {
66+
t.Fatalf("Get(%q,%v) err = %v, want KeyPathNotFoundError", tc.data, tc.keys, err)
67+
}
68+
if dt != NotExist {
69+
t.Fatalf("Get(%q,%v) type = %v, want NotExist", tc.data, tc.keys, dt)
70+
}
71+
if off != -1 {
72+
t.Fatalf("Get(%q,%v) offset = %d, want -1", tc.data, tc.keys, off)
73+
}
74+
if val != nil {
75+
t.Fatalf("Get(%q,%v) value = %v, want nil", tc.data, tc.keys, val)
76+
}
77+
})
78+
}
79+
}
80+
81+
// Verifies: SYS-REQ-016 [boundary]
82+
// Typed Get accessors must propagate KeyPathNotFoundError for an empty key
83+
// component instead of panicking on the underlying searchKeys dereference.
84+
// reqproof:proptest:skip test-case harness function; is itself a unit/integration test, not a pure function amenable to property-based testing
85+
func TestTypedGetEmptyKeyPathComponent(t *testing.T) {
86+
t.Run("GetString", func(t *testing.T) {
87+
var err error
88+
runNoPanic(t, "GetString", func() {
89+
_, err = GetString([]byte(`{"a":"x"}`), "")
90+
})
91+
if !errors.Is(err, KeyPathNotFoundError) {
92+
t.Fatalf("GetString empty-key err = %v, want KeyPathNotFoundError", err)
93+
}
94+
})
95+
t.Run("GetInt", func(t *testing.T) {
96+
var err error
97+
runNoPanic(t, "GetInt", func() {
98+
_, err = GetInt([]byte(`{"a":1}`), "")
99+
})
100+
if !errors.Is(err, KeyPathNotFoundError) {
101+
t.Fatalf("GetInt empty-key err = %v, want KeyPathNotFoundError", err)
102+
}
103+
})
104+
t.Run("GetFloat", func(t *testing.T) {
105+
var err error
106+
runNoPanic(t, "GetFloat", func() {
107+
_, err = GetFloat([]byte(`{"a":1.5}`), "")
108+
})
109+
if !errors.Is(err, KeyPathNotFoundError) {
110+
t.Fatalf("GetFloat empty-key err = %v, want KeyPathNotFoundError", err)
111+
}
112+
})
113+
t.Run("GetBoolean", func(t *testing.T) {
114+
var err error
115+
runNoPanic(t, "GetBoolean", func() {
116+
_, err = GetBoolean([]byte(`{"a":true}`), "")
117+
})
118+
if !errors.Is(err, KeyPathNotFoundError) {
119+
t.Fatalf("GetBoolean empty-key err = %v, want KeyPathNotFoundError", err)
120+
}
121+
})
122+
t.Run("GetUnsafeString", func(t *testing.T) {
123+
var err error
124+
runNoPanic(t, "GetUnsafeString", func() {
125+
_, err = GetUnsafeString([]byte(`{"a":"x"}`), "")
126+
})
127+
if !errors.Is(err, KeyPathNotFoundError) {
128+
t.Fatalf("GetUnsafeString empty-key err = %v, want KeyPathNotFoundError", err)
129+
}
130+
})
131+
}
132+
133+
// =============================================================================
134+
// EachKey — empty key component emits no callback (SYS-REQ-008)
135+
// =============================================================================
136+
137+
// Verifies: SYS-REQ-008 [boundary]
138+
// An empty-string path component cannot address an array index, so EachKey
139+
// must skip the path (missing-request => no callback) and must not panic on
140+
// the `p[level][0]` dereference.
141+
// reqproof:proptest:skip test-case harness function; is itself a unit/integration test, not a pure function amenable to property-based testing
142+
func TestEachKeyEmptyKeyPathComponent(t *testing.T) {
143+
cases := []struct {
144+
name string
145+
data string
146+
paths [][]string
147+
}{
148+
{name: "single empty path on array root", data: `[1,2,3]`, paths: [][]string{{""}}},
149+
{name: "empty path mixed with valid path", data: `{"a":1,"b":2}`, paths: [][]string{{""}, {"a"}}},
150+
{name: "empty leading component", data: `{"a":1}`, paths: [][]string{{"", "a"}}},
151+
{name: "empty trailing component", data: `{"a":[1]}`, paths: [][]string{{"a", ""}}},
152+
}
153+
for _, tc := range cases {
154+
t.Run(tc.name, func(t *testing.T) {
155+
emptyCalled := false
156+
runNoPanic(t, tc.name, func() {
157+
EachKey([]byte(tc.data), func(idx int, val []byte, dt ValueType, err error) {
158+
// Callbacks may fire for the valid path, but the empty path
159+
// must never resolve to a real callback slot.
160+
if len(tc.paths[idx]) == 0 || tc.paths[idx][0] == "" {
161+
emptyCalled = true
162+
}
163+
}, tc.paths...)
164+
})
165+
if emptyCalled {
166+
t.Fatalf("EachKey(%q,%v) emitted a callback for an empty path component", tc.data, tc.paths)
167+
}
168+
// No assertion on the valid-path callback: it may or may not fire
169+
// depending on the case; the regression gate is "no panic, no
170+
// callback for the empty component".
171+
})
172+
}
173+
}
174+
175+
// =============================================================================
176+
// Set — empty key component produces a defined document, no panic (SYS-REQ-009)
177+
// =============================================================================
178+
179+
// Verifies: SYS-REQ-009 [boundary]
180+
// Set with an empty-string key component must not panic in
181+
// createInsertComponent / calcAllocateSpace. The empty key is treated as an
182+
// object property name (not an array index) and produces a defined document.
183+
// reqproof:proptest:skip test-case harness function; is itself a unit/integration test, not a pure function amenable to property-based testing
184+
func TestSetEmptyKeyPathComponent(t *testing.T) {
185+
cases := []struct {
186+
name string
187+
data string
188+
setData string
189+
keys []string
190+
}{
191+
{name: "single empty key on empty object", data: `{}`, setData: `"v"`, keys: []string{""}},
192+
{name: "single empty key on populated object", data: `{"a":1}`, setData: `"v"`, keys: []string{""}},
193+
{name: "empty key leading", data: `{}`, setData: `"v"`, keys: []string{"", "a"}},
194+
}
195+
for _, tc := range cases {
196+
t.Run(tc.name, func(t *testing.T) {
197+
var (
198+
val []byte
199+
err error
200+
)
201+
runNoPanic(t, tc.name, func() {
202+
val, err = Set([]byte(tc.data), []byte(tc.setData), tc.keys...)
203+
})
204+
if err != nil {
205+
t.Fatalf("Set(%q,%q,%v) returned unexpected error: %v", tc.data, tc.setData, tc.keys, err)
206+
}
207+
if val == nil {
208+
t.Fatalf("Set(%q,%q,%v) returned nil document", tc.data, tc.setData, tc.keys)
209+
}
210+
})
211+
}
212+
}
213+
214+
// =============================================================================
215+
// Delete — empty key component leaves the payload unchanged, no panic
216+
// (SYS-REQ-034 / SYS-REQ-035)
217+
// =============================================================================
218+
219+
// Verifies: SYS-REQ-034 [boundary]
220+
// Verifies: SYS-REQ-035 [boundary]
221+
// Delete with an empty-string key component cannot resolve a target; the
222+
// parser must return the original byte payload unchanged and must not panic
223+
// on the `keys[lk-1][0]` dereference.
224+
// reqproof:proptest:skip test-case harness function; is itself a unit/integration test, not a pure function amenable to property-based testing
225+
func TestDeleteEmptyKeyPathComponent(t *testing.T) {
226+
cases := []struct {
227+
name string
228+
data string
229+
keys []string
230+
}{
231+
{name: "single empty key", data: `{"a":1,"b":2}`, keys: []string{""}},
232+
{name: "empty key trailing", data: `{"a":1}`, keys: []string{"a", ""}},
233+
{name: "empty key leading", data: `{"a":1}`, keys: []string{"", "a"}},
234+
}
235+
for _, tc := range cases {
236+
t.Run(tc.name, func(t *testing.T) {
237+
var got []byte
238+
runNoPanic(t, tc.name, func() {
239+
got = Delete([]byte(tc.data), tc.keys...)
240+
})
241+
want := []byte(tc.data)
242+
if string(got) != string(want) {
243+
t.Fatalf("Delete(%q,%v) = %q, want original payload %q (unchanged)",
244+
tc.data, tc.keys, string(got), tc.data)
245+
}
246+
})
247+
}
248+
}

‎escape.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,6 @@ func isUTF16EncodedRune(r rune) bool {
112112
return 0xD800 <= r && r <= 0xDFFF
113113
}
114114

115-
// isUTF16EncodedRuneNot is a thin alias hosting an additional lemma.
116-
// reqproof:lemma isUTF16EncodedRune_high_excluded func(r rune) bool {
117-
// return !(r > 0xDFFF) || !isUTF16EncodedRuneNot(r)
118-
// }
119-
func isUTF16EncodedRuneNot(r rune) bool {
120-
return isUTF16EncodedRune(r)
121-
}
122-
123115
func decodeUnicodeEscape(in []byte) (rune, int) {
124116
if r, ok := decodeSingleUnicodeEscape(in); !ok {
125117
// Invalid Unicode escape

0 commit comments

Comments
 (0)