Skip to content

Encode form and digest input without CharBuffer - #2385

Open
pavel-ptashyts wants to merge 6 commits into
AsyncHttpClient:mainfrom
maygemdev:claude/form-digest-getbytes-35d803
Open

pavel-ptashyts wants to merge 6 commits into
AsyncHttpClient:mainfrom
maygemdev:claude/form-digest-getbytes-35d803

Conversation

@pavel-ptashyts

@pavel-ptashyts pavel-ptashyts commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • StringUtils.charSequence2Bytes / charSequence2ByteBuffer no longer run charset.encode(CharBuffer.wrap(cs)). They are on the path of every url-encoded form body (HttpUtils.urlEncodeFormParams) and every Digest hash (Realm), and of the Digest Authorization header (AuthenticatorUtils).
  • The bytes are the same as before. The encoding step takes 25 to 50% less time on these paths and allocates the same or less, except for UTF-8 Digest input with non-ASCII credentials or realm.
  • No public signature changes. Follows Encode setBody(String) bodies with String.getBytes #2371, which did the same for setBody(String).

Problem

CharBuffer.wrap(CharSequence) returns a buffer with no backing array, so the JDK encoder walks it char by char through virtual get/put calls and misses the array loop and the String intrinsics. All three callers pass a pooled StringBuilder, on the caller thread, once per request attempt (form) or three times per Digest response.

Change

charSequence2Bytes:

  • a String goes through String.getBytes(charset);
  • a StringBuilder (or any other CharSequence) in UTF-8, ISO-8859-1 or US-ASCII is copied char by char into an exact-size byte[] while every char is below a limit under which the charset maps a char to that byte: U+0100 for ISO-8859-1, U+0080 for UTF-8 and US-ASCII. On the first char at or above the limit it falls back to toString().getBytes(charset);
  • any other charset goes through toString().getBytes(charset).

charSequence2ByteBuffer wraps that array. A url-encoded form is always ASCII (everything else is percent-encoded), so form bodies never reach the fallback. The Digest hash input (Realm) and the Digest Authorization header (AuthenticatorUtils.computeRealmHeader) reach it in UTF-8 (charset=UTF-8) when the credentials, realm or uri hold a char at or above U+0080, and in ISO-8859-1, the default Digest charset, only when they hold a char ISO-8859-1 cannot map (at or above U+0100), so Latin-1 credentials such as accented letters stay on the loop.

StringBodyEncodeBenchmark.charsetEncode from #2371 measured the old path through charSequence2ByteBuffer(String, UTF_8), which now goes to getBytes too; it now calls UTF_8.encode(CharBuffer.wrap(body)) directly, so it keeps measuring the old path.

String.getBytes(charset) and Charset.encode both replace malformed and unmappable input with the encoder's replacement and write the same byte order marks; #2371 checked this for every charset. The new tests check it again here:

  • StringUtilsTest: both methods against charset.encode(CharBuffer.wrap(cs)) for every charset the JDK can encode, 17 edge inputs (unpaired surrogates, supplementary chars, BOM, replacement char, NUL/DEL/U+0080/U+00FF/U+0100, and U+0080 alone and between ASCII chars, so each limit is checked on its own: with > 0x80, or with the ISO-8859-1 limit at 0x101, the test fails), and four kinds of CharSequence: String, StringBuilder, a StringBuilder with a UTF-16 coder (the state a pooled builder keeps after it once held a char above U+00FF) and CharBuffer.
  • HttpUtilsTest: urlEncodeFormParams against US_ASCII.encode of the same url-encoded form, in UTF-8, ISO-8859-1 and GBK, with non-ASCII, a supplementary char, unpaired surrogates and a null value, from a pooled builder with a UTF-16 coder.
  • RealmTest: the Digest response for Cyrillic user, a password with a lone surrogate, Euro sign and emoji, and a non-ASCII realm, in ISO-8859-1 (default) and UTF-8 (charset=UTF-8), against MD5 over charset.encode bytes.

The returned ByteBuffer now has capacity equal to its limit. Before, it was whatever the encoder had allocated (1.1 bytes per char for UTF-8, more after growth). That capacity is not specified by Charset.encode, and every caller reads only position to limit.

An alternative, sb.toString().getBytes(charset) for everything, is faster still but copies the builder into a String first, which roughly doubles allocation per form body against main (numbers below), so it was not taken.

