Keep WordUtils.abbreviate from splitting a surrogate pair#761
Draft
vjsai wants to merge 1 commit into
Draft
Conversation
WordUtils.abbreviate() cuts the string at raw char offsets, so an upper
limit that lands between a high surrogate and its trailing low surrogate
leaves an unpaired surrogate at the end of the result.
For example, WordUtils.abbreviate("😀😀😀", 0, 3, "")
returned one emoji followed by a lone high surrogate instead of just the
emoji.
Back the cut off by one when it would split a pair, in both the
no-space-found and the space-found branches, mirroring the existing
handling in WordUtils.wrap() and StringUtils.abbreviate().
Contributor
|
We should document that behavior: This does count characters not code units, it might return less characters to avoid Splitting surrogates, but it does not measure the length in codepoints (or something like that?). maybe add examples to the JavaDoc? Since you mentioned emojis, what about extended grapheme clusters? having said that, a byte or codepoint length semantic might be interesting as well. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WordUtils.abbreviate()slices the input at rawcharoffsets, so an upper limit that lands between a high surrogate and its trailing low surrogate leaves an unpaired surrogate at the end of the result.Reproduction
The string is 6
chars long and contains no space, soabbreviateappendsstr[0, 3)— which is one whole emoji plus the high surrogate of the second one."😀\uD83D"— one emoji followed by a lone high surrogate (renders as😀?)"😀"—"😀"Both call sites are affected: the no-space-found branch (cut at
upper) and the space-found branch (cut atmin(index, upper)).Fix
When the cut would split a pair, back it off by one
charso the whole pair is dropped, and the result stays within the requested upper limit. This is the same rule the surrounding code already applies elsewhere —WordUtils.wrap()keeps a pair whole at a hard break, andStringUtils.abbreviate()in Commons Lang backs its head cut off a pair boundary via asplitsSurrogatePairhelper. This PR adds the same private helper toWordUtils, so the class now handles surrogate pairs consistently acrosswrap,initials, andabbreviate.This is a follow-on to the same class of fix already applied in this repo to
WordUtils.wrap(#755) andTextStringBuilder.reverse(#756);abbreviatewas missed by that sweep.Test
WordUtilsTest.testAbbreviateSurrogatePairs()covers a cut inside a pair (with and withoutappendToEnd), a cut short of a space, and a limit that already falls between two pairs (which must be left alone).The test fails before the change and passes after:
After the fix, the default
mvngoal is green on JDK 17 (clean verify apache-rat:check japicmp:cmp checkstyle:check pmd:check spotbugs:check javadoc:javadoc):Behavior for strings without surrogate pairs is unchanged, and the change is binary compatible (the new helper is private) —
japicmpreports no issues.Claude Code (Anthropic) was used to help locate the bug, draft the fix and the test, and run the build. I reviewed and verified the change myself: I confirmed the failing assertion before the fix, confirmed the full default-goal build passes after it, and checked the fix against the existing surrogate-pair handling in
WordUtils.wrapandStringUtils.abbreviate. I understand and take responsibility for the contents of this PR.mvn; that'smvnon the command line by itself.