Skip to content

Commit d496c5e

Browse files
committed
Change DecodePath to return (bool, error)
The bool indicates whether the path was found, making it possible to distinguish between path-not-found and actual errors.
1 parent b8a56bf commit d496c5e

File tree

5 files changed

+67
-44
lines changed

5 files changed

+67
-44
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## 2.0.0-beta.9
44

5+
- **BREAKING CHANGE**: `DecodePath` now returns `(bool, error)` instead of
6+
just `error`. The bool indicates whether the path was found, making it
7+
possible to distinguish between path-not-found and actual errors.
58
- **SECURITY**: Fixed integer overflow vulnerability in search tree size
69
calculation that could potentially allow malformed databases to trigger
710
security issues.

fuzz_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func FuzzDatabase(f *testing.F) {
4242
_ = result.Decode(&mapResult)
4343
if mapResult != nil {
4444
var output any
45-
_ = result.DecodePath(&output, "country", "iso_code")
45+
_, _ = result.DecodePath(&output, "country", "iso_code")
4646
}
4747
}
4848
})
@@ -192,20 +192,20 @@ func FuzzDecodePath(f *testing.F) {
192192

193193
// Try to decode with the fuzzed path
194194
var output any
195-
_ = result.DecodePath(&output, path...)
195+
_, _ = result.DecodePath(&output, path...)
196196

197197
// Also test with different output types to exercise different decoding paths
198198
var stringOutput string
199-
_ = result.DecodePath(&stringOutput, path...)
199+
_, _ = result.DecodePath(&stringOutput, path...)
200200

201201
var intOutput int
202-
_ = result.DecodePath(&intOutput, path...)
202+
_, _ = result.DecodePath(&intOutput, path...)
203203

204204
var mapOutput map[string]any
205-
_ = result.DecodePath(&mapOutput, path...)
205+
_, _ = result.DecodePath(&mapOutput, path...)
206206

207207
var sliceOutput []any
208-
_ = result.DecodePath(&sliceOutput, path...)
208+
_, _ = result.DecodePath(&sliceOutput, path...)
209209
})
210210
}
211211

internal/decoder/reflection.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ func (d *ReflectionDecoder) DecodePath(
9393
offset uint,
9494
path []any,
9595
v any,
96-
) error {
96+
) (bool, error) {
9797
result := reflect.ValueOf(v)
9898
if result.Kind() != reflect.Ptr || result.IsNil() {
99-
return errors.New("result param must be a pointer")
99+
return false, errors.New("result param must be a pointer")
100100
}
101101

102102
PATH:
@@ -108,23 +108,23 @@ PATH:
108108
)
109109
typeNum, size, offset, err = d.decodeCtrlData(offset)
110110
if err != nil {
111-
return err
111+
return false, err
112112
}
113113

114114
if typeNum == KindPointer {
115115
pointer, _, err := d.decodePointer(size, offset)
116116
if err != nil {
117-
return err
117+
return false, err
118118
}
119119

120120
typeNum, size, offset, err = d.decodeCtrlData(pointer)
121121
if err != nil {
122-
return err
122+
return false, err
123123
}
124124

125125
// Check for pointer-to-pointer after we've already read the data
126126
if typeNum == KindPointer {
127-
return mmdberrors.NewInvalidDatabaseError(
127+
return false, mmdberrors.NewInvalidDatabaseError(
128128
"invalid pointer to pointer at offset %d",
129129
pointer,
130130
)
@@ -135,53 +135,53 @@ PATH:
135135
case string:
136136
// We are expecting a map
137137
if typeNum != KindMap {
138-
return fmt.Errorf("expected a map for %s but found %s", v, typeNum.String())
138+
return false, fmt.Errorf("expected a map for %s but found %s", v, typeNum.String())
139139
}
140140
for range size {
141141
var key []byte
142142
key, offset, err = d.decodeKey(offset)
143143
if err != nil {
144-
return err
144+
return false, err
145145
}
146146
if string(key) == v {
147147
continue PATH
148148
}
149149
offset, err = d.nextValueOffset(offset, 1)
150150
if err != nil {
151-
return err
151+
return false, err
152152
}
153153
}
154-
// Not found. Maybe return a boolean?
155-
return nil
154+
// Not found
155+
return false, nil
156156
case int:
157157
// We are expecting an array
158158
if typeNum != KindSlice {
159-
return fmt.Errorf("expected a slice for %d but found %s", v, typeNum.String())
159+
return false, fmt.Errorf("expected a slice for %d but found %s", v, typeNum.String())
160160
}
161161
var i uint
162162
if v < 0 {
163163
if size < uint(-v) {
164164
// Slice is smaller than negative index, not found
165-
return nil
165+
return false, nil
166166
}
167167
i = size - uint(-v)
168168
} else {
169169
if size <= uint(v) {
170170
// Slice is smaller than index, not found
171-
return nil
171+
return false, nil
172172
}
173173
i = uint(v)
174174
}
175175
offset, err = d.nextValueOffset(offset, i)
176176
if err != nil {
177-
return err
177+
return false, err
178178
}
179179
default:
180-
return fmt.Errorf("unexpected type for %d value in path, %v: %T", i, v, v)
180+
return false, fmt.Errorf("unexpected type for %d value in path, %v: %T", i, v, v)
181181
}
182182
}
183183
_, err := d.decode(offset, result, len(path))
184-
return d.wrapError(err, offset)
184+
return true, d.wrapError(err, offset)
185185
}
186186