Benchmark

FormDigestEncodeBenchmark (new) calls production code only, so the same class runs against main and this branch. JDK 17, -f 2 -wi 4 -i 5 -prof gc, three rounds alternating main and branch; the median round is shown.

benchmark ns/op main -> branch B/op main -> branch
encodeFormBuilder: 6,691-byte url-encoded form, US-ASCII 7,855 -> 3,871 6,768 -> 6,768
encodeDigestBuilder: 131-char ASCII Digest input, ISO-8859-1 109 -> 82 208 -> 208
encodeUtf8DigestBuilder: Cyrillic user:realm:password, UTF-8 405 -> 201 360 -> 584
formSmall: urlEncodeFormParams, 4 params 658 -> 439 224 -> 168
formLarge: urlEncodeFormParams, 256 params within noise ~6,810 -> 6,768
digestAscii: Realm.Builder.build(), ASCII credentials within noise ~2,270 -> ~2,000
digestUtf8: Realm.Builder.build(), Cyrillic credentials, UTF-8 within noise ~2,544 -> ~2,484

The end-to-end form and Digest timings are dominated by url-encoding and by SecureRandom for the cnonce. On this shared host they moved by more than the change itself between runs of the same build (main digestAscii measured anywhere from 1.8 to 9.2 us), so only their B/op is reported. B/op is deterministic up to a few dozen bytes.

encodeUtf8DigestBuilder allocates more: the loop's array is discarded at the first non-ASCII char, and getBytes(UTF_8) on a UTF-16 string uses a 3-bytes-per-char scratch array, the same trade-off as in #2371. In ISO-8859-1, the default Digest charset, the loop covers every char up to U+00FF, so the fallback, with its discarded array and String copy, is left to credentials, realm or uri holding a char ISO-8859-1 cannot map (it becomes ?, as before). Either way this is short Digest input only; ASCII Digest input and every form body stay on the loop. The ISO-8859-1 limit was raised from U+0080 to U+0100 after review; the numbers above were measured before that and do not touch it (encodeDigestBuilder is ASCII).

Loop against toString().getBytes (exploratory, not committed), the encoding step alone, UTF-8, -f 2 -wi 4 -i 5 -prof gc, ns/op and B/op:

input old Charset.encode this PR (loop) toString().getBytes
ASCII, 6,691 chars 12,828 / 7,488 2,905 / 6,768 1,114 / 13,504
ASCII, 6,691 chars, UTF-16-coded builder 12,454 / 7,488 1,693 / 6,768 1,034 / 13,504
ASCII, 128 chars 319 / 272 54 / 200 36 / 368
ASCII, 128 chars, UTF-16-coded builder 273 / 272 41 / 200 31 / 368
128 chars ending in U+00E9, UTF-16-coded builder 302 / 272 136 / 768 125 / 648

The host was shared with other builds, so take the timings as indicative.

Second-agent review

Both reviewers ran read-only, in fresh sessions, on git diff upstream/main...HEAD.

  • Guided reviewer, checklist AGENTS.md: Codex gpt-6-astra, reasoning effort medium.
  • Independent reviewer, no checklist: Claude Code claude-opus-5-5, effort medium, tools Read/Grep/Glob only.
round commit guided independent
1 46ff4250d 1 minor: raw non-ASCII literals in the new tests and bench. Fixed in 665746074 (Java unicode escapes). 1 major: the same. Fixed in 665746074.
2 665746074 1 minor: the returned buffer's capacity now equals its limit. Not changed: the capacity of Charset.encode's result is unspecified and every caller reads only position to limit (see Change). 1 minor: show the loop is not slower than toString().getBytes, including a UTF-16-coded builder. Measured (table above); the loop is kept for its lower allocation.
3 665746074, with the author's answers to rounds 1 and 2 No findings. No findings.
4 a763e6bc9, after the maintainer review below No findings. 2 nits: (a) ISO-8859-1 maps every char below U+0100 to that byte, so Latin-1 Digest input need not take the fallback. Fixed in 253d17b3c (limit U+0100 for ISO-8859-1). (b) The description left out AuthenticatorUtils among the paths that reach the fallback. Fixed in this description.
5 253d17b3c No findings. 1 nit: the digestUtf8 bench Javadoc said HA1 is not ASCII; it is the HA1 input. Fixed in 9dd49bd8b.
6 9dd49bd8b No findings. No findings.

