fix(strutil): clamp Substr stop index to avoid out-of-range panic
Substr panicked with a slice-bounds error when a negative length trimmed more runes than remained after pos, because stopIdx (strLn + length) could fall before pos. Clamp stopIdx to pos so an over-long negative trim returns an empty string, matching the existing pos >= strLn guard.
This commit is contained in:
1 parent
1ffc0f390b
commit
650669002f
3 files changed
+76
No files matched your search
@@ -0,0 +1,64 @@
|
|||||||
|
# fix(strutil): prevent Substr panic on out-of-range negative length
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`strutil.Substr` panics with a slice-bounds error when a negative length trims
|
||||||
|
more characters than remain after `pos`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
strutil.Substr("hello", 4, -3) // panic: slice bounds out of range [4:2]
|
||||||
|
strutil.Substr("hello", 3, -5) // panic
|
||||||
|
strutil.Substr("abcDEF123", 8, -3) // panic
|
||||||
|
```
|
||||||
|
|
||||||
|
A negative `length` means "stop that many runes before the end of the string"
|
||||||
|
(this is the documented, tested behaviour, e.g. `Substr("abcDEF123", 2, -3) ==
|
||||||
|
"cDEF"`). When the resulting stop index falls before `pos`, the final slice
|
||||||
|
`runes[pos:stopIdx]` has `low > high` and panics.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`strutil/split.go`, `Substr`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
stopIdx := pos + length
|
||||||
|
if length == 0 || stopIdx > strLn {
|
||||||
|
stopIdx = strLn
|
||||||
|
} else if length < 0 {
|
||||||
|
stopIdx = strLn + length // can become < pos
|
||||||
|
}
|
||||||
|
return string(runes[pos:stopIdx])
|
||||||
|
```
|
||||||
|
|
||||||
|
For example `Substr("hello", 4, -3)`: `strLn == 5`, `pos == 4`, so
|
||||||
|
`stopIdx = 5 + (-3) = 2`, and `runes[4:2]` panics.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Clamp `stopIdx` to `pos` so an over-long negative trim yields an empty string
|
||||||
|
instead of panicking:
|
||||||
|
|
||||||
|
```go
|
||||||
|
if stopIdx < pos {
|
||||||
|
stopIdx = pos
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This is consistent with the existing out-of-range guard `if pos >= strLn` which
|
||||||
|
also returns an empty string.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Extended `TestSubstr` in `strutil/split_test.go` with the three panicking cases,
|
||||||
|
each asserting an empty-string result.
|
||||||
|
|
||||||
|
- Gate: `go test ./strutil/ -run 'TestSubstr$'` -> ok (PASS)
|
||||||
|
- `go vet ./strutil/` -> exit 0
|
||||||
|
- `go build ./...` -> exit 0
|
||||||
|
|
||||||
|
Counterfactual: reverting the clamp makes `TestSubstr` panic with
|
||||||
|
`slice bounds out of range [4:2]`.
|
||||||
|
|
||||||
|
Note: `TestDateSNv3/dateLen4_base62` in `strutil/gensn_test.go` is flaky and
|
||||||
|
already fails on the unmodified base commit (it asserts uniqueness of randomly
|
||||||
|
generated serial numbers); it is unrelated to this change.
|
||||||
@@ -166,6 +166,12 @@ func Substr(s string, pos, length int) string {
|
|||||||
stopIdx = strLn + length
|
stopIdx = strLn + length
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A negative length can move stopIdx before pos when it trims more than
|
||||||
|
// the remaining substring; clamp to pos to avoid an out-of-range slice.
|
||||||
|
if stopIdx < pos {
|
||||||
|
stopIdx = pos
|
||||||
|
}
|
||||||
|
|
||||||
return string(runes[pos:stopIdx])
|
return string(runes[pos:stopIdx])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -120,6 +120,12 @@ func TestSubstr(t *testing.T) {
|
|||||||
assert.Eq(t, "", strutil.Substr("abcDEF", 23, 5))
|
assert.Eq(t, "", strutil.Substr("abcDEF", 23, 5))
|
||||||
assert.Eq(t, "cDEF12", strutil.Substr("abcDEF123", 2, -1))
|
assert.Eq(t, "cDEF12", strutil.Substr("abcDEF123", 2, -1))
|
||||||
assert.Eq(t, "cDEF", strutil.Substr("abcDEF123", 2, -3))
|
assert.Eq(t, "cDEF", strutil.Substr("abcDEF123", 2, -3))
|
||||||
|
|
||||||
|
// negative length trimming more than the remaining substring must not
|
||||||
|
// panic; it should yield an empty string.
|
||||||
|
assert.Eq(t, "", strutil.Substr("hello", 4, -3))
|
||||||
|
assert.Eq(t, "", strutil.Substr("hello", 3, -5))
|
||||||
|
assert.Eq(t, "", strutil.Substr("abcDEF123", 8, -3))
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSplitInlineComment(t *testing.T) {
|
func TestSplitInlineComment(t *testing.T) {
|
||||||
|
|||||||
Reference in new issue
Block a user