fix(strutil): keep center-aligned Resize within the requested length
The center branch of resize() added the odd-padding space whenever the input was non-empty, because the parity check omitted the source length (wantLen - padLn*2 instead of wantLen - sLen - padLn*2). This made Resize, Align and their utf8 variants overflow wantLen by one for every non-empty string with an even amount of padding. Subtract sLen so the extra space is only added when the total padding is actually odd.
This commit is contained in:
1 parent
b17a55589a
commit
e2e5b5495f
3 files changed
+99
-1
No files matched your search
@@ -0,0 +1,73 @@
|
|||||||
|
# fix(strutil): center-aligned Resize/Align overflows the requested length
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`strutil.Resize` / `strutil.Align` (and their utf8 variants) with
|
||||||
|
`comdef.Center` alignment produce strings longer than the requested length for
|
||||||
|
almost every non-empty input:
|
||||||
|
|
||||||
|
```go
|
||||||
|
strutil.Resize("ab", 6, comdef.Center) // " ab " (len 7), want " ab " (len 6)
|
||||||
|
strutil.Resize("ab", 4, comdef.Center) // " ab " (len 5), want " ab " (len 4)
|
||||||
|
strutil.Resize("abc", 7, comdef.Center) // " abc "(len 8), want " abc "(len 7)
|
||||||
|
```
|
||||||
|
|
||||||
|
The extra space that centers an odd amount of padding is added unconditionally
|
||||||
|
whenever the input is non-empty, not only when the padding is actually odd.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`strutil/padding.go`, `resize`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
if align == comdef.Center {
|
||||||
|
padLn := (wantLen - sLen) / 2
|
||||||
|
if diff = wantLen - padLn*2; diff > 0 { // wrong: missing "- sLen"
|
||||||
|
s += " "
|
||||||
|
}
|
||||||
|
...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The condition is meant to detect an odd amount of total padding
|
||||||
|
(`wantLen - sLen`). Simplifying the buggy expression:
|
||||||
|
|
||||||
|
```
|
||||||
|
wantLen - 2*padLn = wantLen - 2*floor((wantLen-sLen)/2)
|
||||||
|
= sLen + ((wantLen - sLen) mod 2)
|
||||||
|
```
|
||||||
|
|
||||||
|
so `diff > 0` is true whenever `sLen > 0`, regardless of the padding parity.
|
||||||
|
The result is one space too long for every non-empty string with even padding.
|
||||||
|
|
||||||
|
The existing `TestResize("title", 20, PosMiddle)` case did not catch this
|
||||||
|
because its padding (15) is odd, where the extra space is legitimately needed.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Compare against the actual leftover padding by subtracting `sLen`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
if diff = wantLen - sLen - padLn*2; diff > 0 {
|
||||||
|
s += " "
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This adds the extra space only when `(wantLen - sLen)` is odd, keeping the odd
|
||||||
|
case (e.g. the "title" test) unchanged.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added `TestResize_center_length` in `strutil/padding_test.go` asserting both the
|
||||||
|
exact result and `len(result) == wantLen` for even- and odd-padding cases.
|
||||||
|
|
||||||
|
- Gate: `go test ./strutil/ -run TestResize` -> ok (PASS)
|
||||||
|
- `go vet ./strutil/` -> exit 0
|
||||||
|
- `go build ./...` -> exit 0
|
||||||
|
|
||||||
|
Counterfactual: reverting the change makes the new test fail with
|
||||||
|
`actual: int(7)` / `actual: " ab "` vs the expected length 6 / `" ab "`.
|
||||||
|
|
||||||
|
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.
|
||||||
+3
-1
@@ -164,7 +164,9 @@ func resize(s string, sLen, wantLen int, align comdef.Align, cutOverflow bool) s
|
|||||||
|
|
||||||
if align == comdef.Center {
|
if align == comdef.Center {
|
||||||
padLn := (wantLen - sLen) / 2
|
padLn := (wantLen - sLen) / 2
|
||||||
if diff = wantLen - padLn*2; diff > 0 {
|
// add a single extra space only when the total padding is odd,
|
||||||
|
// otherwise the result overflows the requested length.
|
||||||
|
if diff = wantLen - sLen - padLn*2; diff > 0 {
|
||||||
s += " "
|
s += " "
|
||||||
}
|
}
|
||||||
padStr := string(RepeatBytes(' ', padLn))
|
padStr := string(RepeatBytes(' ', padLn))
|
||||||
|
|||||||
@@ -38,6 +38,29 @@ func TestPadding(t *testing.T) {
|
|||||||
assert.Eq(t, "hi,你好000", strutil.Utf8Padding(s, "0", 10, strutil.PosRight))
|
assert.Eq(t, "hi,你好000", strutil.Utf8Padding(s, "0", 10, strutil.PosRight))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestResize_center_length(t *testing.T) {
|
||||||
|
// Center alignment must always produce a string of exactly the requested
|
||||||
|
// length; the extra padding space is only needed when the total padding
|
||||||
|
// (wantLen - len(s)) is odd.
|
||||||
|
tests := []struct {
|
||||||
|
give string
|
||||||
|
len int
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{"ab", 6, " ab "}, // even padding (4): 2 + 2
|
||||||
|
{"ab", 4, " ab "}, // even padding (2): 1 + 1
|
||||||
|
{"ab", 5, " ab "}, // odd padding (3): 1 + 2
|
||||||
|
{"abc", 7, " abc "}, // even padding (4): 2 + 2
|
||||||
|
{"a", 4, " a "}, // odd padding (3): 1 + 2
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
got := strutil.Resize(tt.give, tt.len, comdef.Center)
|
||||||
|
assert.Eq(t, tt.len, len(got), "Resize(%q,%d,Center)=%q", tt.give, tt.len, got)
|
||||||
|
assert.Eq(t, tt.want, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestResize(t *testing.T) {
|
func TestResize(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
want, give string
|
want, give string
|
||||||
|
|||||||
Reference in new issue
Block a user