Pin HTTP/RTSP version + method normalization to Locale.US (#16765)#16770
Merged
Conversation
`HttpVersion`, `RtspVersions` and `RtspMethods` all uppercase their
input via `String.toUpperCase()` without an explicit `Locale`. The JVM
default locale governs that call, and in Turkish (`tr_TR`) the ASCII
letter `'i'` uppercases to `'İ'` (U+0130), not `'I'`.
Concretely, on a Turkish-locale JVM:
- `RtspMethods.valueOf("describe")` produces `"DESCRİBE"` after the
uppercase, fails the cache lookup, falls through to
`HttpMethod.valueOf(...)`, and throws `IllegalArgumentException: Illegal
character in HTTP Method: 0x130` for what is otherwise a perfectly valid
RTSP method. Same for `"redirect"` (`REDİRECT`).
- `HttpVersion.valueOf("icap/1.0")` and the `(protocolName, major,
minor, ...)` constructor with `"icap"` end up with `protocolName()`
containing U+0130 instead of plain ASCII `'I'`. The version no longer
matches its own canonical form.
The protocol grammars are ASCII-only (RFC 7230 §2.6 / RFC 2326 §1.4), so
the uppercase has no business consulting the JVM locale.
Pin every `String.toUpperCase()` in these three classes to `Locale.US`:
-
`codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java`
- `(text, strict, keepAliveDefault)` constructor — line that normalizes
the supplied protocol string.
- `(protocolName, major, minor, keepAliveDefault, bytes)` constructor —
line that normalizes the supplied protocol name.
-
`codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.java`
- `valueOf(String name)` — line that normalizes the supplied RTSP method
name before the cache lookup.
-
`codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspVersions.java`
- `valueOf(String text)` — line that normalizes the supplied version
string before comparing to the cached `RTSP/1.0`.
`AsciiString.toUpperCase()` / `AsciiString.toLowerCase()` paths in the
same module are already ASCII-byte-level and not affected by this issue,
so they are left alone.
`codec-http/src/test/java/io/netty/handler/codec/http/HttpVersionParsingTest.java`:
| Change Point | Test |
|--------------|------|
| `HttpVersion.valueOf(text)` under Turkish locale |
`testLowercaseIotaProtocolNameUnderTurkishLocale` — installs
`Locale.setDefault(new Locale("tr","TR"))` (restored in `finally`) and
asserts `valueOf("icap/1.0")` returns a version whose `protocolName()`
is the ASCII `"ICAP"` and whose `text()` is `"ICAP/1.0"`. |
| `HttpVersion(protocolName, major, minor, ...)` under Turkish locale |
`testProtocolNameConstructorUnderTurkishLocale` — same locale setup,
asserts `new HttpVersion("icap", 1, 0, true)` ends up with
`protocolName()` = `"ICAP"` and `text()` = `"ICAP/1.0"`. |
New
`codec-http/src/test/java/io/netty/handler/codec/rtsp/RtspMethodsTest.java`:
| Change Point | Test |
|--------------|------|
| Cached uppercase lookup still works |
`valueOfReturnsCachedInstanceForUppercaseName` |
| Lowercase normalization under default locale |
`valueOfNormalizesLowercaseInputUnderUsLocale` |
| Lowercase normalization under Turkish locale |
`valueOfNormalizesLowercaseInputUnderTurkishLocale` — installs `tr_TR`,
calls `valueOf("describe")` and `valueOf("redirect")`, asserts both
resolve to the cached `DESCRIBE` / `REDIRECT` instances. |
All three Turkish-locale tests fail deterministically against the
unfixed code (one with a clean assertion failure, the `RtspMethods` one
with `IllegalArgumentException: Illegal character in HTTP Method:
0x130`) and pass after the `Locale.US` change.
Verification:
```
mvn -pl codec-http -am test -Dtest='HttpVersionParsingTest,RtspMethodsTest' -Dsurefire.failIfNoSpecifiedTests=false
```
- Behavior on en/CJK/most locales: unchanged (the previous
default-locale uppercase already produced ASCII).
- Behavior on Turkish (and other locales with non-ASCII case mappings,
e.g. Azerbaijani): RTSP method parsing and HTTP-derived version parsing
of lowercase or mixed-case inputs now succeed instead of throwing or
returning a U+0130-tainted result.
- API: no signature change. All callers continue to use
`valueOf(String)` / the existing constructors.
- Risk: bounded — `Locale.US` is the standard Netty pattern for
protocol-string normalization (see e.g.
`WebSocketClientHandshaker.java:761`).
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.
HttpVersion,RtspVersionsandRtspMethodsall uppercase theirinput via
String.toUpperCase()without an explicitLocale. The JVMdefault locale governs that call, and in Turkish (
tr_TR) the ASCIIletter
'i'uppercases to'İ'(U+0130), not'I'.Concretely, on a Turkish-locale JVM:
RtspMethods.valueOf("describe")produces"DESCRİBE"after theuppercase, fails the cache lookup, falls through to
HttpMethod.valueOf(...), and throwsIllegalArgumentException: Illegal character in HTTP Method: 0x130for what is otherwise a perfectly validRTSP method. Same for
"redirect"(REDİRECT).HttpVersion.valueOf("icap/1.0")and the(protocolName, major, minor, ...)constructor with"icap"end up withprotocolName()containing U+0130 instead of plain ASCII
'I'. The version no longermatches its own canonical form.
The protocol grammars are ASCII-only (RFC 7230 §2.6 / RFC 2326 §1.4), so
the uppercase has no business consulting the JVM locale.
Pin every
String.toUpperCase()in these three classes toLocale.US:codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java(text, strict, keepAliveDefault)constructor — line that normalizesthe supplied protocol string.
(protocolName, major, minor, keepAliveDefault, bytes)constructor —line that normalizes the supplied protocol name.
codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspMethods.javavalueOf(String name)— line that normalizes the supplied RTSP methodname before the cache lookup.
codec-http/src/main/java/io/netty/handler/codec/rtsp/RtspVersions.javavalueOf(String text)— line that normalizes the supplied versionstring before comparing to the cached
RTSP/1.0.AsciiString.toUpperCase()/AsciiString.toLowerCase()paths in thesame module are already ASCII-byte-level and not affected by this issue,
so they are left alone.
codec-http/src/test/java/io/netty/handler/codec/http/HttpVersionParsingTest.java:HttpVersion.valueOf(text)under Turkish localetestLowercaseIotaProtocolNameUnderTurkishLocale— installsLocale.setDefault(new Locale("tr","TR"))(restored infinally) andvalueOf("icap/1.0")returns a version whoseprotocolName()"ICAP"and whosetext()is"ICAP/1.0".HttpVersion(protocolName, major, minor, ...)under Turkish localetestProtocolNameConstructorUnderTurkishLocale— same locale setup,new HttpVersion("icap", 1, 0, true)ends up withprotocolName()="ICAP"andtext()="ICAP/1.0".New
codec-http/src/test/java/io/netty/handler/codec/rtsp/RtspMethodsTest.java:valueOfReturnsCachedInstanceForUppercaseNamevalueOfNormalizesLowercaseInputUnderUsLocalevalueOfNormalizesLowercaseInputUnderTurkishLocale— installstr_TR,valueOf("describe")andvalueOf("redirect"), asserts bothDESCRIBE/REDIRECTinstances.All three Turkish-locale tests fail deterministically against the
unfixed code (one with a clean assertion failure, the
RtspMethodsonewith
IllegalArgumentException: Illegal character in HTTP Method: 0x130) and pass after theLocale.USchange.Verification:
default-locale uppercase already produced ASCII).
e.g. Azerbaijani): RTSP method parsing and HTTP-derived version parsing
of lowercase or mixed-case inputs now succeed instead of throwing or
returning a U+0130-tainted result.
valueOf(String)/ the existing constructors.Locale.USis the standard Netty pattern forprotocol-string normalization (see e.g.
WebSocketClientHandshaker.java:761).