Maintainer review on 665746074, addressed in a763e6bc9 and 253d17b3c:

  • c > 0x80 passed every test: U+0080 alone and a U+0080 b were added to StringUtilsTest; the mutant now fails.
  • StringBodyEncodeBenchmark.charsetEncode measured getBytes through charSequence2ByteBuffer: it now calls UTF_8.encode(CharBuffer.wrap(body)).
  • The fallback also runs for ISO-8859-1, the default Digest charset: it now runs there only for chars ISO-8859-1 cannot map (round 4 (a)), and the description says so.

AI disclosure

Claude Code on behalf of Pavel Ptashyts. The commits include Co-Authored-By per AGENTS.md.

Test plan

  • StringUtilsTest, HttpUtilsTest, RealmTest, AuthenticatorUtilsTest, DigestAuthTest, DigestAuthRfc7616Test, ParamEncodingTest: 94 tests pass.
  • mvnw.cmd -B -ntp -Dgpg.skip=true clean verify on JDK 17 (no JDK 11 on the host): 1,998 tests, 0 failures, 0 errors, 35 skipped, on 665746074 and again on 253d17b3c; Revapi passed. 9dd49bd8b changes only a benchmark Javadoc (JMH sources are not built by Maven). Source/target is 11. JDK 11 coverage comes from CI.
  • Mutation checks: c > 0x80 and an ISO-8859-1 limit of 0x101 each make StringUtilsTest fail.
  • StringBodyEncodeBenchmark and FormDigestEncodeBenchmark compile with javac --release 11.

Claude Code on behalf of Pavel Ptashyts

🤖 Generated with Claude Code

pavel-ptashyts and others added 3 commits November 7, 2026 20:13
StringUtils.charSequence2ByteBuffer ran charset.encode(CharBuffer.wrap(cs)).
The wrapped CharBuffer has no backing array, so the encoder walks it
char by char and starts from an estimated buffer size. Every
url-encoded form body (HttpUtils.urlEncodeFormParams) and every Digest
hash input (Realm) goes through it, on the caller thread.

charSequence2Bytes now copies an ASCII StringBuilder straight into a
byte array for UTF-8, ISO-8859-1 and US-ASCII, which all map a char
below U+0080 to that byte. Any other input or charset goes through
String.getBytes(charset), which AsyncHttpClient#2371 showed to produce the same bytes
as Charset.encode, replacements and byte order marks included.
charSequence2ByteBuffer wraps that array. Signatures are unchanged.

New tests check the bytes against Charset.encode for every charset the
JDK can encode, for String, StringBuilder and CharBuffer input, and on
the form and Digest paths with non-ASCII and unpaired surrogates.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
End-to-end form and Digest timings are dominated by url-encoding and
SecureRandom, so the change under test is lost in their noise. Add arms
that run only charSequence2ByteBuffer on the builders those paths
produce, including a non-ASCII UTF-8 one for the fallback.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AGENTS.md requires ASCII-only repository content. The new test and
benchmark strings carried raw non-ASCII characters; write them as
Java unicode escapes instead. The strings are unchanged.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pavel-ptashyts and others added 3 commits November 10, 2026 07:18
The only input holding U+0080 also had a later char at or above
U+0100, so the whole string took the fallback and a loop that let
U+0080 through as a single byte still passed. U+0080 on its own and
between ASCII chars now pin the boundary of the ASCII copy.

StringBodyEncodeBenchmark.charsetEncode stood for the old
Charset.encode path by calling charSequence2ByteBuffer with a String,
which now goes to String.getBytes as well. It calls
UTF_8.encode(CharBuffer.wrap(body)) directly so it keeps measuring
the old path.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ISO-8859-1 is the default Digest charset, and the Digest hash input
and the Authorization header were taking the String copy fallback
for any char from U+0080 to U+00FF, such as an accented letter in
the user name or realm. ISO-8859-1 maps every char below U+0100 to
that byte, so the copy loop now runs up to U+0100 for it and up to
U+0080 for UTF-8 and US-ASCII, which leaves the fallback to input
that has chars ISO-8859-1 cannot map. StringUtilsTest already holds
U+00FF and U+0100 next to each other for every charset.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HA1 is a hex digest and always ASCII; what digestUtf8 makes non-ASCII
is the user:realm:password input hashed into it, which is the input
that takes the String copy fallback.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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