Skip to content

Pad BMP code points to four hex digits in UnicodeEscaper.toUtf16Escape - #1858

Merged
garydgregory merged 1 commit into
apache:masterfrom
alhudz:unicodeescaper-utf16-bmp-padding
Nov 3, 2026
Merged

garydgregory merged 1 commit into
apache:masterfrom
alhudz:unicodeescaper-utf16-bmp-padding

Conversation

@alhudz

@alhudz alhudz commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

UnicodeEscaper.toUtf16Escape(int) is the protected hook documented since 3.2 as returning the \uXXXX form. Its BMP branch is "\\u" + hex(codePoint), and hex() is Integer.toHexString, so any code point below U+1000 comes back short:

new UnicodeEscaper().toUtf16Escape('A');   // \u41, expected A
new UnicodeEscaper().toUtf16Escape('\n');  // \uA,  expected \u000A
new UnicodeEscaper().toUtf16Escape(0);     // \u0,  expected \u0000

No \u reader decodes those (UnicodeUnescaper passes \u41 through unchanged), while translate() writes the same code point padded through HEX_DIGITS, so the hook and the translator disagree on BMP input. JavaUnicodeEscaper overrides the hook with the surrogate-pair form only and indexes surrogatePair[1] unconditionally, so new JavaUnicodeEscaper(0, Integer.MAX_VALUE, true).toUtf16Escape('A') throws ArrayIndexOutOfBoundsException.

The fix left-pads the BMP hex to four digits inside toUtf16Escape, next to the surrogate-pair branch from #1726, and drops the JavaUnicodeEscaper override, which that PR made redundant: the base method now emits the same surrogate-pair form, so subclasses and direct callers get a well-formed escape for every code point. Output of translate(), and therefore of StringEscapeUtils.escapeJava, escapeEcmaScript and escapeJson, is byte for byte unchanged; translate() only reaches the hook for supplementary code points, where both forms were already identical. UnicodeEscaperTest.testToUtf16EscapeBmp and JavaUnicodeEscaperTest.testToUtf16EscapeBmp fail before the change (\u0 and the ArrayIndexOutOfBoundsException) and pass after.

Following up on the question on #1726: the Commons Text UnicodeEscaper.toUtf16Escape still has the pre-#1726 body ("\\u" + hex(codePoint) for every code point), so it has both the supplementary-code-point defect and this padding one; JavaUnicodeEscaper there has the same BMP ArrayIndexOutOfBoundsException. I will raise the same change on Commons Text separately.

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

The BMP branch of the protected toUtf16Escape hook returned "\\u" +
Integer.toHexString, so code points below U+1000 produced short escapes
such as \u41 that no \u reader decodes, while translate() pads the same
code point through HEX_DIGITS. JavaUnicodeEscaper overrode the hook with
the surrogate-pair form only and threw ArrayIndexOutOfBoundsException
for every BMP code point.

Left-pad the BMP hex to four digits and drop the JavaUnicodeEscaper
override, which is redundant since the base method emits the
surrogate-pair form itself. translate() output is unchanged.
@garydgregory
garydgregory merged commit 085c2dd into apache:master Nov 3, 2026
22 of 23 checks passed
@garydgregory

Copy link
Copy Markdown
Member

@alhudz thanks, merged 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants