Upstream gookit/goutil state of the touched files before 949a07a2cc
This commit is contained in:
1 parent
d6c91f3cdd
commit
b17a55589a
3 files changed
+382
-74
No files matched your search
@@ -1,74 +0,0 @@
|
||||
# fix(strutil): BaseConv family returns "0" for zero instead of empty string
|
||||
|
||||
## Problem
|
||||
|
||||
Converting the value zero through the base-conversion helpers returns an empty
|
||||
string instead of `"0"`:
|
||||
|
||||
```go
|
||||
strutil.BaseConv("0", 10, 16) // "" , want "0"
|
||||
strutil.BaseConv("0", 16, 10) // "" , want "0"
|
||||
strutil.Base10Conv("0", 62) // "" , want "0"
|
||||
strutil.BaseConvInt(0, 64) // "" , want "0"
|
||||
```
|
||||
|
||||
This is also internally inconsistent: `BaseConvInt` returns the correct `"0"`
|
||||
for small bases because it delegates to `strconv.FormatUint`/`big.Int.Text`,
|
||||
but returns `""` for bases in `(62, 64]`, which go through the template path:
|
||||
|
||||
```go
|
||||
strutil.BaseConvInt(0, 16) // "0" (strconv path)
|
||||
strutil.BaseConvInt(0, 64) // "" (template path)
|
||||
```
|
||||
|
||||
## Root cause
|
||||
|
||||
`strutil/convbase.go`, `BaseConvIntByTpl`:
|
||||
|
||||
```go
|
||||
func BaseConvIntByTpl(dec uint64, toBase string) string {
|
||||
var res string
|
||||
toLen := uint64(len(toBase))
|
||||
for dec > 0 { // never runs when dec == 0
|
||||
res = string(toBase[dec%toLen]) + res
|
||||
dec /= toLen
|
||||
}
|
||||
return res // returns "" for dec == 0
|
||||
}
|
||||
```
|
||||
|
||||
The loop condition `dec > 0` is never entered when `dec == 0`, so the function
|
||||
returns the zero-value empty string. Every public entry point that reaches the
|
||||
template path (`BaseConvIntByTpl`, `BaseConvByTpl`, `BaseConv`, `Base10Conv`,
|
||||
`BaseConvInt` for base > 62) inherits the defect.
|
||||
|
||||
## Fix
|
||||
|
||||
Handle the zero case explicitly by emitting the zero digit of the target base
|
||||
(the first character of the template), and guard against an empty template to
|
||||
avoid an out-of-range index:
|
||||
|
||||
```go
|
||||
if toLen == 0 {
|
||||
return res
|
||||
}
|
||||
if dec == 0 {
|
||||
return string(toBase[0])
|
||||
}
|
||||
```
|
||||
|
||||
## Test
|
||||
|
||||
Added `TestBaseConv_zero` in `strutil/convbase_test.go` covering `BaseConv`,
|
||||
`Base10Conv`, `BaseConvInt` (small and large bases) and `BaseConvIntByTpl`.
|
||||
|
||||
- Gate: `go test ./strutil/ -run 'TestBaseConv|TestBase10Conv'` -> ok (PASS)
|
||||
- `go vet ./strutil/` -> exit 0
|
||||
- `go build ./...` -> exit 0
|
||||
|
||||
Counterfactual: reverting the change in `BaseConvIntByTpl` makes the new test
|
||||
fail with `actual: string("")` vs `expect: string("0")`.
|
||||
|
||||
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 over 60 iterations); it is unrelated to this change.
|
||||
Reference in new issue
Block a user