vendor: github.com/mattn/go-runewidth v0.0.24#7038
Conversation
- Optimize EastAsian RuneWidth with precomputed width table full diff: mattn/go-runewidth@v0.0.23...v0.0.24 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
Vendoring go-runewidth v0.0.24 introduces a precomputed East Asian width table and several fast-path optimisations. The overall approach is sound — the Unicode table merging logic and binary search are correct. Two findings worth noting are called out inline.
| } | ||
|
|
||
| func inWidthTable(r rune, t widthTable) (int, bool) { | ||
| if r < t[0].first { |
There was a problem hiding this comment.
[MEDIUM/LIKELY] inWidthTable has no empty-slice guard — potential panic
inWidthTable accesses t[0].first and t[len(t)-1].last on lines 167/170 without first checking len(t) > 0. If the function is ever called with an empty widthTable, it will panic with an index-out-of-range error.
In the current init() flow this cannot happen because eastAsianWidth is built from real, non-empty Unicode tables. However, the pre-existing inTable (for the table type) has the same pattern, so this appears to be an intentional design decision for this package. The risk is theoretical given that inWidthTable is unexported and only called with eastAsianWidth.
|
|
||
| // StringWidth return width as you can see | ||
| func (c *Condition) StringWidth(s string) (width int) { | ||
| if len(s) == 1 { |
There was a problem hiding this comment.
[MEDIUM/CONFIRMED] StringWidth single-byte fast-path changes EastAsian width for invalid UTF-8 bytes
The new len(s) == 1 fast-path (lines 290-295) returns 1 for any byte in the range 0x80–0xFF:
if len(s) == 1 {
b := s[0]
if b < 0x20 || b == 0x7F {
return 0
}
return 1 // ← returns 1 for bytes 0x80-0xFF too
}Behavioral regression in EastAsian mode: Under the old code, a lone byte ≥ 0x80 (invalid UTF-8) was decoded by utf8.DecodeRuneInString as U+FFFD (REPLACEMENT CHARACTER, RuneError). U+FFFD is in the doublewidth table, so c.RuneWidth(U+FFFD) returned 2 in EastAsian mode. The new fast-path hard-codes 1 for all bytes 0x80–0xFF, bypassing RuneWidth entirely.
Impact: Any caller using EastAsian mode (c.EastAsianWidth = true) who passes a single-byte invalid UTF-8 sequence will now get width 1 instead of 2. The practical impact is low (malformed UTF-8 is an edge case), but it is a silent breaking change for that code path.
3 participants
full diff: mattn/go-runewidth@v0.0.23...v0.0.24
- What I did
- How I did it
- How to verify it
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)