Files
gookit-goutil/PR.md
T
dualfroz 650669002f 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.
2026-09-04 09:10:31 +02:00

1.8 KiB

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:

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:

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:

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.