Dse netty 4.1.137 - #43
Draft
emerkle826 wants to merge 65 commits into
Draft
Conversation
netty#16814) Motivation: Subclasses of `SingleThreadEventExecutor` can silently take down the event-loop thread — and with it every `Channel` registered to that loop — if their `run()` implementation lets a `Throwable` escape from a task invocation. The default helpers (`runAllTasks*`, `safeExecute`) catch `Throwable` correctly, but the abstract `run()` contract gives no hint that this is a hard requirement; the existing one-line javadoc just says "Run the tasks in the taskQueue". Subclassers writing bespoke task loops on top of `pollTask`/`takeTask` have no guidance — see netty#16102 for the full report. Modification: Expand the javadoc on `SingleThreadEventExecutor#run()` to spell out: - `run()` must keep going until `confirmShutdown()` returns `true`; - an uncaught `Throwable` terminates the event-loop thread and silently breaks every `Channel` registered to it; - `runAllTasks()`, `runAllTasks(long)`, and `safeExecute(Runnable)` already handle `Throwable`, so prefer them; - custom loops built on `pollTask()`/`takeTask()` must wrap each task invocation themselves. No code change. Result: Implementers of `SingleThreadEventExecutor` see the safety contract on the method they are required to override, rather than discovering the failure mode in production. Refs netty#16102. The optional "Defensive Mechanism" piece (enforce an `UncaughtExceptionHandler` on event-loop threads) from the original issue is intentionally out of scope here — happy to do that as a follow-up if maintainers want it. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com>
## Problem
`AbstractHttp2StreamFrame` and `DefaultHttp2PingFrame` both violate the
`Object` contract that requires `a.equals(b) => a.hashCode() ==
b.hashCode()`:
```java
// AbstractHttp2StreamFrame
class C extends AbstractHttp2StreamFrame {
@OverRide public String name() { return null; }
}
Object o1 = new C();
Object o2 = new C();
o1.equals(o2); // true — both have null stream
o1.hashCode() == o2.hashCode(); // false — identity hash from Object.hashCode()
```
```java
// DefaultHttp2PingFrame
Object o1 = new DefaultHttp2PingFrame(1, true);
Object o2 = new DefaultHttp2PingFrame(1, true);
o1.equals(o2); // true — equal ack and content
o1.hashCode() == o2.hashCode(); // false — hash seeded with identity from Object.hashCode()
```
Any code that puts these frames into a `HashMap`/`HashSet` silently
fails to find entries it just inserted.
## Root Cause
Both classes call `super.hashCode()` in paths where no structural hash
is available. `super.hashCode()` resolves to `Object.hashCode()`, which
returns a per-instance identity hash, so it diverges from `equals()`
which compares structural fields (stream for `AbstractHttp2StreamFrame`;
ack and content for `DefaultHttp2PingFrame`). In the ping frame the
problem is compounded: `content` is not folded into the hash at all, so
pings differing only in content also collide.
```java
// AbstractHttp2StreamFrame.hashCode (before)
if (stream == null) {
return super.hashCode(); // identity hash
}
return stream.hashCode();
```
```java
// DefaultHttp2PingFrame.hashCode (before)
int hash = super.hashCode(); // identity hash
hash = hash * 31 + (ack ? 1 : 0);
return hash;
```
## Fix
- `AbstractHttp2StreamFrame.hashCode`: when `stream` is `null`, return
`0` (a constant) so two frames whose `equals()` returns `true` via `null
== null` also produce the same hash.
- `DefaultHttp2PingFrame.hashCode`: fold the full `content` and `ack`
into the hash. Use the same `(int)(v ^ v >>> 32)` pattern already used
by `DefaultHttp2ResetFrame.hashCode` to fold the `long` for Java 8
consistency, then combine with `ack` via the standard `hash * 31 +
field` pattern.
`AbstractHttp2StreamFrame` is the base of `DefaultHttp2DataFrame`,
`DefaultHttp2ResetFrame`, `DefaultHttp2HeadersFrame`,
`DefaultHttp2PriorityFrame`, and `DefaultHttp2WindowUpdateFrame`, all of
which start their own `hashCode` with `int hash = super.hashCode();`.
Fixing the base class transitively fixes them: when their `stream` is
null, the base now contributes a deterministic value rather than
identity.
## Tests Added
| Change point | Test |
|---|---|
| `AbstractHttp2StreamFrame.hashCode` null-stream branch |
`testAbstractHttp2StreamFrameEqualInstancesHaveEqualHashCodes` — two
anonymous subclass instances with null stream are `.equals()` true and
their hashes match. |
| `DefaultHttp2PingFrame.hashCode` contract |
`testDefaultHttp2PingFrameEqualInstancesHaveEqualHashCodes` — two pings
with the same `ack` and `content` are `.equals()` true and their hashes
match. |
| Regression (sanity) for ping hash |
`testDefaultHttp2PingFrameHashCodeDistinguishesDifferentValues` — pings
that differ in `content` or `ack` do not trivially collide. |
All 195 tests in the HTTP/2 frame test set pass locally (0 failures / 0
errors / 2 pre-existing skips). Each failing test was verified to
reproduce the contract violation against the pre-fix code (hash mismatch
for equal instances).
## Impact
- `AbstractHttp2StreamFrame` subclasses (`DefaultHttp2DataFrame`,
`DefaultHttp2ResetFrame`, `DefaultHttp2HeadersFrame`,
`DefaultHttp2PriorityFrame`, `DefaultHttp2WindowUpdateFrame`) and
`DefaultHttp2PingFrame` can now be used as keys in hashed collections.
- The concrete hash values change for instances whose `stream` is `null`
— any caller that serialized or persisted a hash-code externally would
see a difference, but no public API, no `equals()` semantics, and no
wire format changes.
Fixes netty#13659
Motivation: DnsQueryIdSpace uses SecureRandom.nextBytes. This can call java.io.FileInputStream#readBytes, which triggers a BlockingOperationError when BlockHound is enabled. Modification: - Allow blocking calls in DnsQueryIdSpace#nextId and DnsQueryIdSpace$DnsQueryIdRange#pushId. - Add testDnsNameResolverAllowsBlockingCalls to verify that DnsNameResolver does not trigger BlockHound exceptions. Result: No BlockHound exceptions during DNS resolution. Co-authored-by: Violeta Georgieva <696661+violetagg@users.noreply.github.com>
Motivation: The current implementation still has two problems: 1. The handling of auto-read and self-triggered channelReadComplete events is hidden inside helper methods, making the control flow harder to follow and reason about. 2. When auto-read is enabled, FlowControlHandler should behave as if it is not present in the pipeline. However, the current implementation violates this contract: 1. read() does not always delegate to ctx.read() when auto-read is enabled. 2. channelReadComplete() does not always propagate channelReadComplete when auto-read is enabled. 3. When all reads are satisfied, FlowControlHandler may self-fire channelReadComplete even though it needs to wait for upstream firing channelReadComplete when auto-read is enabled. Modification: 1. Moved auto-read handling into the top-level control flow, making case-handling explicit. 2. Fixed all cases where FlowControlHandler deviated from transparent behavior when auto-read is enabled. Result: 1. With auto-read enabled, FlowControlHandler now behaves transparently and preserves the expected channelReadComplete propagation semantics. 2. The control flow is easier to understand and reason about. Co-authored-by: Szymon Habrainski <56340221+schiemon@users.noreply.github.com>
…setMaxHeaderListSize (netty#16911) Auto-port of netty#16901 to 4.1 Cherry-picked commit: 667e3e8 --- Motivation: HpackDecoder#setMaxHeaderListSize validates the supplied value against MIN_HEADER_LIST_SIZE and MAX_HEADER_LIST_SIZE, but the error message thrown when the value is out of range incorrectly references MIN_HEADER_TABLE_SIZE and MAX_HEADER_TABLE_SIZE. This is misleading: it reports header table size limits while the method actually validates the header list size, which makes the resulting Http2Exception confusing to diagnose. Modifications: In HpackDecoder#setMaxHeaderListSize, update the connectionError call so that the formatted message uses MIN_HEADER_LIST_SIZE and MAX_HEADER_LIST_SIZE instead of the header-table-size constants, matching the actual range check performed on the argument. Result: When an invalid maxHeaderListSize is supplied, the thrown Http2Exception now reports the correct lower and upper bounds, making the error self-consistent and easier to debug. No behavioral change in validation logic. Co-authored-by: skyguard1 <qhdxssm@qq.com>
…#16916) ### Motivation netty#16787 fixed the `READ_VARIABLE_HEADER` too-long check on the **4.2** branch by replacing a `ReplayingDecoderByteBuf.readableBytes()` probe with the message size declared by the fixed header. That fix was auto-ported to 4.1 in netty#16838, **but a later CVE-2026-44248 security merge re-introduced the broken code on 4.1**, so the 4.1 branch still carries the regression: ```java int initialAvailableBytes = buffer.readableBytes(); ... if (initialAvailableBytes < maxBytesInMessage) { throw signal; // REPLAY } else { bailOut = true; // too long } ``` Because `MqttDecoder extends ReplayingDecoder`, `buffer` is a `ReplayingDecoderByteBuf` whose `readableBytes()` returns `Integer.MAX_VALUE - readerIndex` rather than the bytes actually buffered. With the default `maxBytesInMessage` of `8092`, the `initialAvailableBytes < maxBytesInMessage` check is therefore effectively always false. So when `decodeVariableHeader` asks for a `REPLAY` because the variable header has not fully arrived yet, the decoder takes the `bailOut` branch and rejects the message with a `TooLongFrameException` instead of waiting for the rest — a valid message whose variable header is split across reads is dropped. ### Modification Re-apply the netty#16787 fix on 4.1: drop the `initialAvailableBytes` / `readableBytes()` probe and decide based on `bytesRemainingBeforeVariableHeader` (the remaining length declared by the fixed header). A genuinely oversized message is still rejected by the existing `bytesRemainingBeforeVariableHeader > maxBytesInMessage` check; an incomplete one now correctly `REPLAY`s. ### Result A message whose variable header arrives in chunks is decoded once complete, instead of being rejected as too long, while declared-oversize messages still fail with `TooLongFrameException`. The two regression tests from netty#16787 are ported here. All `codec-mqtt` tests pass locally. Note: this targets **4.1 only** and intentionally carries no cherry-pick label — 4.2 already has the fix via netty#16787. Found while addressing @chrisvest's review comment on netty#16813 about the same `readableBytes()` antipattern.
…ty#16813) Motivation: The CVE-2026-44248 fix in 82f47fa added a guard at the top of `decodeProperties` to trigger an early REPLAY when the cumulation buffer did not yet have the full properties block: ```java if (buffer.readableBytes() < totalPropertiesLength) { buffer.readSlice(totalPropertiesLength); } ``` Because `MqttDecoder` extends `ReplayingDecoder`, the buffer passed to `decodeProperties` is a `ReplayingDecoderByteBuf` whose `readableBytes()` returns `Integer.MAX_VALUE - readerIndex` rather than the actual number of bytes available (see `ReplayingDecoderByteBuf.readableBytes()`). The `if`-condition is therefore false in practice and the `readSlice()` call never executes, so the early-REPLAY optimization is effectively dead code. When the properties block arrives in chunks the decoder still falls back to partial parsing followed by a mid-loop REPLAY, which defeats the optimization's intent on slow streams. Modification: Replace the broken `readableBytes()` check with a `getByte()` probe at `buffer.readerIndex() + totalPropertiesLength - 1`. `ReplayingDecoderByteBuf.checkIndex` throws `Signal.REPLAY` when `index + 1 > writerIndex` — i.e. exactly when the cumulation buffer does not yet hold the full properties block. The call has no side effect on `readerIndex`, so subsequent parsing is unchanged once the data arrives. A `totalPropertiesLength > 0` guard skips the probe when there are no properties. Result: The early-REPLAY guard now actually fires when properties data is incomplete, restoring the CVE-2026-44248 fix's intent of avoiding repeated partial-properties parsing on slow streams. No semantic change on the happy path; all 107 existing `codec-mqtt` tests pass locally. Note: this is complementary to netty#16787, which addresses the *outer* variable-header REPLAY handling broken by the same CVE patch (same `buffer.readableBytes()` antipattern in a ReplayingDecoder context). Both fixes are independent and can land in either order; together they restore the optimization the CVE-2026-44248 fix originally aimed for.
…#16723) (netty#16933) ## Problem `HttpMethod`'s constructor accepts a wire-level method name such as `\x00GET\x00` and silently treats it as `GET`. Because the method name is later compared against expected values (`HttpMethod.GET`, etc.), this masks the difference between a clean `GET` and a control-byte-padded one — a known HTTP request-smuggling vector when Netty sits behind a proxy or in front of a backend that interprets the bytes differently. A reproducer is in netty#15047: ``` printf '\x00GET\x00 / HTTP/1.1\r\n\r\n' | nc localhost 80 # → request is decoded as method=GET, isSuccess=true ``` ## Root Cause `HttpMethod(String)` runs `checkNonEmptyAfterTrim(name, …)` before validating the name as an HTTP token. `String.trim()` strips every character with code point ≤ 0x20, which includes `NUL`, `CR`, `LF`, `VT`, `FF`, and the rest of the C0 range. After the trim the surviving string is a clean `"GET"`, which passes `HttpHeaderValidationUtil.validateToken` even though the wire bytes contained a non-token character at the boundary. ## Fix In `codec-http/src/main/java/io/netty/handler/codec/http/HttpMethod.java`: - Replace the `String.trim()`-based pre-pass with an explicit loop that only skips the single space (`0x20`) and horizontal tab (`0x09`) characters at the start and end. - Throw `IllegalArgumentException("name cannot be empty")` if the result is empty. - Run the existing `HttpHeaderValidationUtil.validateToken` facade against the resulting substring, so any non-token character — including a `NUL` left at the boundary — is reported via `"Illegal character in HTTP Method: 0x…"`. The HTTP request decoder already wraps `createMessage` exceptions into a decoder failure on the resulting `HttpRequest`, so the upstream effect is that `\x00GET\x00 …` produces an `HttpRequest` with `decoderResult().isSuccess() == false` instead of a phantom `GET`. ## Tests Added New `codec-http/src/test/java/io/netty/handler/codec/http/HttpMethodTest.java` (17 tests). NUL bytes are constructed via `String.valueOf((char) 0x00)` so the source file stays text-only. | Change point | Test | |--------------|------| | Cached lookup of standard methods unchanged | `valueOfReturnsCachedInstanceForKnownMethods` | | Custom method names still accepted | `constructorAcceptsCustomMethodName` | | `SP` trim still works (regression) | `constructorTrimsLeadingAndTrailingSpaces` | | `HT` trim still works (regression) | `constructorTrimsLeadingAndTrailingTabs` | | Reject NUL at start/end/both/embedded | `constructorRejectsLeadingNul`, `constructorRejectsTrailingNul`, `constructorRejectsLeadingAndTrailingNul`, `constructorRejectsEmbeddedNul` | | Reject other C0 control chars previously stripped by `trim()` | `constructorRejectsCarriageReturn`, `constructorRejectsLineFeed`, `constructorRejectsVerticalTab`, `constructorRejectsFormFeed` | | Reject embedded space (still a non-token char per RFC 7230) | `constructorRejectsEmbeddedSpace` | | Reject empty / blank-only names | `constructorRejectsEmptyString`, `constructorRejectsBlankString` | | End-to-end: decoder fails on NUL-padded method | `requestDecoderRejectsNulPaddedMethod` | | End-to-end regression: clean `GET` still parses | `requestDecoderAcceptsCleanMethod` | `mvn -pl codec-http test` runs 7834 tests with 0 failures locally. ## Impact - API: `HttpMethod`'s public constructor becomes stricter — inputs containing characters that were previously silently stripped (anything below `0x20` other than `SP` and `HT`) now throw `IllegalArgumentException`. Method names that already conform to RFC 7230's `token` rule are unaffected, and lenient `SP`/`HT` padding is still tolerated for backward compatibility. - Wire effect: A request whose method on the wire contains `NUL`, `CR`, `LF`, `VT`, `FF`, or other control bytes now produces a failed `HttpRequest` (`decoderResult().isSuccess() == false`) instead of being silently normalised — the existing `HttpRequestDecoder` failure plumbing handles the rest. - No changes outside `HttpMethod` and the new test class. Fixes netty#15047 (cherry picked from commit netty@6ad888e) --------- Co-authored-by: Guimu <30684111+daguimu@users.noreply.github.com>
Auto-port of netty#16936 to 4.1 Cherry-picked commit: 09156ac --- Motivation: tcnative 2.0.78.Final was released Modifications: Update to latest release Result: Depend on latest tcnative release Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…etty#16955) Auto-port of netty#16952 to 4.1 Cherry-picked commit: 6dabf56 --- Motivation: `DefaultHttp2FrameReader` did not verify that `PUSH_PROMISE` frames are associated with a stream before processing the frame-specific payload. As a result, a `PUSH_PROMISE` frame with stream ID `0` was not rejected by the same stream association validation used by DATA, HEADERS, PRIORITY, and RST_STREAM frames. Per RFC 9113 section 6.6, `PUSH_PROMISE` frames identify the stream they are associated with, and receipt of a `PUSH_PROMISE` frame with stream ID `0` MUST be treated as a connection error of type `PROTOCOL_ERROR`. Modifications: - Add `verifyAssociatedWithAStream()` to `verifyPushPromiseFrame()`. - Add a regression test for `PUSH_PROMISE` with stream ID `0`. - Verify that the error is connection-level `PROTOCOL_ERROR`. - Verify that `onPushPromiseRead(...)` is not invoked for the invalid frame. Result: Invalid `PUSH_PROMISE` frames with stream ID `0` are now rejected consistently with other stream-associated HTTP/2 frame types. Co-authored-by: skyguard1 <qhdxssm@qq.com>
…netty#16957) Auto-port of netty#16951 to 4.1 Cherry-picked commit: a655e64 --- ## Problem `JdkZlibDecoder` cannot decode a gzip stream that carries an **FEXTRA** extra field (`FLG.FEXTRA`, `0x04`). Such streams are valid per [RFC 1952](https://datatracker.ietf.org/doc/html/rfc1952#section-2.3.1.1) and decode fine with `java.util.zip.GZIPInputStream`, but netty throws `DecompressionException: decompression failure`. This affects, for example, gzip variants that use the extra field (BGZF and others). ## Root Cause In `JdkZlibDecoder` the gzip XLEN handling has two combined defects: ```java private int xlen = -1; ... xlen |= xlen1 << 8 | xlen2; // FLG_READ, when FEXTRA is set ... case XLEN_READ: if (xlen != -1) { // never true -> extra field never skipped ... in.skipBytes(xlen); } ``` 1. `xlen` starts at the `-1` "no extra field" sentinel and the length is merged with `xlen |= ...`. OR-ing anything into `-1` (`0xFFFFFFFF`) leaves it `-1`, so `if (xlen != -1)` is always `false` and the extra field is **never skipped**. The unskipped bytes are then handed to the `Inflater` as deflate data, which fails. 2. XLEN is a **little-endian** unsigned 16-bit value, but it was assembled as `xlen1 << 8 | xlen2` (big-endian). So even with defect 1 fixed in isolation, the wrong number of bytes would be skipped (e.g. a 6-byte extra field would skip 1536). This has been latent since gzip support was added, because `GZIPOutputStream` never emits an FEXTRA field, so no existing test exercised this path. ## Fix Assemble XLEN as an assignment in little-endian order: ```java xlen = xlen2 << 8 | xlen1; ``` Added a `JdkZlibTest` case that crafts a gzip stream with an FEXTRA field, cross-checks that it is valid by decoding it with the JDK `GZIPInputStream`, and asserts `JdkZlibDecoder` decodes it to the same bytes. ## Result `JdkZlibDecoder` correctly skips the gzip extra field and decodes streams that carry one. Full `JdkZlibTest` (23 tests) passes. Co-authored-by: Guimu <30684111+daguimu@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…s for HTTP/2 (netty#16964) Auto-port of netty#16932 to 4.1 Cherry-picked commit: 91ec8cd --- Motivation: RFC 9113 Section 8.3 requires HTTP/2 requests to include `:method`, `:scheme` and `:path`, and responses to include `:status`. Netty's message API (HttpConversionUtil) already rejects a missing :method, :path or :status, but the raw frame API (Http2FrameCodec / Http2MultiplexHandler) does not validate this and accepts requests and responses with missing required pseudo-headers. This is reported in netty#10633 and netty#13630. Modification: Add an optional `validateRequiredPseudoHeaders` setting (disabled by default) to `DefaultHttp2ConnectionDecoder` and the HTTP/2 builders. When enabled, initial request and response HEADERS that omit a required pseudo-header are rejected with a `PROTOCOL_ERROR` stream error. `CONNECT` and extended `CONNECT` requests are handled according to RFC 9113 Section 8.5 and RFC 8441. Result: The raw frame API can now opt into RFC 9113 Section 8.3 required-pseudo-header validation, while preserving existing behavior by default. Co-authored-by: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com>
Motivation:
Per MQTT 3.1.1 [MQTT-1.5.3-1] / MQTT 5.0 [MQTT-1.5.4-1], the character
data
in a UTF-8 Encoded String MUST be well-formed UTF-8 as defined by the
Unicode
specification and restated in RFC 3629. In particular it MUST NOT
contain
encodings of code points between U+D800 and U+DFFF, overlong sequences,
or
sequences longer than 4 bytes. If received, the Control Packet MUST be
treated as a Malformed Packet.
`MqttDecoder` currently decodes every UTF-8 Encoded String via
`ByteBuf#readString(size, UTF_8)`, which delegates to `new String(bytes,
UTF_8)`. That constructor uses replacement semantics: malformed UTF-8
sequences are silently replaced with U+FFFD instead of being
reported, and an embedded U+0000 byte is accepted. This affects every
UTF-8
Encoded String in MQTT, including ClientId, Will Topic, Topic Name
(PUBLISH /
SUBSCRIBE / UNSUBSCRIBE filter), User Name, and the MQTT 5 string
properties
(Content Type, Response Topic, Reason String, Authentication Method,
Server
Reference, User Property, …). Beyond spec compliance, silently rewriting
these fields can cause routing/ACL/identity mismatches between the wire
representation and what the application sees.
Modifications:
* `MqttDecoder` now performs strict UTF-8 validation by default:
* UTF-8 strings are decoded through a per-instance `CharsetDecoder`
configured with `CodingErrorAction.REPORT` for both `onMalformedInput`
and `onUnmappableCharacter`. Any `CharacterCodingException` is converted
to a `DecoderException`.
* After successful decoding, the resulting `String` is scanned for
`U+0000`; if present, a `DecoderException` is thrown.
* The exceptions propagate through the existing decode error path
(`MqttMessageFactory.newInvalidMessage`), so callers continue to receive
a single `MqttMessage` with `decoderResult().isFailure() == true`,
matching the existing behaviour for other malformed-packet conditions
(e.g. non-zero reserved flag).
* A new opt-out constructor `MqttDecoder(int maxBytesInMessage, int
maxClientIdLength, boolean strictUtf8Validation)`
is provided for users that need to retain the historical
replacement-char
behaviour. The pre-existing constructors delegate to it with
`strictUtf8Validation = true`.
* The previously-static `decodeString` and `decodeProperties` helpers
are
converted to instance methods to access the new flag (the surrounding
variable-header / payload decoders were already instance methods that
call them; no signature changes for any other helpers).
* New tests in `MqttCodecTest` exercise:
* invalid 2-byte continuation (`0xC3 0x28`)
* truncated multi-byte sequence at end-of-string (lone `0xC3`)
* Modified-UTF-8 overlong NUL (`0xC0 0x80`)
* isolated UTF-16 high surrogate (`0xED 0xA0 0x80` → U+D800)
* 5-byte sequence forbidden by RFC 3629 (`0xF8 …`)
* embedded U+0000
* well-formed multi-byte UTF-8 ("hello") decodes successfully
* empty ClientId is still accepted under strict mode
* legacy mode (`strictUtf8Validation = false`) still accepts malformed
UTF-8 (replaced with U+FFFD) and embedded U+0000
Result:
* MQTT spec [MQTT-1.5.3-1/2] and [MQTT-1.5.4-1/2] are enforced for every
UTF-8 Encoded String parsed by `MqttDecoder` (ClientId, Will Topic,
Topic
Name, Topic Filter, User Name, MQTT 5 string properties, including User
Property key/value pairs).
* Behaviour change: packets that previously decoded into messages
containing U+FFFD or U+0000 will now be reported as malformed. Users
relying on the old behaviour can opt out via the new constructor flag.
* No API removed; the existing `MqttDecoder()`, `MqttDecoder(int)` and
`MqttDecoder(int, int)` constructors remain source- and binary-
compatible.
(cherry picked from commit ba8e9e6)
Co-authored-by: skyguard1 <qhdxssm@qq.com>
…e parse end (netty#16968) Auto-port of netty#16958 to 4.1 Cherry-picked commit: 692d23f --- ## Problem `DateFormatter.parseHttpDate(txt, start, end)` is documented to parse only the `[start, end)` substring. Its tokenizer loop correctly stops at `end`, but the **trailing token** — the one still open when the loop finishes — was terminated at `txt.length()` instead of `end`: ```java // terminate trailing token return tokenStart != -1 && parseToken(txt, tokenStart, txt.length()); ``` When `end < txt.length()` and the date's *last* token is the one that completes the parse, the trailing token swallows the bytes after `end` and fails to parse. This is reachable in practice through cookies. A `Set-Cookie` header like `foo=bar; Expires=<date>; Path=/` makes `ClientCookieDecoder` call `parseHttpDate(header, start, end)` with `end` pointing at the `;` before `Path`. RFC 6265 §5.1.1 cookie-date tokens are **order-independent**, so a valid date whose year (or day) is the last token — e.g. `Sun 08:49:37 06 Nov 1994` — parses fine in isolation but returns `null` as a substring, silently dropping the expiry and downgrading the cookie to a session cookie. Standard `Sun, 06 Nov 1994 08:49:37 GMT` ordering does not trigger it (the time token completes the parse mid-loop, before the trailing `GMT`), which is why existing tests miss it. ## Fix Terminate the trailing token at `end` rather than `txt.length()`. Since `end <= txt.length()` always, this only ever shrinks the trailing token to the intended bound; the full-string `parseHttpDate(txt)` path (where `end == txt.length()`) is unaffected. Added a `DateFormatterTest` case that parses such a date both in isolation and as a substring and asserts they agree. ## Result `parseHttpDate` honours the `end` bound for the trailing token; valid order-independent cookie dates followed by other attributes now parse correctly. Full `DateFormatterTest` (14) passes. Co-authored-by: Guimu <30684111+daguimu@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…fault (netty#16973) Auto-port of netty#16961 to 4.1 Cherry-picked commit: f9d8c42 --- Motivation: We should better let the user explicit configure if something should be accepted by default or not so it's not done by mistake. Modifications: - Deprecate constructor which accept by default Result: Make user aware of default behaviour Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation: Add basic blocks for RFC 10008 (The HTTP QUERY method) Modification: Added the QUERY HttpMethod as well as the Accept-Query header constant Result: Downstream consumers have an easier time implementing RFC 10008 Co-authored-by: Mario Daniel Ruiz Saavedra <desiderantes93@gmail.com>
…tty#16959) (netty#16976) Motivation: The AbstractTrafficShapingHandler#calculateSize supports ByteBuf, ByteBufHolder, and FileRegion, so traffic-shaping handlers may queue delayed ByteBufHolder messages such as HTTP content. When a channel becomes inactive and queued writes are discarded, ChannelTrafficShapingHandler, GlobalTrafficShapingHandler, and GlobalChannelTrafficShapingHandler only release messages that are direct ByteBuf instances. This leaks queued ByteBufHolder / other ReferenceCounted messages. The corresponding write promises are also left incomplete even though the messages will never be written. Modifications: • Add a shared queued-write cleanup helper in AbstractTrafficShapingHandler. • Use ReferenceCountUtil.safeRelease(...) instead of instanceof ByteBuf checks. • Fail discarded queued write promises with ClosedChannelException. • Reset per-channel queue size after discarded queued writes are cleaned up. • Add tests covering queued ByteBufHolder writes for: ◦ ChannelTrafficShapingHandler ◦ GlobalTrafficShapingHandler ◦ GlobalChannelTrafficShapingHandler Result: Queued delayed writes are cleaned up consistently when the channel is closed: reference-counted messages are released and callers waiting on write promises are notified with failure. Internal queue-size accounting is left in a clean state on removal. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com> Co-authored-by: skyguard1 <qhdxssm@qq.com>
…le dequeueing (netty#16983) Auto-port of netty#16949 to 4.1 Cherry-picked commit: 8081e23 --- ## Motivation `FlowControlHandler` does not respect changes to the channel's auto-read setting while flushing the queue. As such, a downstream handler that disables auto-read from within `channelRead()` cannot stop `FlowControlHandler` from flushing the rest of the queue. ## Modification - Merge `dequeueOne()` and `dequeueAll()` into a single `dequeue()` loop that re-checks `config.isAutoRead()` and `unsatisfiedReads` before every message. - Add tests for auto-read toggled on and off from `channelRead` and re-entrant reads satisfied from the queue, plus previously missing coverage for handler removal, `channelInactive` release, and `releaseMessages = false`. ## Result A downstream handler that disables auto-read from `channelRead()` now stops delivery of the rest of the queue. Fixes netty#16945 Co-authored-by: Szymon Habrainski <56340221+schiemon@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…out/resetReadTimeout (netty#16982) (netty#16989) ## Motivation `IdleStateHandler `exposes two programmatic reset methods — `resetWriteTimeout()` and `resetReadTimeout()` — intended to let callers defer idle detection without performing an actual write or read. Their documented contract is to restart the idle timer from the point of the call. Both methods update the relevant timestamp but do not reset the corresponding "first event" flags (firstWriterIdleEvent / firstReaderIdleEvent). As a result, if a non-first idle event has already fired before the programmatic reset, the next idle event after the reset is incorrectly reported as `WRITER_IDLE_STATE_EVENT`/ `READER_IDLE_STATE_EVENT` (first=false) instead of `FIRST_WRITER_IDLE_STATE_EVENT`/ `FIRST_READER_IDLE_STATE_EVENT`. This is inconsistent with the writeListener path, which resets both the timestamp and the first-event flags together. ## Modifications - resetWriteTimeout(): add `firstWriterIdleEvent = firstAllIdleEvent = true` - resetReadTimeout(): add `firstReaderIdleEvent = firstAllIdleEvent = true` - Add IdleStateHandlerResetFlagTest with two tests that exercise the post-non-first-event reset sequence for both methods ## Result After calling `resetWriteTimeout()` or `resetReadTimeout()`, the next idle event correctly fires as `FIRST_WRITER_IDLE_STATE_EVENT `or `FIRST_READER_IDLE_STATE_EVENT`,consistent with what a real write or read activity would produce. --- Note: the same issue exists in the 4.1 branch. Happy to provide a backport if the team considers it in scope. Signed-off-by: husseinvr97 <husseinmustafabas05@gmail.com> (cherry picked from commit 512ba9d) Co-authored-by: husseinvr97 <husseinmustafabas05@gmail.com>
Auto-port of netty#16988 to 4.1 Cherry-picked commit: 02bbf71 --- Motivation: There is a typo in the Javadoc tag for the maxClientHelloLength parameter. Modification: Replace @paramm with @PARAM. Result: Javadoc uses the correct parameter tag. Co-authored-by: CoderBruis <37364336+coderbruis@users.noreply.github.com>
…s when it drains, and fail stuck HTTP/2 streams instead of spinning empty DATA frames (netty#16997) Auto-port of netty#16947 to 4.1 Cherry-picked commit: a5d4f28 --- **Motivation:** On a proxy doing HTTP/2 egress we OOMed when the remote flow controller spun writing empty DATA frames (~134M) into an already-unwritable channel. This is the OOM from netty#11959, still reachable on `4.2.12.Final` despite netty#14220 - that fix only closed the exception-in-`remove` route and left two gaps: * `CoalescingBufferQueue.remove(...)` can leave `readableBytes > 0` with an empty deque -> it decrements by a buffer's *live* readable bytes, and its empty-queue early return asserts rather than reconciles * Also `writeAllocatedBytes` re-writes a frame that makes no progress without ever checking channel writability. Full investigation and heap dump in netty#16946. **Modification:** * `AbstractCoalescingBufferQueue` - add a `reconcileReadableBytes()` (called at the `remove(...)` early return and after the drain loop) so an empty queue always reports 0 readable bytes. * `DefaultHttp2RemoteFlowController.writeAllocatedBytes` - if the head frame is given a positive budget but is neither removed nor shrinks across two consecutive iterations, fail the stream via cancel path instead. * Tests cover the queue desync, the stuck-frame spin (failing the stream), and a single no-progress pass (tolerated). **Result:** The queue can't report bytes it can't produce, and the flow controller can't emit empty DATA frames unboundedly - a wedged stream fails cleanly instead of OOMing the connection. No public API change. Fixes netty#16946 Co-authored-by: Gavin Bunney <409207+gavinbunney@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…etty#16971) (netty#16986) Motivation: `HttpRequestDecoder` accepts a request whose HTTP-version token carries a boundary control byte (`NUL`, `CR`, `LF`, `VT`, `FF`, …): ```bash printf 'GET / \x00HTTP/1.1\r\nHost: localhost\r\n\r\n' | nc <host> <port> ``` `HttpVersion.valueOf(String, boolean)` ran `text.trim()` before matching the version, and `String.trim()` removes every character with code point `<= 0x20`. The boundary control byte was therefore silently stripped and the surviving `"HTTP/1.1"` matched the cached constant, so the malformed request decoded as a clean `HTTP/1.1` one. The method token on the same request line already rejects such a byte since netty#16723 (issue netty#15047), so the two tokens were inconsistent — this is the same boundary-control-character / request-smuggling class, left open for the version token. Modification: - `HttpVersion`: drop the `trim()` in `valueOf` and in the `HttpVersion(String, boolean, boolean)` constructor. The existing strict format check (`length == 8 && startsWith("HTTP/") && charAt(6) == '.'`) now rejects a token padded with a control byte, and the non-strict path rejects control/whitespace in the protocol name via `hasControlOrWhitespace`. Without the `trim()`, `SP`/`HT` padding is rejected too, mirroring the method-token behaviour from netty#16723. - `RtspVersions.valueOf`: drop the `trim()` the same way. `HttpRequestDecoder` already turns `createMessage` exceptions into a decoder failure, so the wire effect is `decoderResult().isSuccess() == false` instead of a phantom `HTTP/1.1`. Result: Before this change `"GET / \x00HTTP/1.1\r\n…"` decoded successfully (`decoderResult().isSuccess() == true`, `protocolVersion() == HTTP_1_1`); after it the decoder reports a failure, matching the method-token behaviour. A clean `HTTP/1.1` request still decodes as before. ``` Test set: io.netty.handler.codec.http.HttpVersionParsingTest Tests run: 53, Failures: 0, Errors: 0, Skipped: 0 Test set: io.netty.handler.codec.http.HttpRequestDecoderTest Tests run: 85, Failures: 0, Errors: 0, Skipped: 0 Test set: io.netty.handler.codec.rtsp.RtspDecoderTest Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 Test set: io.netty.handler.codec.rtsp.RtspEncoderTest Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 ``` Fixes netty#16970 --------- Co-authored-by: Bryce Anderson <bl_anderson@apple.com> --------- Co-authored-by: HwangRock <157935545+HwangRock@users.noreply.github.com> Co-authored-by: Bryce Anderson <bl_anderson@apple.com>
…netty#17003) Auto-port of netty#16991 to 4.1 Cherry-picked commit: fc86cc6 --- Motivation: When a CR (`0x0D`) byte appears in the middle of a multi-byte UTF-8 sequence within a STOMP header line, `Utf8LineParser.process` takes the CR branch and returns early **without clearing its partial UTF-8 decode state** (`interim` / `nextRead`). The next byte is then combined with the stale state and decoded incorrectly, corrupting the decoded value. For example, a header value made of the bytes `0xC3 0x0D 0x41` (`0xC3` starts a 2-byte sequence, `CR`, then `A`) decodes to `Á` instead of `A`. Modifications: Clear `interim` and `nextRead` in the CR branch of `Utf8LineParser.process`, so that a CR resets the UTF-8 decode state. CR is an ASCII control byte and is never a valid UTF-8 lead or continuation byte, so if it interrupts a multi-byte sequence that sequence is malformed and the partial state should be discarded. Result: Bytes following a CR are decoded correctly. Adds a regression test (`StompSubframeDecoderTest#testCRResetsUtf8DecodeState`) that fails before this change (`expected: <A> but was: <Á>`) and passes after it. The full `StompSubframeDecoderTest` suite continues to pass. Co-authored-by: Vasiliy Mikhailov <vasiliy-mikhailov@users.noreply.github.com>
…ng goaway (netty#17017) Auto-port of netty#16392 to 4.1 Cherry-picked commit: c40a344 --- Motivation: We did not use the correct number of arguments as future.cause() is null. Modifications: Remove future.cause() as argument Result: Fixes netty#16391 Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…tty#17015) Motivation: Malformed FastLZ compressed blocks can declare input that is empty or ends in the middle of match metadata. FastLz.decompress should treat these blocks as corrupted input instead of reading past the supplied chunk length. Modification: Return 0 for empty compressed input and check the input length before reading optional match length and distance bytes. Add FastLzFrameDecoder regression coverage for empty compressed payloads and truncated match metadata. Result: Malformed FastLZ compressed blocks now fail through the decoder as DecompressionException instead of triggering ByteBuf bounds exceptions. Security impact: This is not considered a security issue. Malformed compressed input can already cause the decoder to fail the channel, and the previous behavior was a Java `ByteBuf` bounds exception rather than native memory corruption, data disclosure, or privilege boundary bypass. The change makes the failure mode consistent by treating truncated FastLZ blocks as corrupted input and surfacing the existing `DecompressionException` path. --------- Co-authored-by: multicode <multicode@yawk.at> (cherry picked from commit 775ad71) Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
…tty#17022) Auto-port of netty#16762 to 4.1 Cherry-picked commit: 6661ee4 --- ## Problem `DefaultHttp2Headers` accepts header names that contain bytes outside the HTTP token grammar — non-ASCII (e.g. `0xF0`), control characters (NUL/CR/LF/VT/FF/HTAB/...), SP, DEL, and the RFC 7230 separators (`"(),/:;<=>?@[\\]{}\""`). The current `HTTP2_NAME_VALIDATOR` only rejects upper-case ASCII and lets everything else through, so something like `headers.add(new AsciiString(new byte[]{(byte)0xF0}), "v")` succeeds and is then encoded onto the wire. This was reported in netty#11975 with maintainer agreement on direction: - @idelpivnitskiy: confirmed the field-name grammar - @ejona86: "good and appropriate to restrict keys to the values permitted in HTTP/1, with little risk" ## Root Cause `DefaultHttp2Headers.HTTP2_NAME_VALIDATOR_PROCESSOR` returns `!isUpperCase(value)` and the non-`AsciiString` fallback only loops on `isUpperCase(charAt(i))`. Both paths therefore enforce the lower-case rule but skip the rest of RFC 9113 §8.2.1, which inherits the RFC 7230 token grammar. ## Fix In `codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java`: - After the empty/null and pseudo-header checks, run `HttpHeaderValidationUtil.validateToken(name)` and throw `connectionError(PROTOCOL_ERROR, ...)` when it returns a non-`-1` index. - Keep the existing upper-case `ByteProcessor` / fallback loop. The token check is additive: pseudo-headers still short-circuit, valid lower-case tokens still pass, upper-case ASCII still gets rejected (now at the upper-case check, not the token check). Pseudo-headers (`:method`, `:path`, ...) deliberately bypass the token check via the existing `hasPseudoHeaderFormat` early return, since `:` is not a valid token character but is required for pseudo-headers. ## Tests Added | Change point | Test | |--------------|------| | Reject non-ASCII via the `AsciiString` byte path | `rejectNonAsciiHeaderNameAsciiString` (uses the U+1F631 / `0xF0 0x9F 0x98 0xB1` reproducer from netty#11975) | | Reject non-ASCII via the `CharSequence` `charAt` path | `rejectNonAsciiHeaderNameCharSequence`, `rejectHighBitHeaderNameCharSequence` (covers char ≤ 0xFF and char > 0xFF) | | Reject every C0 control byte and every RFC 7230 separator | `rejectNonTokenCharactersInHeaderName` (parameterized — 28 cases: NUL, SOH, BEL, BS, HTAB, LF, VT, FF, CR, US, DEL, SP, and `, ; : / = ? @ ( ) [ ] { } < > \ "`) | | Regression: upper-case rejection unchanged | `uppercaseHeaderNameStillRejected` | | Regression: full RFC 7230 lower-case token still accepted | `acceptValidLowercaseTokenHeaderName` (covers `x-custom-header`, `2name`, and a name using every special token char `!#$%&'*+-.^_\`|~`) | | Regression: pseudo-headers still accepted | `acceptPseudoHeaderName` | | Existing wire-level test now exercises the validator | `InboundHttp2ToHttpAdapterTest.clientRequestSingleHeaderNonAsciiShouldThrow` updated — the rejection now fires at `headers.add(...)` instead of at the HPACK encoder | `mvn -pl codec-http2 test -Drevapi.skip=true` runs 1512 tests with 0 failures locally; `codec-http` regression run is also clean (8891 tests, 0 failures). ## Impact - **API**: `new DefaultHttp2Headers().add(name, value)` now throws `Http2Exception(PROTOCOL_ERROR, ...)` for any name that is not a valid lower-case RFC 7230 token. Code that was relying on the previous lax behaviour (passing non-ASCII or separator bytes) needs to either (a) clean up the name before adding, or (b) construct the headers with `new DefaultHttp2Headers(false)` to opt out of validation, which already exists. - **Wire effect**: Inbound HPACK-decoded headers with malformed names now produce an `Http2Exception` at validation time and are surfaced via the existing decoder failure path. The previous behaviour silently accepted them and could allow malformed bytes to reach the application as parsed `Http2Headers`. - **Hot path**: Standard requests are unaffected — `DefaultHttp2Headers` validation only runs when adding non-pseudo headers, and the new `validateToken` call is a single forward scan that returns `-1` on the first invalid byte for clean tokens. Fixes netty#11975 --------- Co-authored-by: Guimu <30684111+daguimu@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: An attacker who compromises any of three upstream GitHub Actions repositories can force-push a malicious commit to a mutable version tag, causing the next Netty release run to exfiltrate SSH deploy keys, GPG signing keys, and Maven Central credentials — enabling publication of backdoored `io.netty:*` artifacts to Maven Central. Modifications: Pin github actions to sha Result: Reduce risk --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
…specify desired maxPipelineDepth (netty#17085) Auto-port of netty#17068 to 4.1 Cherry-picked commit: 61aca07 --- Motivation: In scope of netty#17063, the `HttpContentEncoder` got a new property `maxPipelineDepth` and the respective constructor variant. However, the subclasses, notably `HttpContentCompressor` have not been updated to allow changing `maxPipelineDepth` and always use the default `128`. Modification: Add `HttpContentCompressor` constructor variant with ability to specify desired `maxPipelineDepth`. It would help us to absorb the change through configuration setting. @chrisvest would appreciate your feedback, thank you. Signed-off-by: Andriy Redko <drreta@gmail.com> Co-authored-by: Andriy Redko <drreta@gmail.com>
…LE (netty#17102) Auto-port of netty#17098 to 4.1 Cherry-picked commit: 303d834 --- Motivation: AdaptiveByteBuf._setLongLE delegates to the checked setLongLE on the root parent, unlike all other _set/_get methods which use the unchecked underscore-prefixed variants. This adds redundant bounds checking and ensureAccessible() on every little-endian long write. Modification: Call rootParent()._setLongLE instead of rootParent().setLongLE. Result: _setLongLE now follows the same unchecked pattern as _setLong, _setInt, _setIntLE, etc. Co-authored-by: Francesco Nigro <nigro.fra@gmail.com>
…tty#17095) Auto-port of netty#17093 to 4.1 Cherry-picked commit: 010b6e8 --- Motivation: The valid range of `maxOrder` is 0 to 14, but negative values were accepted because only the upper bound was validated. When configured through `io.netty.allocator.maxOrder`, a negative value may result in an invalid chunk size during static initialization. Modification: Validate the lower bound of `maxOrder` and add a regression test. Result: Negative `maxOrder` values are rejected with an `IllegalArgumentException`. Co-authored-by: CoderBruis <37364336+coderbruis@users.noreply.github.com>
…etty#17110) Auto-port of netty#17099 to 4.1 Cherry-picked commit: b41f2a6 --- Motivation: Malformed Snappy framed input can declare compressed or uncompressed chunk lengths that are too short to contain the mandatory masked checksum. In the compressed case this can lead to lower-level ByteBuf index errors instead of a DecompressionException. Modification: Validate minimum chunk lengths in SnappyFrameDecoder before reading the checksum or Snappy preamble. Added parameterized tests that cover invalid compressed and uncompressed chunk lengths with checksum validation both enabled and disabled. Result: Malformed Snappy chunks with invalid lengths are rejected with DecompressionException. Verification: ./mvnw -pl codec-compression -am -Dtest=SnappyFrameDecoderTest -Dsurefire.failIfNoSpecifiedTests=false test -DskipNativeTests -DskipAutobahnTests Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at>
Backport netty#16079 and netty#17114
Auto-port of netty#17118 to 4.1 Cherry-picked commit: e64a6b5 --- Motivation: `Lz4FrameDecoder` currently uses `LZ4FastDecompressor`. Recent lz4-java security hardening has degraded the performance of this path, while the decoder already knows the exact compressed block length required by `LZ4SafeDecompressor`. The fast API also does not accept the compressed source length, allowing malformed blocks to consume trailing readable bytes. Modification: - Use `LZ4SafeDecompressor` from the configured `LZ4Factory`. - Pass the exact declared compressed and decompressed lengths to the bounded decompression API. - Reject output whose actual decompressed length differs from the frame header. - Add regression coverage for reading beyond the declared compressed length and for decompressed-length mismatches. Result: Valid LZ4 frames continue to decode normally, while malformed blocks are constrained to their declared input and output bounds. The `codec-compression` test suite passes with 359 tests, and the focused `Lz4FrameDecoderTest` passes after allocating test inputs through the channel allocator. --------- Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Auto-port of netty#17129 to 4.1 Cherry-picked commit: 7990444 --- Motivation: TestLens is a new tool by the JUnit maintainers that helps to get flaky tests under control. It is free for open source projects. Modification: TestLens needs to instrument the maven POM files, so this adds a github action invocation to the PR builds that do that before running our maven builds. Result: TestLens is now enabled in our PR builds. Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
netty#17124) …or (netty#17037) Motivation HttpContentDecompressor documents maxAllocation as the maximum decompression buffer size. For gzip / deflate / zstd it is routed to each decoder's output-cap parameter. For brotli it was passed to new BrotliDecoder(maxAllocation), whose single argument is inputBufferSize, not the output cap. So a large maxAllocation enlarged brotli's input buffer while the output cap stayed at the 64 KiB default; a small maxAllocation did not tighten the output cap for brotli at all. Modifications Add BrotliDecoder.newDecoderWithMaxAllocation(int): routes maxAllocation to outputBufferSize, 0 falls back to new BrotliDecoder(). `HttpContentDecompressor`: brotli branch uses the new factory instead of new BrotliDecoder(maxAllocation). Add HttpContentDecompressorTest#testBrotliDecodingHonorsMaxAllocationAsOutputCap: brotli-compress 128 KiB, decode with HttpContentDecompressor(64), assert lossless decode and >100 chunks. Result Fixes netty#17034. maxAllocation now has consistent semantics across gzip / deflate / zstd / brotli. No API removed, one new public factory added on BrotliDecoder. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com> (cherry picked from commit 0332676) Co-authored-by: skyguard1 <qhdxssm@qq.com>
netty#17143) …17138) Motivation: TestLens requires this in order to activate its profile. It's possible some of our other tools, e.g. sdkman also make use of this. Modification: Add `CI` to the environments in our docker-compose files. Result: We should now get testlens results from our docker-based linux builds. (cherry picked from commit 6e8c31d)
…oding (netty#17137) Auto-port of netty#17117 to 4.1 Cherry-picked commit: cddcdef --- Motivation: MQTT 3.x UNSUBACK only includes the Packet Identifier, while the reason code payload is a new addition in MQTT 5.0. Modification: drop UNSUBACK reason codes for MQTT 3.x encoding --------- Co-authored-by: 如梦技术 <596392912@qq.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
…y#17176) Motivation: The shared ConcurrentSkipListChunkCache eviction policy picked the chunk with the lowest refCnt when the cache exceeded CHUNK_REUSE_QUEUE. Since chunks enter the cache with refCnt = 1 + N (N live buffers), evicting them creates zombie chunks: out of the cache, backing buffer still alive until all N buffers release. This caused 16 GB RSS on the AG benchmark. Modification: Only evict chunks where refCnt == 1 (no live buffers — just the base construction reference). If no idle chunk exists, let the cache grow past the cap. markToDeallocate on an idle chunk decrements refCnt to 0, triggering deallocate which frees the backing buffer cleanly. Result: Less memory churn under allocation pressure still allowing it to shrink on subsequence cache requests (cherry picked from commit franz1981@34bfcc7)
…tionFuture failure (netty#17189) Auto-port of netty#17140 to 4.1 Cherry-picked commit: cee0005 --- ### Motivation `GlobalEventExecutor.INSTANCE` is a static singleton that lives for the lifetime of the classloader that loaded it. Its `terminationFuture` holds a `FailedFuture` whose cause was a plain `UnsupportedOperationException`. Although `ThrowableUtil.unknownStackTrace(...)` replaces the *visible* stack trace with a single synthetic frame, the exception's internal (native) `backtrace` field is still populated at construction time by `fillInStackTrace()`. That backtrace pins the classloader of whatever thread happened to trigger the lazy initialization of `INSTANCE` (e.g. a web application's `WebAppClassLoader`), making all of that classloader's classes immortal/undeployable. This is a follow-up to the leak fixed in netty#14622. Fixes netty#17128 ### Modification Introduce a private `StacklessUnsupportedOperationException` whose `fillInStackTrace()` is a no-op, so the native backtrace is never captured, and use it for the `terminationFuture` failure. This mirrors the stackless-exception pattern already used throughout Netty. ### Result The `terminationFuture` failure no longer retains a native backtrace, so it can no longer pin the triggering thread's classloader. **Verification done:** Added `GlobalEventExecutorTest#testTerminationFutureFailureDoesNotFillInStackTrace`, which asserts that after `fillInStackTrace()` the cause's stack trace stays the single synthetic `terminationFuture` frame. It fails before the change (backtrace repopulates to 76 native frames) and passes after. Ran `mvn -pl common test -Dtest=GlobalEventExecutorTest` (6/6 pass) and `checkstyle:check@check-style` (clean) on JDK 25. Co-authored-by: seonwoojung <seonwooj0810@gmail.com>
…uffer leak when a `Throwable` is thrown during header encoding (netty#17178) Auto-port of netty#17089 to 4.1 Cherry-picked commit: 10e24f9 --- ### Motivation Fixes netty#17088. Same class of bug as the `SslHandler` leak fixed in netty#17059: a header buffer is allocated, then a `Throwable` (typically `OutOfMemoryError`) is thrown before the buffer is handed off, leaving it unreleased. netty#6729 reported the exact symptom on `HttpObjectEncoder.encodeHeaders` back in 2017 but couldn't be reproduced on demand and was closed; the root cause was never fixed. Affected paths: - HTTP/1 — `encodeInitHttpMessage()` and `encodeFullHttpMessage()` in `HttpObjectEncoder`: `buf` from `ctx.alloc().buffer(...)` leaks if `encodeHeaders()` throws before it is added to `out`. - HTTP/2 — `writeHeadersInternal()`, `writePushPromise()` and `writeContinuationFrames()` in `DefaultHttp2FrameWriter`: the retained `fragment` / frame-header buffer leaks if a later allocation throws after the fragment is sliced off the header block. With pooled direct buffers this leaks off-heap memory the GC cannot reclaim, so repeated OOME on these paths ends in `OutOfDirectMemoryError` / process death. ### Modification - `HttpObjectEncoder`: guard the header buffer with a success/handed-off flag and release it in a `finally` unless ownership was transferred. In `encodeFullHttpMessage()` the flag is set immediately before `encodeByteBufHttpContent()` so the chunked path — where `buf` is already added to `out` before `encodeChunkedHttpContent()` can throw — does not double-release. - `DefaultHttp2FrameWriter`: hoist `fragment` to method scope, null it right after `ctx.write(fragment, ...)`, and release it in `finally` if non-null. `writeContinuationFrames()` additionally guards the reused frame-header buffer with a per-fragment flag. - Add tests to both modules using a tracking allocator that injects an `OutOfMemoryError` on a targeted allocation, asserting every tracked buffer reaches `refCnt() == 0` after the failure. ### Result No buffer leak when a `Throwable` is thrown mid header encoding. The normal path is unchanged. Measured with the tracking allocator (each OOME on these paths leaks exactly one header buffer, so the leak grows linearly with the number of affected requests): | path | leak / request | before | after | |---|---|---|---| | `HttpObjectEncoder` init / full | 256 B | `refCnt == 1` | `0` | | Http2 `writeHeaders` / `writePushPromise` | 256 B | `refCnt == 1` | `0` | | Http2 `writeContinuationFrames` (large headers) | 64 KiB | `refCnt == 1` | `0` | At scale on the 256 B paths that is ~244 MiB leaked per 1M affected requests; on the CONTINUATION path (large headers) ~61 GiB per 1M. After the fix the leak is `0` regardless of request count. --------- Co-authored-by: HwangRock <157935545+HwangRock@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
…y#17193) Motivation: `BrotliEncoderChannel.close()` closes its destination, which re-enters `BrotliEncoder.Writer.close()`. The re-entrant call schedules another finish task before `isClosed` is set. If the first finish fails, the second task can retry with an already-failed close promise. Modification: Track whether writer close has already been initiated and ignore subsequent close calls before scheduling another finish task. Result: Brotli encoder close is idempotent while a finish is pending, and exceptional finalization no longer retries with an already-completed promise. Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at>
Auto-port of netty#17194 to 4.1 Cherry-picked commit: 035d76e --- Motivation: Update Netty's optional LZF compression dependency to the latest Maven Central release. Modification: Bump `com.ning:compress-lzf` from `1.2.0` to `1.2.1` in root dependency management. Result: The `codec-compression` LZF tests pass locally: ```bash ./mvnw -B -ntp -Dmaven.repo.local=/tmp/opencode/netty-maven-home/repository \ -pl codec-compression -am test \ -Dtest=LzfEncoderTest,LzfDecoderTest,LzfIntegrationTest,LengthAwareLzfIntegrationTest \ -Dsurefire.failIfNoSpecifiedTests=false ``` Tests run: 36, Failures: 0, Errors: 0, Skipped: 0 Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at>
…netty#17192) (netty#17200) `WebSocketServerHandshaker` writes the 101 Switching Protocols response via `Channel.writeAndFlush()`. The write starts at the tail of the pipeline, so every `ChannelOutboundHandler` placed after `WebSocketServerProtocolHandler` sees the raw HTTP upgrade response, even though handlers there operate on WebSocket frames. In netty#17141 a handler that queues outbound messages until `HandshakeComplete` captured the response itself, so the handshake could never complete. `WebSocketServerHandshaker` already provides `ChannelHandlerContext` overloads for `close()` for the same reason; this change applies the same pattern to the handshake response. - Add `handshake(ChannelHandlerContext, ...)` overloads to `WebSocketServerHandshaker`, following the existing `close(ChannelHandlerContext, ...)` overloads in the same class, and route both variants through a shared `handshake0(ChannelOutboundInvoker, Channel, ...)`. - `WebSocketServerProtocolHandshakeHandler` now passes its ctx, so the response is written from the handshake handler's former position. - Update the benchmarkserver example to use the new overload. - Tests: a regression test asserts a handler behind the protocol handler observes no write during the handshake, for both full and non-full upgrade requests. Existing tests that captured the response behind the protocol handler now read it via `readOutbound()`. Four `handshake(null, ...)` calls needed a `(Channel)` cast to stay unambiguous. Notes: - With the documented pipeline ordering (extension/compression handler before the protocol handler, as in `WebSocketServerInitializer`) nothing changes: the write still traverses `WebSocketServerExtensionHandler` and the HTTP encoder. Only handlers behind the protocol handler stop seeing the 101, which is the bug being fixed. - The tail-write behavior of the `Channel` overloads is unchanged and still covered by `WebSocketServerHandshaker00/08/13Test` and `WebSocketServerHandshakerTest`. Fixes netty#17141. --------- Co-authored-by: el-psy-kongroo-d <307969302+el-psy-kongroo-d@users.noreply.github.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com> Co-authored-by: el-psy-kongroo-d <el.psy.kongroo.d@gmail.com> Co-authored-by: el-psy-kongroo-d <307969302+el-psy-kongroo-d@users.noreply.github.com>
…sponses (netty#17182) (netty#17203) Resolves netty#17181. `HttpClientCodec` and `HttpContentEncoder`, but `HttpServerCodec` was left out. `isContentAlwaysEmpty(HttpResponse)` in `HttpServerCodec` polls the request method queue for every response it encodes: ```java @OverRide protected boolean isContentAlwaysEmpty(HttpResponse msg) { methodFlag = pollMethod(); return methodFlag == METHOD_FLAG_HEAD || super.isContentAlwaysEmpty(msg); } ``` It is invoked for every `HttpResponse` on both encoder paths — `encodeFullHttpMessage()` and `encodeInitHttpMessage()` in `HttpObjectEncoder`. An interim response is not the final response to the queued request, so consuming an entry shifts the queue by one and the wrong method gets paired with the final response. Both failure modes are reachable from first-party handlers: `HttpServerExpectContinueHandler` and `HttpObjectAggregator` write `100 Continue` through this encoder. Skip `pollMethod()` when the response status class is `INFORMATIONAL` and delegate to the super method, mirroring the guard already present in `isContentAlwaysEmpty(HttpMessage)` in `HttpClientCodec`. `methodFlag` is intentionally left untouched on the interim path. `sanitizeHeadersBeforeEncode()` reads it for the CONNECT check, but that branch also requires `codeClass() == SUCCESS`, which a 1xx response never satisfies. Interim responses no longer shift the method queue, so HEAD and CONNECT are paired with their own final response. Encoder output for the two broken cases, measured with the tests added here: | scenario | before | after | |---|---|---| | `HEAD /a`, then `103`, then final `200 OK` with content | `HTTP/1.1 200 OK\r\ncontent-length: 4\r\n\r\nbody` | `HTTP/1.1 200 OK\r\ncontent-length: 4\r\n\r\n` | | pipelined `GET /a` + `HEAD /b`, then `103`, then `/a`'s `200 OK` with content | `HTTP/1.1 200 OK\r\ncontent-length: 6\r\n\r\n` | `HTTP/1.1 200 OK\r\ncontent-length: 6\r\n\r\nbody-a` | | `HEAD /` with `Upgrade`, then `101` | `HTTP/1.1 101 Switching Protocols\r\nconnection: upgrade\r\nupgrade: websocket\r\n\r\n` | unchanged | The first row is a HEAD response carrying a body, which per RFC 9110 section 9.3.2 must never happen — on a keep-alive connection the peer reads those bytes as the start of the next response. The second is a non-HEAD response losing its body while keeping `Content-Length`, leaving the peer waiting for six bytes that never arrive. Sequential request/response traffic recovers on its own, since an empty queue falls back to `METHOD_FLAG_OTHER`. Corruption needs either a 1xx preceding a HEAD response, or pipelining combined with a 1xx. 101 is included in `INFORMATIONAL` and is therefore covered by the guard, with no change on the wire. After switching protocols the codec is removed from the pipeline — by `upgradeFrom()` in `HttpServerCodec` for h2c, and by `WebSocketServerHandshaker` for WebSocket — so the entry left behind is discarded together with the handler. The third row above pins this. This also corrects CONNECT: previously a 1xx could consume the CONNECT entry, so the final 2xx response missed the `Transfer-Encoding` stripping in `sanitizeHeadersBeforeEncode()`. `codec-http` passes in full: 8973 tests, 0 failures. Co-authored-by: Norman Maurer <norman_maurer@apple.com> --------- Co-authored-by: HwangRock <157935545+HwangRock@users.noreply.github.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Backport netty#16766 and netty#17166 to 4.1 --------- Co-authored-by: Francesco Nigro <nigro.fra@gmail.com>
…etty#17205) Motivation: ReferenceCountedOpenSslContext strongly references every engine through its `engines` map (the SSL* -> engine reverse lookup used by the OpenSSL callbacks). As an SslContext is typically a long-lived singleton, a leaked engine stays reachable for the context's whole lifetime, so OpenSslEngine.finalize() - the backstop meant for that case - never runs and its native memory is freed only when the context is destroyed. Modification: Reintroduce OpenSslEngineMap as a class holding the engines as WeakReferences. A live engine is always strongly reachable via its SslHandler, so only a leaked one becomes collectable. The abstraction removed in netty#15444 was a strong wrapper that rightly added no value; weak values give it a reason to exist. Add OpenSslEngineTest.leakedEngineIsReclaimedWhileContextAlive. Result: A leaked OPENSSL engine is reclaimed per-engine independently of its context. For OPENSSL_REFCNT a leaked engine now also becomes collectable, so the ResourceLeakDetector reports it instead of it being pinned silently. (cherry picked from commit 26255b1) This also backports netty#15444 which is harmless because neither of those types are public. --------- Co-authored-by: Bryce Anderson <bl_anderson@apple.com>
…netty#17209) Auto-port of netty#17052 to 4.1 Cherry-picked commit: a96226c --- ## Motivation The Netty project has well-established coding conventions defined in its checkstyle configuration, but these are only enforced at build time and require specific tooling setup. Without an `.editorconfig` file, each IDE and editor must be configured individually to match the project's style, leading to inconsistent formatting, unnecessary whitespace changes in commits, and friction for contributors. `.editorconfig` is auto-detected by IntelliJ IDEA, VS Code, and many other editors — no plugin or configuration required. This lowers the barrier for new contributors and ensures consistent style out of the box. ## Modification Added `.editorconfig` at the project root with settings derived from the project's existing conventions, verified against actual source files and the canonical checkstyle configuration at [`netty-build/common/src/main/resources/io/netty/checkstyle.xml`](https://github.com/netty/netty-build/blob/master/common/src/main/resources/io/netty/checkstyle.xml) — `LineLength = 120`, `FileTabCharacter` (tab prohibition), `NewlineCheck` (LF), `NewlineAtEndOfFile`, trailing whitespace prohibition, UTF-8 charset: | File type | Indent | Details | |-----------|--------|---------| | `*.java` | 4 spaces | 120-char max line width | | `*.c`, `*.h` | 4 spaces | Native transport code | | `*.xml` | 4 spaces | POM and config files | | `*.{yml,yaml,json,js,css}` | 2 spaces | Build configs, web assets | | `*.{md,txt}` | 2 spaces | Documentation | | `*.properties` | 2 spaces | Maven wrapper, native-image config | | `*.sh` | 4 spaces | Build and CI scripts | | `Makefile` | tab | Required by GNU make | Global defaults for all files: UTF-8 encoding, LF line endings, final newline at end of file, no trailing whitespace. ## Result This is a formatting-only convention change with no behavioral impact. It ensures every editor shows code with the correct style automatically and provides the necessary foundation for automatic code formatters to enforce consistent style in CI pipelines. Addresses [netty#15642](netty#15642). Co-authored-by: Vasily Pelikh <Java.deveveloper@gmail.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation: We used an outdated version Modifications: Update to latest release Result: Use latest release
Motivation: We released a new version of tcnative. Modifications: Update to latest version Result: Use latest tcnative version --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com> (cherry picked from commit 4670762) --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com>
[CORS: Don't override vary header if it already exists](netty@1a896eb) Motivation: Netty's CorsHandler silently overwrites existing Vary headers, enabling cache poisoning and sensitive information disclosure. Modifications: - Only set vary header if it not already exists - Add unit test Result: No more cache poisoning possible --- [Encoding-side validation of MQTT fields](netty@9975553) Motivation: The MqttEncoder should not produce malformed messages if client id, topics, or usernames contain illegal characters. Modification: Add validation of client identifier, will topic, and username to the encoding path of CONNECT messages, and also to the topic name when encoding PUBLISH messages. Result: The encoder will now throw an exception if any of these fields contain a NUL byte. --- [Bound SCTP fragmented message buffering](netty@c07b97c) --- [Correctly handle ClientHello with a handshake header split across TLS…](netty@a8f53f2) … records Motivation: The handshake header (HandshakeType + 3-byte length = 4 bytes) may be delivered across multiple TLS records. SslClientHelloHandler assumed these 4 bytes were always contained in the first record and read them directly from it, which is incorrect when the header spans records. Modification: - Read the handshake header directly from the record only when the full 4 bytes are contained in it. - Otherwise aggregate the record payloads into handshakeBuffer and read the handshakeType and handshakeLength from the buffer once at least 4 bytes are available. - Aggregate the handshake header together with the body and slice past the 4-byte header once the full ClientHello has been buffered. - Add SniHandlerTest cases for fragment sizes 1-4. Result: ClientHello messages whose handshake header is fragmented across multiple TLS records are parsed correctly. --- [Harden the SOCKS4/5 input validation at encoding time](netty@4625406) Motivation: SOCKS4 is a delimiter-based protocol, and does not support NUL bytes in string or byte-sequence fields. SOCKS5 is a Tag-Length-Value protocol, and does not support lengths greater than 255. Modification: Add validations for NUL bytes and lengths for SOCKS4 and 5, respectively, and ensure both client- and server-side encoders behave correctly. Add tests to verify the correct handling of boundary conditions. Result: Correct delimiter and length handling in the SOCKS4/5 encoders, even when integrators use custom implementations of the message types. --- [Do not re-aggregate the ClientHello on every received TLS record](netty@bb74154) --- [Validate trust manager configuration](netty@5f1888e) Motivation: Certain trust manager configurations were not being validated against the configured endpoint identification algorithm. Modification: Add a check that fails fast when the configured trust manager does not support the required verification mode. Result: Misconfigured trust manager setups are now rejected explicitly instead of failing silently. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com> Co-authored-by: yawkat <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at> Co-authored-by: Violeta Georgieva <696661+violetagg@users.noreply.github.com> --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com> Co-authored-by: yawkat <jonas.konrad@oracle.com> Co-authored-by: multicode <multicode@yawk.at> Co-authored-by: Violeta Georgieva <696661+violetagg@users.noreply.github.com>
This forward-ports the DSE netty fork to the netty-4.1.137.Final release of netty. [maven-release-plugin] copy for tag netty-4.1.137.Final
emerkle826
force-pushed
the
dse-netty-4.1.137
branch
2 times, most recently
from
August 14, 2026 13:19
5425f18 to
337ff75
Compare
This patch updates the build-and-publish workflow to use a script to correctly publish packages to GitHub. For publishing to Datastax artifactory, you will need to download the merged-local-satging bundle from the GitHub build and then publish using approrpiate credentials, possibly behind the AWS VPN fro lab Artifactory.
emerkle826
force-pushed
the
dse-netty-4.1.137
branch
from
August 14, 2026 19:13
337ff75 to
c8b8ea8
Compare
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
9 participants
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.
No description provided.