[DO NOT MERGE] Upgrades Netty to 4.1.135#41
Open
tiagomlalves wants to merge 212 commits into
Open
Conversation
Backport of netty#15752 Motivation: Reflective field accesses were initially disabled in netty#10428 because back then native-image did not support `Unsafe.staticFieldOffset()`. This is no longer an issue since GraalVM 21.2.0 for JDK 11 (July 2021) see oracle/graal@f97bdb5 Modification: Remove the check for native-image before accessing fields using `Unsafe`. Result: Netty can directly access fields necessary for `PlatformDependent0` initialization using `Unsafe`.
Motivation: There are JDK updates we should use them Modifications: - Update JDK versions Result: Use latest JDK versions
Motivation: An invalid content length in a continue request would induce HttpObjectAggregator to throw a NumberFormatException. Modification: Use the existing isContentLengthInvalid to guard the getContentLength call in continue request processing. Result: No exception thrown.
…etty#15798) Motivation: The consumeCPU call can disturb potential optimizations, so its desirable to be able to benchmark the case where it's not present. Modification: Add a zero-delay parameter to the benchmark, and make sure the consumeCPU call is skipped in that case. Result: More flexible benchmark.
Motivation: When the padding of an HTTP/2 DATA frame exceeds the bounds of the frame, an IndexOutOfBoundsException would be thrown instead of the expected Http2Exception: ``` Exception in thread "main" java.lang.IndexOutOfBoundsException: readerIndex: 1, writerIndex: 0 (expected: 0 <= readerIndex <= writerIndex <= capacity(4)) at io.netty.buffer.AbstractByteBuf.checkIndexBounds(AbstractByteBuf.java:112) at io.netty.buffer.AbstractByteBuf.writerIndex(AbstractByteBuf.java:135) at io.netty.buffer.WrappedByteBuf.writerIndex(WrappedByteBuf.java:132) at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readDataFrame(DefaultHttp2FrameReader.java:408) at io.netty.handler.codec.http2.DefaultHttp2FrameReader.processPayloadState(DefaultHttp2FrameReader.java:244) at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readFrame(DefaultHttp2FrameReader.java:164) ``` Modification: Instead of verifying the padding size against the full payloadLength, which includes the pad length, move the check to lengthWithoutTrailingPadding where the exact number of remaining bytes is known. Result: Proper protocol error. Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
Motivation: BlockHound version 1.0.15.RELEASE comes with newer byte-buddy dependency Modification: - Bump BlockHound version as byte-buddy dependency is updated Result: BlockHound version 1.0.15.RELEASE with newer byte-buddy dependency
…ty#15800) Motivation: I've gotten allocation profile data from "a large e-commerce application", which has sizable allocation volume at 32 KiB and 64 KiB, with very little in between that isn't covered by the existing size classes. Modification: Add 32 KiB and 64 KiB size classes to the adaptive allocator. Make the adaptiveChunkMustDeallocateOrReuseWthBufferRelease test more size-class agnostic. Result: Nearly 50% memory usage reduction in this e-commerce application use case, according to the allocation pattern simulator for the 1024 live buffers case, which brings adaptive on par with the pooled allocator for this use case. Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
(… (netty#15810) …netty#15805) Motivation: Fixes netty#15804. In java 25, some mqtt flows cause `IndexOutOfBoundsException` as unsafe is no longer used. Modification: Use `MqttEncoder.writeEagerUTF8String` which precalculates expected buffer size instead of `MqttEncoder.writeUnsafeUTF8String`. Result: No more IndexOutOfBoundsException in MqttEncoder and Java 25 Co-authored-by: Dmytro Dumanskiy <doom369@gmail.com>
Motivation: If the ByteToMessageDecoder gets a reentrant channelRead call, we need to avoid closing or otherwise manipulating the cumulation buffer. Modification: Guard reentrant channelRead calls by queueing up messages and letting the top-level call process them all in order. Result: Reentrant calls to ByteToMessageDecoder.channelRead will no longer cause weird IllegalReferenceCountException on the cumulation buffer.
Motivation: Comment on the sizeId2sizeTab table looks like it's referencing wrong field, going by how its created and used. Modification: Make it reference the nSizes field, which is used in the computation of the table. Result: Clearer comment. Fixes netty#15333
…5846) Motivation: AsciiString should have a consistent hash code regardless of the platform we're running on, because we don't know of the hash code gets exposed or used across platforms. An optimized version of the hash code was assuming a little-endian platform but could end up being used on big endian platforms. Modification: Add a condition that the safe, platform-agnostic hash code implementation should be used on big endian platforms. Result: The AsciiString hash code values are now always the same regardless of the platform and JVM configuration we're running on.
Motivation: GitHub has deprecated the macOS 13 runners and will be removing them. Details in actions/runner-images#13046 Modification: Replace the runner OS for all Intel Mac builds with macos-15-intel. Result: MacOS builds now run on the latest and last version of MacOS that will have Intel support.
Motivation: Better javadoc for `CompositeByteBuf#release` method Modification: Add javadoc Result: Fixes netty#15863 --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation: The `AbstractHttp2StreamChannel` subclasses use a synthetic id in their `ChannelId`, but include the real stream id in their `toString()` method. Using two different ids makes it harder to correlate the two in logs. The synthetic ids are necessary because the stream id is assigned at a protocol level at some point after the channel has been created. Modification: Include the synthetic id in the `AbstractHttp2StreamChannel.toString()` output. Result: Easier to correlate HTTP/2 channel and stream ids, to help with debugging.
### Motivation The failure was found with [NonDex](https://github.com/TestingResearchIllinois/NonDex), which explores non-determinism in tests. These tests can cause test failure under different JVMs, etc. The non-determinism in the test class `io.netty.handler.codec.http.websocketx.extensions.WebSocketExtensionUtilTest` in `codec-http` module is caused by the `WebSocketExtensionUtil` storing extension parameters in a HashMap, whose iteration order is nondeterministic. When `computeMergeExtensionsHeaderValue` serializes these parameters using `data.parameters().entrySet()`, the output ordering varies depending on the underlying hash iteration order. #### Modifications Replacing the parameter hashmaps with `LinkedHashMap` fixes the issue by preserving deterministic insertion/iteration order. ### Result After the 3-line modification, the tests no longer depend on non-deterministic order of HashMap iteration. ### Failure Reproduction 1. Checkout the 4.2 branch and the recent commit I tested on: 0a3df28 2. Environment: > openjdk 17.0.16 2025-07-15 OpenJDK Runtime Environment (build 17.0.16+8-Ubuntu-0ubuntu124.04.1) OpenJDK 64-Bit Server VM (build 17.0.16+8-Ubuntu-0ubuntu124.04.1, mixed mode, sharing) > Apache Maven 3.9.11 3. Run test with [NonDex](https://github.com/TestingResearchIllinois/NonDex), for example: ``` mvn -pl codec-http edu.illinois:nondex-maven-plugin:2.1.7:nondex \ -Dtest=io.netty.handler.codec.http.websocketx.extensions.WebSocketExtensionUtilTest#computeMergeExtensionsHeaderValueWhenNoConflictingUserDefinedHeader \ -DnondexRerun=true -DnondexRuns=1 -DnondexSeed=974622 \ -Djacoco.skip -Drat.skip -Dpmd.skip -Denforcer.skip ``` 4. Observe the test failure Co-authored-by: lycoris106 <53146702+lycoris106@users.noreply.github.com>
netty#15894) …size (netty#15859) Motivation: If the size of the input buffer is smaller than the configured block size (64 KB by default), then ZstdEncoder.write only produces an empty buffer. With the default sizes, this makes the encoder unusable for variable size content. This is for fixing the issue(netty#15340) Modification: Return the uncompressed data if for small size data Result: Fixes netty#15340 Signed-off-by: xingrufei <qhdxssm@qq.com> Co-authored-by: skyguard1 <qhdxssm@qq.com>
Motivation: Zstd compression fails for sources larger than 31MB since the default max encode size is 32M. This is PR is to change default max encode size to `Integer.MAX_VALUE` so that `ZstdEncoder` can works for large data Modification: Change default max encode size to `Integer.MAX_VALUE` so that `ZstdEncoder` can works for large data Result: Fixes netty#14972 `ZstdEncoder` can works for the large data Signed-off-by: xingrufei <qhdxssm@qq.com> Signed-off-by: skyguard1 <qhdxssm@qq.com> Co-authored-by: skyguard1 <qhdxssm@qq.com>
netty#15916) …netty#15911) Motivation: The splice implementation was quite buggy and also not really fits in the whole pipeline idea so we removed it already for 5.x. Let's mark it as deprecated as well in earlier versions. Modifications: Add deprecation to methods Result: Prepare users that these methods / functionallity will go away
… (netty#15917) Motivation: We used Bootstrap to bootstrap EpollServerSocketChannel in the test which is not correct. Modification: Use ServerBootstrap Result: Correct bootstrap used in test
…nEventLoop() results (netty#15927) ## Motivation A race condition exists in `NonStickyEventExecutorGroup.NonStickyOrderedEventExecutor` that can cause `inEventLoop()` to return incorrect results, potentially leading to deadlocks and synchronization issues. ## Modifications - Restore `executingThread` in the exception handler before setting `state` to `RUNNING` in `common/src/main/java/io/netty/util/concurrent/NonStickyEventExecutorGroup.java:262` - Add test `testInEventLoopAfterReschedulingFailure()` to verify `inEventLoop()` returns true after failed reschedule attempt ## Result The executor correctly maintains `executingThread` even when rescheduling fails, and `inEventLoop()` consistently returns accurate results. ## Details When the executor processes `maxTaskExecutePerRun` tasks and needs to reschedule itself, if `executor.execute(this)` throws an exception (e.g., queue full), the catch block previously reset `state` to `RUNNING` but did not restore `executingThread`, leaving it `null`. This fix ensures `executingThread.set(current)` is called before `state.set(RUNNING)` to maintain the invariant that when `state == RUNNING`, the executing thread is properly tracked. ## Testing The new test `testInEventLoopAfterReschedulingFailure()` has been verified to fail without the fix and pass with the fix applied.
Motivation: lz4-java has been discontinued (see [README](https://github.com/lz4/lz4-java)) and I've forked it to fix [CVE-2025-12183](https://sites.google.com/sonatype.com/vulnerabilities/cve-2025-12183). org.lz4:lz4-java:1.8.1 contains a redirect, but the latest version needs new coordinates. Modification: Update to latest version, with the forked group ID. Result: No more CVE-2025-12183. Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
netty#15952) …re (netty#15949) Motivation: Connect is an outbound operation and so should onyl fail the promise. Modifications: - Don'T call fireExceptionCaught(...) - Add unit test Result: Correct exception reporting for connect failures
netty#15957) …… (netty#15954) …e DomainSocketAddress via LinuxSocket Motivation: We did not correctly handle abstract namespaces with UDS on linux when creating the DomainSocketAddress Modification: Correctly detect and handle abstract namespaces on linux Result: Correct code
…15965) Motivation: DefaultSctpServerChannelConfig did miss to respect SO_BACKLOG which is not expected Modifications: Correctly handle SO_BACKLOG Result: Correctly handle all ChannelOptions for SctpServerChannelConfig
…netty#15962) Motivation: HttpServerCodec always constructs an `EmptyLastHttpContent` and passes it to the Handler chain when processing OPTIONS requests, The `EmptyLastHttpContent` propagate through the handler chain. However, the CORS handler might still propagate the EmptyLastHttpContent to downstream handlers via fireChannelRead(), causing subsequent handlers to receive only this empty content and lose access to the original request URL. Because `CorsHandler` does not consume this message, it calls `ctx.fireChannelRead(msg)` for the EmptyLastHttpContent. Downstream handlers then observe: No HttpRequest A LastHttpContent with empty content This often breaks other handlers that rely on receiving the `HttpRequest` first or expect consistent HTTP message. This PR fixes an issue in `CorsHandler` where, after handling a CORS preflight (OPTIONS) request, the handler still propagates the subsequent `EmptyLastHttpContent` sent by the `HttpServerCodec` to downstream handler. Modification: - `CorsHandler` track handled preflight and consume the following HttpContent, LastHttpContent until the next HttpRequest is forwarded instead of firing it downstream. - Add tests Result: Fixes netty#15148 It also improves compatibility with application frameworks and routing handlers that expect well-formed HTTP request/response flows. Signed-off-by: skyguard1 <qhdxssm@qq.com> Co-authored-by: skyguard1 <qhdxssm@qq.com> Co-authored-by: xingrufei <xingrufei@yl.com> Co-authored-by: code-xing_wcar <code.xing@wirelesscar.com>
netty#15970) …… (netty#15896) …an error Motivation: We should close the Channel and fail the future of the bootstrap if during setting a ChannelOption we observe an error. At the moment we just log which might make things hard to debug and leave the Channel in an unexpected state. People can go back to the old behavior by using `-Dio.netty.bootstrap.closeOnSetOptionFailure=false`. Modifications: - Adjust code to also close and faile the future - Add testcases Result: Related to netty#15860 (comment)
Motivation: If initializeIfNecessary(...) fails we still need to ensure that we release the message. Modifications: Correctly release message before rethrow or fail promise Result: Fix possible buffer leak
) (netty#15983) Motivation: We used some odd number for the length, just use the exact length that we need. Modifications: - Allocate a byte[] of 25 length and remove odd comment Result: Cleanup
New vulnerability: [CVE-2025-66566](GHSA-cmp6-m4wj-q63q) Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
…te (netty#16847) Motivation: The catch block in `ChannelInitializer.initChannel(ChannelHandlerContext)` currently attributes the explicit `exceptionCaught(...)` call to two reasons that don't match the current code: ```java } catch (Throwable cause) { // Explicitly call exceptionCaught(...) as we removed the handler before calling initChannel(...). // We do so to prevent multiple calls to initChannel(...). exceptionCaught(ctx, cause); } finally { if (!ctx.isRemoved()) { ctx.pipeline().remove(this); } } ``` - The handler is **not** removed before `initChannel(...)` is called — removal happens in the `finally` block immediately below. - Re-entrance is guarded by the `initMap.add(ctx)` check at the top of the method, **not** by this catch block. Both clauses of the comment mislead a reader trying to understand why the catch block exists. Modification: Rewrite the two-line comment to name the actual mechanisms: - the `exceptionCaught(...)` call routes the failure into the pipeline; - re-entrance is guarded by `initMap.add(ctx)` above; - handler removal happens in the `finally` block below. No code change. Result: Future readers of `initChannel(ChannelHandlerContext)` can trust the comment instead of having to cross-check it against the surrounding code. Fixes netty#16846. The same wording exists on 4.1, 4.2, and 5.0; targeting 4.1 so auto-port carries it forward.
…6856) Manual backport of netty#16810 to 4.1. This is not a literal bot auto-port: the Java source, tests, fuzz test, and microbenchmark changes from the 4.2 PR applied as-is, but the root `pom.xml` needed a 4.1-specific conflict resolution. The backport keeps 4.1's existing `junit.version` (`5.12.1`) and adds only `jazzer.version` (`0.30.0`) for the new fuzz test, instead of taking 4.2's newer JUnit/JUnit Platform version lines. Cherry-picked source commit: a42c7fc --- Motivation: `HttpConversionUtil.toHttp2Headers` currently depends on `java.net.URI` for absolute-form request-target parsing. On JDKs that still enforce older URI syntax, path or query characters that appear in real HTTP request-targets can make HTTP/1.x to HTTP/2 conversion fail before `:path` is produced. Netty only needs URI parsing for the lower-frequency `scheme://authority` validation/extraction path. The hot path/query extraction can follow the same lightweight parsing shape used by Vert.x while avoiding full URI parsing and avoiding a try/catch fallback. Modification: - Split request-target path and query parsing into Vert.x-shaped `parsePath` and `parseQuery` helpers, with comments for Netty-specific differences. - Keep `URI` parsing for `scheme://authority` validation/extraction only after stripping path/query/fragment data. - Preserve origin-form and asterisk-form behavior. - Add regression tests for characters rejected by `java.net.URI`, authority-only and missing-authority absolute-form targets, empty query/fragment handling, and malformed authority validation. - Add a Jazzer fuzz test that compares the new behavior against the old URI-based conversion using broad `consumeString(128)` request-target input and narrow documented compatibility exceptions. 4.1 CI note: The Jazzer test is opt-in via `JAZZER_FUZZ=1` on this branch. Netty 4.1 CI still runs Linux jobs on old CentOS images whose glibc is too old for Jazzer's native driver. The initial CI failure was: `HttpConversionUtilFuzzTest.currentConversionMatchesOldUriBasedConversion` → `Failed to run Agent.install` → `libjazzer_driver_*.so: /lib64/libc.so.6: version 'GLIBC_2.14' not found`. The deterministic `HttpConversionUtilTest` regression tests still run by default; the fuzz oracle remains available on compatible hosts by setting `JAZZER_FUZZ=1`. Result: HTTP/2 conversion no longer relies on full `java.net.URI` parsing for request-target path/query extraction, while preserving meaningful existing behavior and continuing to validate/extract scheme and authority through URI where appropriate. Verification performed locally: - Default CI-like targeted path: `./mvnw -pl codec-http2 -am -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dforbiddenapis.skip=true -Danimal.sniffer.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=HttpConversionUtilTest,HttpConversionUtilFuzzTest test` — 31 tests, 0 failures, 1 skipped (`HttpConversionUtilFuzzTest`). - Opt-in fuzz path: `JAZZER_FUZZ=1 ./mvnw -pl codec-http2 -am -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dcheckstyle.skip=true -Dforbiddenapis.skip=true -Danimal.sniffer.skip=true -Dsurefire.failIfNoSpecifiedTests=false -Dtest=HttpConversionUtilFuzzTest test` — fuzz test ran successfully on the local JDK 21 host. - Checkstyle/compile path: `./mvnw -pl codec-http2 -am -DskipTests -Drevapi.skip=true -DskipJapicmp -DskipHttp2Testsuite -DskipAutobahn -Dforbiddenapis.skip=true -Danimal.sniffer.skip=true test` — build success. Notes: - Java LSP diagnostics were unavailable locally because `jdtls` is not installed in the environment. - The 4.1 backport keeps 4.1's existing `junit.version` and only adds `jazzer.version`; `codec-http2` excludes Jazzer's JUnit/JUnit Platform transitives so the branch-managed test stack is used. --------- Co-authored-by: multicode <multicode@yawk.at>
…y initial CRLF characters are permitted (netty#16861) Motivation: RFC 9112 permit empty lines (CR LF sequences) prior to the request line, but we were skipping over any ISO control characters. This is parsing leniency beyond what the standard mandates and can be a security liability. Modification: Expand the scope of the "strict line parsing" decoder configuration option to also include enforcing that any octets prior to the initial line can only be the line separators CR LF. Result: Strict line parsing covers more cases. Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: We need to correctly handle subnets for ipv6 Modifications: - Correctly handle subnets - Add unit test Result: Correctly filter ipv6 addresses
Motivation: The RedisArrayAggregator was pre-allocating the aggregating ArrayList instance based on just the array header message. Modification: Add a RedisArrayAggregator constructor that takes a max number of elements as an argument. Deprecate the other constructor and make it use 1.000.000 as a default, defined by a new system property. Why 1.000.000? It seemed big enough to not cause too many compatibility problems from this change, yet small enough to somewhat constrain the resource usage from nefarious peers. Result: Less exposure to excessive resource usage by adversarial peers. Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: To guard against unbounded memory usage we should enforce a limit when we try to decode the length. This needs to fit into a signed 64 bit integer. Modifications: - Enforce limit during decoding - Add unit test Result: Ensure we will limit the number of bytes we buffer
Motivation: Netty's DNS resolver uses a predictable PRNG for generating DNS transaction IDs and defaults to a static UDP source port. This combination reduces the entropy of DNS queries, enabling DNS Cache Poisoning (Kaminsky attack). Modifications: - Replace usage of ThreadLocalRandom with SecureRandom Result: query id is not predictible anymore
…etty#16868) Motivation: We need to ensure we don't wrap X509TrustManager into X509ExtendedTrustManager as this will silently disable hostname verification Modifications: - Remove wrapping code - Change InsecureTrustManagerFactory to disable verification Result: Correctly leaverage hostname verifications that is provided by the JDK internally.
…gth (netty#16852) Motivation: `MqttDecoder.decodePayload`'s `default` branch returns `null` for payload-less message types (`PINGREQ`, `PINGRESP`, `DISCONNECT`, `AUTH`, `CONNACK`, `PUBACK`, `PUBREC`, `PUBREL`, `PUBCOMP`) without checking that `bytesRemainingInVariablePart` is `0`. A fixed header that claims a non-zero Remaining Length for one of these types is silently accepted, and the leftover bytes are interpreted as another packet. The reporter's exact repro from netty#16851 is `C0 02 D0 00`: a `PINGREQ` with Remaining Length `2` (invalid per [MQTT 3.1.1](https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html) and [MQTT 5.0](https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html), which both require Remaining Length `0` for `PINGREQ`) is currently decoded as a valid `PINGREQ` followed by a valid `PINGRESP`. Modification: Call `validateNoBytesRemain(0)` at the start of the `default` branch. The helper — already used by the `SUBSCRIBE` and `UNSUBSCRIBE` payload decoders — subtracts `0` from `bytesRemainingInVariablePart` and throws `DecoderException` when the result is non-zero, surfacing a single invalid message instead of letting leftover bytes leak into the next decode pass. Added `MqttCodecTest#testPingReqWithNonZeroRemainingLengthIsRejected` that feeds the reporter's exact byte sequence and verifies a single failed message is produced. Confirmed it fails on the unfixed code (`expected: <true> but was: <false>`) and passes with the fix; all 112 `codec-mqtt` tests pass locally. Result: Malformed payload-less MQTT frames are flagged as a single invalid message rather than allowed to silently spill into the next packet. The fix applies uniformly to every message type that goes through the `default` branch. Fixes netty#16851. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: Recent merges from private repos caused these to sneak past CI. Modification: Silence the revapi warnings, because the `@Skip` annotation being removed from a method is harmless. Result: Build should pass again. (cherry picked from commit e067b6e)
…ory (netty#16866) Motivation: We need to verify that the contained length actually is valid before retaining the buffer as otherwise we will leak memory. Modifications: - Verify length is valid - Only retain buffer if we not throw exceptio - Add unit test Result: Ensure we never leak memory when handling invalid messages
…out (netty#16871) Motivation: There were various issues here... first of in SslClientHelloHandler we used 16MB as default maximum limit for the client hello which could lead to huge memory usage. Second we used no limit at all and no timeout in AbstractSniHandler and its subclasses which is even worse. Modifications: - Use 64KB as default limit - Use 10 seconds as default timeout (same as in SslHandler) Result: Saner defaults which helps to guard against high memory usage
Motivation: We should only cache the CNAME if it is part of the queried domain to ensure the name server is really authoritive for it and not provide us incorrect data. Modifications: - Only cache if CNAME is part of the queried domain - Add unit test Result: No more DNS Cache Poisoning (Bailiwick Bypass) possible
…#16876) Motivation: Clients might choose to omit including the max concurrent streams setting in the SETTINGS frame they send to the server. Or the client might open streams before sending the settings frame. In which case the server should enforce its own limits regardless. Modification: In the AbstractHttp2ConnectionHandlerBuilder, make sure to configure the max active streams on the remote endpoint as early as possible. Add tests to prove that the enforcement works. Result: The max active streams setting is always enforced. --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation: Netty's `DnsResolveContext` insufficiently validates the bailiwick of NS records, enabling DNS Cache Poisoning. An attacker controlling an authoritative name server for a subdomain can poison the cache for parent domains (like `.co.uk`). Modifications: - Add correct Bailiwick checks when caching NS. - Adjust tests Result: No more risk of cache poising
… cases (netty#16880) Motivation: We need to ensure we also release the buffer if the flow controller for example throws, missing to do so might result in OOME. Modifications: Move buf.release() to finally block Result: Always release buffer
…6886) Motivation: Brotli and Zstd should using the same maxAllocation limits as gzip and zlib, if these limits are defined. This leads to unification of usage and as a prtoection against zip bombs even on memory-strained environments, as well as allowing larger archives on explicitly configured envirironments. Modifications: Field maxAllocation of class HttpContentDecompressor is now passed to constructors of BrotliDecoder and ZstdDecoder classes. Result: If user configures max allocation for decryption, then the same allocation will be used for decryption of zstd and Brotli loads (cherry picked from commit 5a52600) Co-authored-by: fedinskiy <fdudinsk@redhat.com>
Motivation: SETTINGS_MAX_HEADER_LIST_SIZE is advisory per RFC 9113 §6.5.2. When acting as a server, the remote peer is a client whose advertised value reflects what it is prepared to receive. Honoring arbitrarily small values creates a DoS vector: a client setting this to 1 byte would prevent any valid response from being sent. Modification: When running as a server, the `DefaultHttp2ConnectionEncoder` now ignore the max header list size setting from clients. Result: Clients can no longer cheaply induce errors on an HTTP/2 server by advising unreasonably smaller header size limits. The behavior of ignoring the max header list size matches other H2 implementations, like nghttp2, nginx, and envoy. --------- Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
…ory allocation (netty#16894) Auto-port of netty#16850 to 4.1 Cherry-picked commit: 0bd1657 --- Motivation: The ZstdDecoder does not currently constrain the memory the underlying zstd decoder may allocate per stream. RFC 8478 ("Security Considerations") states: An attacker may provide correctly formed compressed frames with unreasonable memory requirements. A decoder must always control memory requirements and enforce some (system-specific) limits in order to protect memory usage from such scenarios. The most common instance of this attack is a tiny (a few hundred bytes) Zstandard frame whose header declares a very large Window_Size — for example Window_Log = 31, a 2 GiB sliding window. When such a frame is fed to libzstd, the declared window is allocated as native memory before any actual content is decoded, so a handful of concurrent connections is enough to drive a server into OOM. This is exactly the class of "unreasonable memory requirements" calls out. The libzstd manual exposes a parameter (ZSTD_d_windowLogMax) specifically to bound this, and zstd-jni surfaces it via ZstdInputStreamNoFinalizer.setLongMax(int). The fix is to wire it up in ZstdDecoder with a sensible default. Note that the existing maximumAllocationSize parameter only caps the Netty-side output buffer handed to the next handler; it does not bound the native window memory libzstd allocates, so the attack surface remained open prior to this change. Modification: Add a new public constant DEFAULT_MAX_WINDOW_LOG = 27 (128 MiB window), a reasonable default for general-purpose server use that still leaves significant headroom for typical zstd CLI output (whose default Window_Log is ≤ 23). Add a new constructor ZstdDecoder(int maximumAllocationSize, int maxWindowLog). maxWindowLog is validated to be in [10, 31] per RFC 8478. The existing ZstdDecoder(int maximumAllocationSize) constructor now delegates to the new one with DEFAULT_MAX_WINDOW_LOG, so the default behavior of existing call sites is preserved at the API level while no longer being vulnerable to the attack above. In handlerAdded, after the existing setContinuous(true), call zstdIs.setLongMax(maxWindowLog). Frames whose Window_Log exceeds the configured cap are rejected by libzstd with a ZstdIOException("Frame requires too much memory for decoding"), which the existing catch (Exception e) path wraps into a DecompressionException and transitions the handler to CORRUPTED. Result: Fixes #. ZstdDecoder now controls the per-stream native window memory libzstd will allocate, addressing the "unreasonable memory requirements" scenario in RFC 8478. Users who legitimately need to accept frames with very large windows can opt in via the new constructor with a larger maxWindowLog. --------- Co-authored-by: skyguard1 <qhdxssm@qq.com> Co-authored-by: Chris Vest <christianvest_hansen@apple.com> Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation: When parsing a PROXY protocol v2 header containing nested PP2_TYPE_SSL TLVs at depth two or greater, the underlying cumulation buffer can remain retained after the message is deallocated. This happens because deeply nested TLVs are not flattened into the main tlvs() list, causing them to be missed during the standard release process. Modifications: - Update the release logic in HAProxyMessage to correctly identify and recursively release deeply nested TLVs. - Introduce a releaseTlvs helper method to handle the recursive release without altering the existing public API behavior of the tlvs() list. Result: The cumulation buffer is properly released when handling deeply nested SSL TLVs, preventing memory leaks while preserving backwards compatibility. Co-authored-by: Violeta Georgieva <696661+violetagg@users.noreply.github.com>
Motivation: We didn't correctly handle all error cases and special cases when receiving FDs via UDS. This could lead to leaked FDs. Modifications: - Correctly handle truncation - Correctly handle the case when we receive more then one FD Result: No more FD leak
…umber of fragments (netty#16875) Motivation: SctpMessageCompletionHandler did not inforce any limits and so could cause unbound memory usage. Modifications: - Add new constructor that takes a limit for the maximum incomplete SCTP messages in flight and the number of fragments - Use sane defaults - Add unit tests Result: No more unbound memory usage
…isArrayAggregator (netty#16878) Motivation: The RedisArrayAggregator needs to ensure it releases buffered messages when the handler is removed as otherwise it will result in memory leaks. Modifications: - fire exception when channel is closed while we still aggregate - release messages on handler removal - add unit tests Result: Fix memory leak
Motivation: We didn'T limit the maximum number of nested array and so we were in risk of OOME Modifications: - Add constructor that allows to set a limit and use 1024 by default - Add unit test Result: Limit maximum nested arrays
|
The branch looks sensible to me, I have one question though. It seems to me that the netty branch merged into our fork is equal to netty's The merge commit seems to create version 4.1.135, which if I am reading it correctly bumps the patch segment of the version number (from 134 to 135) and adds the dse specific bit of |
Adds GitHub action to build Netty artifacts for all platforms.
Adds Bob skill to automate merging upstream changes in our branch and resolving merge conflicts.
This forward-ports the DSE netty fork to the netty-4.1.135.Final release of netty. [maven-release-plugin] copy for tag netty-4.1.135.Final
tiagomlalves
force-pushed
the
dse-netty-4.1.135
branch
from
6070608 to
4f5f5d4
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
14 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.