187187
// wrapError wraps an error with context information when an error occurs.

reader_test.go

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -332,40 +332,55 @@ func TestDecodePath(t *testing.T) {
332332

333333
var u16 uint16
334334

335-
require.NoError(t, result.DecodePath(&u16, "uint16"))
336-
335+
found, err := result.DecodePath(&u16, "uint16")
336+
require.NoError(t, err)
337+
assert.True(t, found)
337338
assert.Equal(t, uint16(100), u16)
338339

339340
var u uint
340-
require.NoError(t, result.DecodePath(&u, "array", 0))
341+
found, err = result.DecodePath(&u, "array", 0)
342+
require.NoError(t, err)
343+
assert.True(t, found)
341344
assert.Equal(t, uint(1), u)
342345

343346
var u2 uint
344-
require.NoError(t, result.DecodePath(&u2, "array", 2))
347+
found, err = result.DecodePath(&u2, "array", 2)
348+
require.NoError(t, err)
349+
assert.True(t, found)
345350
assert.Equal(t, uint(3), u2)
346351

347352
// This is past the end of the array
348353
var u3 uint
349-
require.NoError(t, result.DecodePath(&u3, "array", 3))
354+
found, err = result.DecodePath(&u3, "array", 3)
355+
require.NoError(t, err)
356+
assert.False(t, found)
350357
assert.Equal(t, uint(0), u3)
351358

352359
// Negative offsets
353360

354361
var n1 uint
355-
require.NoError(t, result.DecodePath(&n1, "array", -1))
362+
found, err = result.DecodePath(&n1, "array", -1)
363+
require.NoError(t, err)
364+
assert.True(t, found)
356365
assert.Equal(t, uint(3), n1)
357366

358367
var n2 uint
359-
require.NoError(t, result.DecodePath(&n2, "array", -3))
368+
found, err = result.DecodePath(&n2, "array", -3)
369+
require.NoError(t, err)
370+
assert.True(t, found)
360371
assert.Equal(t, uint(1), n2)
361372

362373
var u4 uint
363-
require.NoError(t, result.DecodePath(&u4, "map", "mapX", "arrayX", 1))
374+
found, err = result.DecodePath(&u4, "map", "mapX", "arrayX", 1)
375+
require.NoError(t, err)
376+
assert.True(t, found)
364377
assert.Equal(t, uint(8), u4)
365378

366379
// Does key not exist
367380
var ne uint
368-
require.NoError(t, result.DecodePath(&ne, "does-not-exist", 1))
381+
found, err = result.DecodePath(&ne, "does-not-exist", 1)
382+
require.NoError(t, err)
383+
assert.False(t, found)
369384
assert.Equal(t, uint(0), ne)
370385
}
371386

@@ -1000,7 +1015,7 @@ func BenchmarkDecodePathCountryCode(b *testing.B) {
10001015
s := make(net.IP, 4)
10011016
for range b.N {
10021017
ip := randomIPv4Address(r, s)
1003-
err = db.Lookup(ip).DecodePath(&result, path...)
1018+
_, err = db.Lookup(ip).DecodePath(&result, path...)
10041019
if err != nil {
10051020
b.Error(err)
10061021
}

result.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,26 +59,31 @@ func (r Result) Decode(v any) error {
5959
//
6060
// If the path is empty, the entire data structure is decoded into v.
6161
//
62-
// Returns an error if:
63-
// - the path is invalid
64-
// - the data cannot be decoded into the type of v
65-
// - v is not a pointer or the database record cannot be stored in v due to
66-
// type mismatch
67-
// - the Result does not contain valid data
62+
// Returns (found, error):
63+
// - found is true if the path was found and decoded successfully
64+
// - found is false if the path was not found (but no error occurred)
65+
// - error is returned for actual errors (invalid path, type mismatch, etc.)
6866
//
6967
// Example usage:
7068
//
7169
// var city string
72-
// err := result.DecodePath(&city, "location", "city", "names", "en")
70+
// found, err := result.DecodePath(&city, "location", "city", "names", "en")
71+
// if err != nil {
72+
// // Handle error
73+
// } else if found {
74+
// // city contains the decoded value
75+
// } else {
76+
// // path was not found
77+
// }
7378
//
7479
// var geonameID int
75-
// err := result.DecodePath(&geonameID, "subdivisions", 0, "geoname_id")
76-
func (r Result) DecodePath(v any, path ...any) error {
80+
// found, err := result.DecodePath(&geonameID, "subdivisions", 0, "geoname_id")
81+
func (r Result) DecodePath(v any, path ...any) (bool, error) {
7782
if r.err != nil {
78-
return r.err
83+
return false, r.err
7984
}
8085
if r.offset == notFound {
81-
return nil
86+
return false, nil
8287
}
8388
return r.decoder.DecodePath(r.offset, path, v)
8489
}

0 commit comments

Comments
 (0)