Motivation:
This bug could occasionally cause SSL handshakes to time out, because the server-side handshake would fail to resume its event loop.
Modification:
Async delegate SSL tasks now lower their NEED_TASK status after they have executed, but before they run their completion callback.
This is important because the completion callback could be querying the handshake status.
This could cause the task delegator thread and the event look to race.
If the event look queries the handshake status first, it might think that it still needs to delegate another task.
If this happens, the delegator find a null task, and then fail to resume the event loop, causing the handshake to stall.
Result:
This data race no longer causes handshake timeouts.
Motivation:
We need to ensure we call wrap as long as there is something left to be send to the remote peer in cases of non-application data (like for example alerts).
Modifications:
Check the pending data and based on it return NEED_WRAP even when the handshake was done.
Result:
Always produce alerts etc
Motivation:
We should have guards in place to check increment or decrement count in `ReferenceCountUtil`. Increment and Decrement counts must be a positive integer.
Modification:
Added `ObjectUtil#checkPositive` checks.
Result:
Prevent release due to invalid count.
Motivation:
We the build failed we should ensure we also include hs_err* files as it may have failed due a native crash
Modifications:
Adjust path matching to include hs_err as well
Result:
Easier to debug native crashes
Add an ByteBuf -> Buffer adaptor
Motivation:
For migration of APIs from `ByteBuf` to `Buffer` sometime we may have to bridge APIs which use a `ByteBuf` (eg: `ByteToMessageDecoder` at the moment) to APIs that use a `Buffer` even when the allocator for the `ByteBuf` isn't migrated to use `ByteBufAllocatorAdaptor`.
Modification:
- Add a simple copy adaptor that copies all data from `ByteBuf` to a new `Buffer`.
- I noticed we do not have a `writeBytes` method with offsets, so added that too.
Result:
One more adaptor to bridge old and new buffer APIs.
Motivation:
There is no decoder and encoder for TCP based DNS.
Result:
- Added decoder and encoder
- Added tests
- Added example
Result:
Be able to decode and encode TCP based dns
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
Due a bug in the statemachine we produced an decoding error when the GZIP footer was fragmented in some cases
Modifications:
- Fix statemachine
- Add testcase
Result:
Correctly decode GZIP in all cases
Motivation:
Since Java 7, there are new constructors available that allow us to avoid initialising the stack traces of certain exceptions.
Modification:
Use these constructors instead of overriding Throwable.fillInStackTrace.
Result:
Cleaner code
Motivation:
Users may want to clear the cache manually. For this it should be possible to access it first.
Modifications:
Change the visibility to public
Result:
Be able to clear the cache manually. Related to https://github.com/netty/netty/issues/11519
Motivation:
750d235 did introduce a change in ApplicationProtocolNegotiationHandler that let the handler buffer all inbound data until the SSL handshake was completed. Due of this change a user might get buffer data forever if the SslHandler is not in the pipeline but the ApplicationProtocolNegotiationHandler was added. We should better guard the user against this "missconfiguration" of the ChannelPipeline.
Modifications:
- Remove the ApplicationProtocolNegotiationHandler when no SslHandler is present when the first message was received
- Add unit test
Result:
No possible that we buffer forever
Motivation:
While its technical impossible that a chunk is larger than 64kb it still makes things easier to read and more robust to add some size checks to LzfDecoder.
Modifications:
Check the maximum length
Result:
More robust and easier to reason about code
Motivation:
At the moment FastLzFrameEncoder depends on the buffer type. This is not needed and may cause memory copies
Modifications:
Rewrite to be able to just act on the ByteBuf
Result:
Less memory copies
Motivation:
We did observe that the mutal auth tests are flaky on windows when running on the CI. Let's disable these for now.
Modifications:
Disable mutual auth tests on windows
Result:
More stable build. Related to https://github.com/netty/netty/issues/11489
Motivation:
For the code pattern of `Object.wait(...)` in `io.netty.util.concurrent.DefaultPromise.await0(...)`, it's better to follow the recommended code pattern according to [Object.wait(...)'s doc](https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#wait()):
```
synchronized (obj) {
while (<condition does not hold>)
obj.wait();
... // Perform action appropriate to condition
}
```
Modification:
Change the `Object.wait(...)`'s code pattern in `io.netty.util.concurrent.DefaultPromise.await0(...)`.
Result:
The `Object.wait(...)`'s code pattern in `io.netty.util.concurrent.DefaultPromise.await0(...)` meets the Java doc.
Motivation:
SelfSignedCertificate generates EC/RSA key pair and this should be explicitly mentioned in JavaDoc. Currently, only "RSA" was mentioned not "EC".
Modification:
Changed RSA to EC/RSA
Result:
Correct JavaDoc
Motivation:
Brotli4j had some changes for performance improvements. So we should upgrade to the latest version of Brotli4j.
See https://github.com/hyperxpro/Brotli4j/pull/27
Modification:
Upgraded Broti4j from 1.4.2 to 1.5.0.
Result:
Up-to-date Broti4j library.
Motivation:
As the release of JDK17 is getting closer and there are ea builds already we should ensure we can actually build netty with it.
Modifications:
- Add profile for JDK17
- Remove test-code that would fail with JDK17 due the changes in 4f4d0f5366.
Result:
Be able to build and run testsuite with JDK17
Motivation:
japicmp did fix some issues related to Java16+. Let's update so we can also enable it in later java versions
Modifications:
Update to 0.15.3
Result:
Be able to use japicmp with all java versions
Motivation:
FastLzFrameDecoder currently not use the allocator to alocate the output buffer. This means that if you use the PooledByteBufAllocator you still can't make use of the pooling. Beside this the decoder also does an uncessary memory copy when no compression is used.
Modifications:
- Allocate the output buffer via the allocator
- Don't allocate and copy if we handle an uncompressed chunk
- Make use of ByteBufChecksum for a few optimizations when running on a recent JDK
Result:
Less allocations when using FastLzFrameDecoder
Motivation:
There is no test case of `StringDecoder` here
Modification:
Need to add `StringDecoder` test case
Result:
Added test case of `StringDecoder`
Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
At the moment we not correctly propagate cancellation in some case when we use the PromiseNotifier.
Modifications:
- Add PromiseNotifier static method which takes care of cancellation
- Add unit test
- Deprecate ChannelPromiseNotifier
Result:
Correctly propagate cancellation of operation
Co-authored-by: Nitesh Kant <nitesh_kant@apple.com>
Motivation:
New versions of Bouncy Castle libraries are out and we should upgrade to them.
Modification:
Upgraded all Bouncy Castle libraries to the latest version.
Result:
The latest versions of Bouncy Castle libraries.
Motivation:
As suggested in [section 5.3.4 in http2 spec](https://datatracker.ietf.org/doc/html/rfc7540#section-5.3.4):
> When a stream is removed from the dependency tree, its dependencies can be moved to become dependent on the parent of the closed stream. The weights of new dependencies are recalculated by distributing the weight of the dependency of the closed stream proportionally based on the weights of its dependencies.
For example, we have stream A and B depend on connection stream with default weights (16), and stream C depends on A with max weight (256). When stream A was closed, we move stream C to become dependent on the connection stream, then we should distribute the weight of stream A to its children (only stream C), so the new weight of stream C will be 16. If we keep the weight of stream C unchanged, it will get more resource than stream B
Modification:
- distribute weight to its children when closing a stream
- add a unit test for the case above and fix other related unit tests
Result:
More spec-compliant and more appropriate stream reprioritization
Co-authored-by: Heng Zhang <zhangheng@imo.im>
Motivation:
The TLS handshake must be able to finish on its own, without being driven by outside read calls.
This is currently not the case when TCP FastOpen is enabled.
Reads must be permitted and marked as pending, even when a channel is not active.
This is important because, with TCP FastOpen, the handshake processing of a TLS connection will start
before the connection has been established -- before the process of connecting has even been started.
The SslHandler on the client side will add the Client Hello message to the ChannelOutboundBuffer, then
issue a `ctx.read` call for the anticipated Server Hello response, and then flush the Client Hello
message which, in the case of TCP FastOpen, will cause the TCP connection to be established.
In this transaction, it is important that the `ctx.read` call is not ignored since, if auto-read is
turned off, this could delay or even prevent the Server Hello message from being processed, causing
the server-side handshake to time out.
Modification:
Attach a listener to the SslHandler.handshakeFuture in the EchoClient, that will call ctx.read.
Result:
The SocketSslEchoTest now tests that the SslHandler can finish handshakes on its own, without being driven by 3rd party ctx.read calls.
The various channel implementations have been updated to comply with this behaviour.
Motivation:
JdkOpenSslEngineInteroptTest.testMutualAuthSameCerts() is flaky on the CI and so fails the PR build quite often.
Let's disable it for now until we were able to reproduce it locally and fix it.
Modifications:
Disable flaky test
Result:
More stable CI builds
__Motivation__
Upon receiving a DNS answer, we match whether the name in the question matches the name in the record. Some DNS servers we have encountered append a search domain to the record name which fails this match. eg: for question name `netty` and search domains `io` and `com`, we will do 2 queries: `netty.io.` and `netty.com.`, if the answer for `netty.io` contains `netty.com` then we ignore this record.
__Modification__
If the name in the record does not match the name in the question, append configured search domains to the question name to see if it matches the record name.
__Result__
Records names with appended search domains are still returned as valid answers.
Motivation:
We need to add `--add-exports java.base/sun.security.x509=ALL-UNNAMED` when running the tests for codec-http2 as some of the tests use SelfSignedCertificate.
Modifications:
- Add `--add-exports java.base/sun.security.x509=ALL-UNNAMED` when running the tests for codec-http2
- Ensure we export correct when running with JDK12, 13, 14 and 15 as well
Result:
No more tests failure due not be able to access classes
Motivation:
#11468 was merged but didn't fix tests completely. There is a fight between `LF` and `CRLF`. So to eliminate this, we should just get rid of them.
Modification:
Use a small sample dataset without `LF` and `CRLF`.
Result:
Simple and passing test.
Motivation:
We did migrate all these modules to junit5 before but missed a few usages of junit4
Modifications:
Replace all junit4 imports by junit5 apis
Result:
Part of https://github.com/netty/netty/issues/10757
Motivation:
JavaDoc of StandardCompressionOptions should point towards public methods. Also, Brotli tests were failing on Windows.
Modification:
Fixed JavaDoc and enabled Brotli tests on Windows.
Result:
Better JavaDoc and Brotli tests will run on Windows
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
In #11256, We introduced `Iterable` as a parameter but later in review, it was removed. But we forgot to change `compressionOptionsIterable` to just `compressionOptions`.
Modification:
Changed `compressionOptionsIterable` to `compressionOptions`.
Result:
Correct ObjectUtil message
Motivation:
There are use cases when Unix domain datagram sockets are needed for communication.
This PR adds such support for Epoll/KQueue.
Modification:
- Expose Channel, Config and Packet interfaces/classes for Unix domain datagram sockets.
All interfaces/classes are in `transport-native-unix-common` module in order to be available
for KQueue and Epoll implementations
- Add JNI code for Unix domain datagram sockets
- Refactor `DatagramUnicastTest` so that it can be used for testing also Unix domain datagram sockets
- Add Unix domain datagram sockets implementation for KQueue transport
- Add Unix domain datagram sockets implementation for Epoll transport
Result:
Fixes#6737
Motivation:
At the moment we only support signing / decrypting the private key in a synchronous fashion. This is quite limited as we may want to do a network call to do so on a remote system for example.
Modifications:
- Update to latest netty-tcnative which supports running tasks in an asynchronous fashion.
- Add OpenSslAsyncPrivateKeyMethod interface
- Adjust SslHandler to be able to handle asynchronous task execution
- Adjust unit tests to test that asynchronous task execution works in all cases
Result:
Be able to asynchronous do key signing operations
Motivation:
Currently, Netty only has BrotliDecoder which can decode Brotli encoded data. However, BrotliEncoder is missing which will encode normal data to Brotli encoded data.
Modification:
Added BrotliEncoder and CompressionOption
Result:
Fixes#6899.
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
ZSTD has a wide range of uses on the Internet, so should consider adding `application/zstd` HTTP media-type and `zstd` content-encoding, see https://tools.ietf.org/html/rfc8478
Modification:
Add `application/zstd` HTTP media-type and `zstd` content-encoding
Result:
netty provides `application/zstd` HTTP media-type and `zstd content-encoding` as http headers
Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
Motivation:
At the moment we always build all modules. This script can be used to only build affected modules for a given change
Modifications:
Add script that will only build modules that are affected by a change
Result:
More targeted build
Motivation:
At the moment all methods in `ChannelHandler` declare `throws Exception` as part of their method signature. While this is fine for methods that handle inbound events it is quite confusing for methods that handle outbound events. This comes due the fact that these methods also take a `ChannelPromise` which actually need to be fullfilled to signal back either success or failure. Define `throws...` for these methods is confusing at best. We should just always require the implementation to use the passed in promise to signal back success or failure. Doing so also clears up semantics in general. Due the fact that we can't "forbid" throwing `RuntimeException` we still need to handle this in some way tho. In this case we should just consider it a "bug" and so log it and close the `Channel` in question. The user should never have an exception "escape" their implementation and just use the promise. This also clears up the ownership of the passed in message etc.
As `flush(ChannelHandlerContext)` and `read(ChannelHandlerContext)` don't take a `ChannelPromise` as argument this also means that these methods can never produce an error. This makes kind of sense as these really are just "signals" for the underlying transports to do something. For `RuntimeException` the same rule is used as for other outbound event handling methods, which is logging and closing the `Channel`.
Motifications:
- Remove `throws Exception` from signature
- Adjust code to not throw and just notify the promise directly
- Adjust unit tests
Result:
Much cleaner API and semantics.
Motivation:
We should only run one SSL task per delegation to allow more SSLEngines to make progress in a timely manner
Modifications:
- Only run one task per delegation to the executor
- Only create new SSL task if really needed
- Only schedule if not on the EventExecutor thread
Result:
More fair usage of resources and less allocations