fix(stringutil): operate on runes instead of bytes in Truncate (#22388)#22467
fix(stringutil): operate on runes instead of bytes in Truncate (#22388)#22467johnstcn merged 1 commit intorelease/2.30from
Conversation
Documentation CheckNo Changes NeededThis PR is a purely internal bug fix — it corrects multi-byte UTF-8 handling in Automated review via Coder Tasks |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where stringutil.Truncate was operating on byte indices rather than rune (Unicode code point) counts, causing multi-byte UTF-8 characters (e.g. em dash —, CJK characters, emoji) to be split at truncation boundaries, producing invalid byte sequences that PostgreSQL rejects. It also introduces a safe Capitalize utility to replace raw s[:1] byte-slicing for capitalizing display names.
Changes:
Truncatenow converts the string to a[]runeslice and counts/slices by rune index instead of byte index, making it safe for all valid UTF-8 inputs.- A new
Capitalize(s string) stringhelper is introduced instrutil, usingutf8.DecodeRuneInStringto safely upper-case the first rune of a string. - Both
generateFromPromptandgenerateFromAnthropicintaskname.goare updated to usestrutil.Capitalizeinstead of the unsafestrings.ToUpper(s[:1]) + s[1:]pattern.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
coderd/util/strings/strings.go |
Core fix: Truncate now works on rune slices; new Capitalize helper added |
coderd/util/strings/strings_test.go |
Tests for multi-byte truncation (CJK, emoji, mixed) and new TestCapitalize |
coderd/taskname/taskname.go |
generateFromPrompt and generateFromAnthropic now use strutil.Capitalize |
coderd/taskname/taskname_test.go |
New FromPromptMultiByte test exercising multi-byte capitalize path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| taskName := taskname.Generate(ctx, testutil.Logger(t), "über cool feature") | ||
|
|
||
| require.NoError(t, codersdk.NameValid(taskName.Name)) | ||
| require.True(t, len(taskName.DisplayName) > 0) |
There was a problem hiding this comment.
The require.True(t, len(taskName.DisplayName) > 0) assertion on this line is redundant, since the require.Equal(t, "Über cool feature", taskName.DisplayName) assertion on the next line already implies the display name is non-empty. The require.True check provides no additional safety and can be removed to keep the test clean and avoid misleading readers about the test's purpose.
| require.True(t, len(taskName.DisplayName) > 0) |
Fixes #22375
Updates
stringutil.Truncateto properly handle multi-byte UTF-8 characters.Adds tests for multi-byte truncation with word boundary.
Created by Mux using Opus 4.6
(cherry picked from commit 0cfa037)