fix(read): prevent panic on negative slice index in a key path
GetValue and Exists validated only the upper bound of a numeric key-path segment, so a negative index such as "arr.-1" passed the check and indexed the slice with -1, causing an index-out-of-range panic. Add a lower-bound check (i < 0) to the []int, []string and []any cases in both functions.
This commit is contained in:
1 parent
a78ca9b221
commit
76e67fb3e0
3 files changed
+86
-6
No files matched your search
@@ -0,0 +1,58 @@
|
|||||||
|
# fix(read): prevent panic on negative slice index in a key path
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
Looking up a key whose path contains a negative array index panics instead of
|
||||||
|
reporting "not found":
|
||||||
|
|
||||||
|
```go
|
||||||
|
c.LoadData(map[string]any{"arr": []any{"a", "b", "c"}})
|
||||||
|
|
||||||
|
c.GetValue("arr.-1") // panic: runtime error: index out of range [-1]
|
||||||
|
c.Exists("arr.-1") // panic: runtime error: index out of range [-1]
|
||||||
|
```
|
||||||
|
|
||||||
|
Key paths frequently come from user input or configuration, so a stray
|
||||||
|
negative index crashes the caller.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`read.go`, in both `GetValue` and `Exists`, the slice cases parse the path
|
||||||
|
segment with `strconv.Atoi` and validate only the upper bound:
|
||||||
|
|
||||||
|
```go
|
||||||
|
case []any: // is array(load from file)
|
||||||
|
i, err := strconv.Atoi(k)
|
||||||
|
if err != nil || len(typeData) <= i { // no lower-bound check
|
||||||
|
...
|
||||||
|
return
|
||||||
|
}
|
||||||
|
item = typeData[i] // typeData[-1] panics
|
||||||
|
```
|
||||||
|
|
||||||
|
`strconv.Atoi("-1")` succeeds with `i == -1`, which passes `len(typeData) <= i`
|
||||||
|
(`3 <= -1` is false), so `typeData[-1]` is evaluated and panics. This affects
|
||||||
|
the `[]int`, `[]string` and `[]any` cases in both functions (six sites).
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Add a lower-bound check (`i < 0`) to every slice-index guard so a negative
|
||||||
|
index is treated as not found:
|
||||||
|
|
||||||
|
```go
|
||||||
|
if err != nil || i < 0 || len(typeData) <= i { // GetValue
|
||||||
|
if err != nil || i < 0 || i >= len(typeData) { // Exists
|
||||||
|
```
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added `TestConfig_GetValue_negativeIndex` in `read_test.go`, asserting that both
|
||||||
|
`GetValue("arr.-1")` and `Exists("arr.-1")` do not panic and report not found.
|
||||||
|
|
||||||
|
- Gate: `go test ./...` -> ok (PASS, exit 0, all packages)
|
||||||
|
- `go vet ./...` -> exit 0
|
||||||
|
- `go build ./...` -> exit 0
|
||||||
|
- `gofmt -l read.go read_test.go` -> clean
|
||||||
|
|
||||||
|
Counterfactual: reverting `read.go` makes the test fail with
|
||||||
|
`Panic value: runtime.boundsError{x:-1, y:3, ...}`.
|
||||||
@@ -63,19 +63,19 @@ func (c *Config) Exists(key string, findByPath ...bool) (ok bool) {
|
|||||||
}
|
}
|
||||||
case []int: // is array(is from Set)
|
case []int: // is array(is from Set)
|
||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
if err != nil || i >= len(typeData) {
|
if err != nil || i < 0 || i >= len(typeData) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
item = typeData[i]
|
item = typeData[i]
|
||||||
case []string: // is array(is from Set)
|
case []string: // is array(is from Set)
|
||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
if err != nil || i >= len(typeData) {
|
if err != nil || i < 0 || i >= len(typeData) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
item = typeData[i]
|
item = typeData[i]
|
||||||
case []any: // is array(load from file)
|
case []any: // is array(load from file)
|
||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
if err != nil || i >= len(typeData) {
|
if err != nil || i < 0 || i >= len(typeData) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
item = typeData[i]
|
item = typeData[i]
|
||||||
@@ -214,7 +214,7 @@ func (c *Config) GetValue(key string, findByPath ...bool) (value any, ok bool) {
|
|||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
|
|
||||||
// check slice index
|
// check slice index
|
||||||
if err != nil || len(typeData) <= i {
|
if err != nil || i < 0 || len(typeData) <= i {
|
||||||
ok = false
|
ok = false
|
||||||
c.addError(err)
|
c.addError(err)
|
||||||
return
|
return
|
||||||
@@ -223,7 +223,7 @@ func (c *Config) GetValue(key string, findByPath ...bool) (value any, ok bool) {
|
|||||||
item = typeData[i]
|
item = typeData[i]
|
||||||
case []string: // is array(is from Set)
|
case []string: // is array(is from Set)
|
||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
if err != nil || len(typeData) <= i {
|
if err != nil || i < 0 || len(typeData) <= i {
|
||||||
ok = false
|
ok = false
|
||||||
c.addError(err)
|
c.addError(err)
|
||||||
return
|
return
|
||||||
@@ -232,7 +232,7 @@ func (c *Config) GetValue(key string, findByPath ...bool) (value any, ok bool) {
|
|||||||
item = typeData[i]
|
item = typeData[i]
|
||||||
case []any: // is array(load from file)
|
case []any: // is array(load from file)
|
||||||
i, err := strconv.Atoi(k)
|
i, err := strconv.Atoi(k)
|
||||||
if err != nil || len(typeData) <= i {
|
if err != nil || i < 0 || len(typeData) <= i {
|
||||||
ok = false
|
ok = false
|
||||||
c.addError(err)
|
c.addError(err)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -499,3 +499,25 @@ func TestConfig_Duration_getter(t *testing.T) {
|
|||||||
is.Eq(7*time.Second, c.Duration("missing", 7*time.Second))
|
is.Eq(7*time.Second, c.Duration("missing", 7*time.Second))
|
||||||
is.Eq(time.Duration(0), c.Duration("missing"))
|
is.Eq(time.Duration(0), c.Duration("missing"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestConfig_GetValue_negativeIndex(t *testing.T) {
|
||||||
|
is := assert.New(t)
|
||||||
|
|
||||||
|
c := New("test-neg")
|
||||||
|
err := c.LoadData(map[string]any{
|
||||||
|
"arr": []any{"a", "b", "c"},
|
||||||
|
})
|
||||||
|
is.NoErr(err)
|
||||||
|
|
||||||
|
// A negative slice index in a key path must not panic; it is simply
|
||||||
|
// treated as not found.
|
||||||
|
is.NotPanics(func() {
|
||||||
|
v, ok := c.GetValue("arr.-1")
|
||||||
|
is.False(ok)
|
||||||
|
is.Nil(v)
|
||||||
|
})
|
||||||
|
|
||||||
|
is.NotPanics(func() {
|
||||||
|
is.False(c.Exists("arr.-1"))
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in new issue
Block a user