Support TCP Fast Open for clients and make SslHandler take advantage
Motivation:
- TCP Fast Open allow us to send a small amount of data along side the initial SYN packet when establishing a TCP connection.
- The TLS Client Hello packet is small enough to fit in there, and is also idempotent (another requirement for using TCP Fast Open), so if we can save a round-trip when establishing TLS connections when using TFO.
Modification:
- Add support for client-side TCP Fast Open for Epoll, and also lowers the Linux kernel version requirements to 3.6.
- When adding the SslHandler to a pipeline, if TCP Fast Open is enabled for the channel (and the channel is not already active) then start the handshake early by writing it to the outbound buffer.
- An important detail to note here, is that the outbound buffer is not flushed at this point, like it would for normal handshakes. The flushing happens later as part of establishing the TCP connection.
Result:
- It is now possible for clients (on epoll) to open connections with TCP Fast Open.
- The SslHandler automatically detects when this is the case, and now send its Client Hello message as part of the initial data in the TCP Fast Open flow when available, saving a round-trip when establishing TLS connections.
Co-authored-by: Colin Godsey <crgodsey@gmail.com>
Motivation:
Doing releases manually is error-prone, it would be better if we could do it via a workflow
Modification:
- Add workflow to cut releases
- Add related scripts
Result:
Be able to easily cut a release via a workflow
Motivation:
The current netty's graalvm dependency version is too low, so you need to upgrade the plugin
Modification:
Upgrade Graalvm version to the latest version, please review this pr, thank you
Result:
Use up-to-date version.
Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
Motivation:
The testGlobalWriteThrottle is flaky and failed our build multiple times now. Lets disable it for now until we had time to investigate
Modifications:
Disable flaky test
Result:
Less failures during build
Motivation:
When etcResolver/hosts files are parsed, FileInputStream.read(...) is internally called by
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverSearchDomains
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverOptions
- HostsFileParser#parse
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entries to BlockHound configuration
- Fix typos in UnixResolverDnsServerAddressStreamProvider
- Add tests
Result:
Fixes#11004
* Revert "Add a profile for debugging tests that run from Maven (#11011)"
This reverts commit 83895f0f
The same functionality is already natively available in surefire, by adding the `-Dmaven.surefire.debug` flag to Maven.
* Update surefire/failsafe version
These new versions copes better when our tests prints to STDOUT, and disturbs the progress processing that these plugins do.
Motivation:
In some cases, Intellij struggles to recreate the build and test
environment/configuration that Maven produces, and this can lead to tests
behaving differently when run from Intellij compared to when they run from
Maven.
This in turn can make debugging those tests harder.
Modification:
Add a profile to the Maven build, that will add the necessary command line
arguments for attaching the Intellij debugger to tests that are executed from
Maven.
Result:
It is now possible to debug the tests that Maven is running, from Intellij,
by enabling the -PijDebug Maven profile.
Motivation:
At the moment we always set SSL_OP_NO_TICKET when building our context. The problem with this is that this also disables resumption for TLSv1.3 in BoringSSL as it only supports stateless resumption for TLSv1.3 which uses tickets.
We should better clear this option when TLSv1.3 is enabled to be able to resume sessions. This is also inline with the OpenJDK which enables this for TLSv1.3 by default as well.
Modifications:
Check for enabled protocols and if TLSv1.3 is set clear SSL_OP_NO_TICKET.
Result:
Be able to resume sessions for TLSv1.3 when using BoringSSL.
Motivation:
Jabba does not contain version 1.8 anymore
Modifications:
Use some java version that exists
Result:
Builder the docker image from scratch work again
Motivation:
File.createTempFile(String, String)` will create a temporary file in the system temporary directory if the 'java.io.tmpdir'. The permissions on that file utilize the umask. In a majority of cases, this means that the file that java creates has the permissions: `-rw-r--r--`, thus, any other local user on that system can read the contents of that file.
This can be a security concern if any sensitive data is stored in this file.
This was reported by Jonathan Leitschuh <jonathan.leitschuh@gmail.com> as a security problem.
Modifications:
Use Files.createTempFile(...) which will use safe-defaults when running on java 7 and later. If running on java 6 there isnt much we can do, which is fair enough as java 6 shouldnt be considered "safe" anyway.
Result:
Create temporary files with sane permissions by default.
Motivation:
To make it easier to understand why a build failed let us publish the rest results
Modifications:
Use a new workflow to be able to publish the test reports
Result:
Easier to understand why a PR did fail
Motivation:
DecodeHexBenchmark needs to be less branch-predictor friendly
to mimic the "real" behaviour while decoding
Modifications:
DecodeHexBenchmark uses a larger sets of inputs, picking them at
random on each iteration and the benchmarked method is made !inlineable
Result:
DecodeHexBenchmark is more trusty while showing the performance
difference between different decoding methods
Motivation:
when customer need large of 'byteBuf.capacity' in [7168, 8192], the size of 'chunk.subpages' may be inflated when large of byteBuf be released, not consistent with other 'byteBuf.capacity'
Modification:
when maxNumElems == 1 need consider remove from pool
Result:
Fixes#10896.
Co-authored-by: zxingy <zxingy@servyou.com.cn>
Bumps ant from 1.9.15 to 1.10.9.
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Motivation:
The changes introduced in 1c230405fd did cause various issues while the fix itself is not considered critical. For now it is considered the best to just rollback and investigate more.
Modifications:
- Revert changes done in 1c230405fd (and later) for
the post decoders.
- Ensure we give memory back to the system as soon as possible in a safe manner
Result:
Fixes https://github.com/netty/netty/issues/10973
Motivation:
Some of the features we want to support can only be supported by some of the SslContext implementations. We should allow to configure these in a consistent way the same way as we do it with Channel / ChannelOption
Modifications:
- Add SslContextOption and add builder methods that take these
- Add OpenSslContextOption and define two options there which are specific to openssl
Result:
More flexible configuration and implementation of SslContext
Motivation:
It was not 100% clear who is responsible calling close() on the InputStream.
Modifications:
Clarify javadocs.
Result:
Related to https://github.com/netty/netty/issues/10974
Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Motivation:
TLS_FALSE_START slightly changes the "flow" during handshake which may cause suprises for the end-user. We should better disable it by default again and later add a way to enable it for the user.
Modification:
This reverts commit 514d349e1f.
Result:
Restore "old flow" during TLS handshakes.
Motivation:
We should add some more NULL checks to ensure we not SEGV in some cases
Modifications:
Add more NULL checks
Result:
More robust implementation
Motivation:
We didnt correctly filter out TLSv1.3 ciphers when TLSv1.3 is not enabled.
Modifications:
- Filter out ciphers that are not supported due the selected TLS version
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/10911
Co-authored-by: Bryce Anderson <banderson@twitter.com>
Motivation:
The testWriteAfterShutdownOutputNoWritabilityChange() failed a few times on the CI randomly. Let's skip it for now while we investigate and see if there is anything we can do to make the test less flaky on the CI.
Modifications:
Add @Ignore on the testWriteAfterShutdownOutputNoWritabilityChange method
Result:
Less flaky CI
Motivation:
It turns out we can't use the action to check for build failures as it can't be used when a PR is done from a fork. Let's just use our simple script.
Modifications:
- Replace action with custom script
Result:
Builds for PRs that are done via forks work again.
Motivation:
We did have the architecture hardcoded in the dependency which is not correct as this will let the build fail on Applie Silicion (m1). Also we did miss some dependencies on other BSDs
Modifications:
- Fix classifier
- Add missing dependencies
Result:
Be able to build on Apple Silicon (m1)
Motivation:
We need to ensure we copy the attributes and options when bootstrap the channel as otherwise we may change the underlying Entry.
This is similar to what was reported in https://github.com/netty/netty-incubator-codec-quic/issues/152.
Modifications:
- Do a copy and re-use methods
- Add unit tests
Result:
Don't affect attributes / options of channels that are already bootstrapped
Motivation:
To make it easier to understand why the build fails lets use an action that will report which unit test failed
Modifications:
- Replace custom script with action-surefire-report
Result:
Easier to understand test failures
Motivation:
If the given port is already bound, the PcapWriteHandlerTest will sometimes fail.
Modification:
Use a dynamic port using `0`, which is more reliable
Result:
Less Flaky
Motivation:
We currently append extensions to the user defined "sec-websocket-extensions" headers. This can cause duplicated entries.
Modifications:
* Replace existing `WebSocketExtensionUtil#appendExtension` private helper with a new `computeMergeExtensionsHeaderValue`. User defined parameters have higher precedence.
* Add tests (existing method wasn't tested)
* Reuse code for both client and server side (code was duplicated).
Result:
No more duplicated entries when user defined extensions overlap with the ones Netty generated.
Motivation:
Exception occurs in the MQTT message builder class (`io.netty.handler.codec.mqtt.MqttMessageBuilders`) when trying to create a message with packetId > 32767
Modification:
-add method that takes int
- deprecate old methods that take short.
Result:
Fixes#10929 .
Motivation:
We currently append extensions to the user defined "sec-websocket-extensions" headers. This can cause duplicated entries.
Modifications:
* Replace existing `WebSocketExtensionUtil#appendExtension` private helper with a new `computeMergeExtensionsHeaderValue`. User defined parameters have higher precedence.
* Add tests (existing method wasn't tested)
* Reuse code for both client and server side (code was duplicated).
Result:
No more duplicated entries when user defined extensions overlap with the ones Netty generated.
Motivation:
Creating certificates from a byte[] while lazy parse it is general useful and is also needed by https://github.com/netty/netty-incubator-codec-quic/pull/141
Modifications:
Move classes, rename these and make them public
Result:
Be able to reuse code
Motivation:
We should override the get*ApplicationProtocol() methods in ReferenceCountedOpenSslEngine to make it easier for users to obtain the selected application protocol
Modifications:
Add missing overrides
Result:
Easier for the user to get the selected application protocol (if any)
Motivation:
We should expose some methods as protected to make it easier to write custom SslContext implementations.
This will be reused by the code for https://github.com/netty/netty-incubator-codec-quic/issues/97
Modifications:
- Add protected to some static methods which are useful for sub-classes
- Remove some unused methods
- Move *Wrapper classes to util package and make these public
Result:
Easier to write custom SslContext implementations
Motivation:
https://github.com/netty/netty/pull/10765 added support for push promise and priority frames when using the Http2FrameCodec. Unfortunally it didnt correctly guard against the possibility to receive a priority frame for an non-existing stream, which resulted in a NPE
Modifications:
- Ignore priority frame for non existing stream
- Correctly implement equals / hashcode for DefaultHttp2PriorityFrame
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/10941
Motivation:
We had multiple bugs in JdkZlibDecoder which could lead to decoding errors when the data was received in a fragmentated manner.
Modifications:
- Correctly handle skipping of comments
- Correctly handle footer / header decoding
- Add unit test that verifies the correct handling of fragmentation
Result:
Fixes https://github.com/netty/netty/issues/10875
Motivation:
We produced a lot of noise during loading native libraries as we always included the stacktrace if we could not load by one mechanism. We should better just not include the stacktrace in the debugging logging if one mechanism fails. We will log all the stacks anyway when all of the mechanisms fail.
Modifications:
Make logging less aggressive
Result:
Less confusing behaviour for the end-user
Motivation:
We should use GracefulShutdown when we try to create a stream and fail it because the stream space is exhausted as we may still want to process the active streams.
Modifications:
- Use graceful shutdown
- Add unit test
Result:
More graceful handling of stream creation failure due stream space exhaustation
Motivation:
Internally UnixResolverDnsServerAddressStreamProvider#parse calls FileInputStream.read(...)
when parsing the etcResolverFiles.
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entry to BlockHound configuration
- Add test
Result:
Fixes#10925
Motivation:
We need to ensure we also build the modules we depend on as otherwise we may not be able to resolve the dependencies correctly
Modifications:
- Add -am when calling maven
Result:
Deployment works all the time
Motiviation:
We need to ensure we only register the methods for unix-native-common once as otherwise it may have strange side-effects.
Modifications:
- Add extra method that should be called to signal that we need to register the methods. The registration will only happen once.
- Adjust code to make use of it.
Result:
No more problems due incorrect registration of these methods.
Motivation:
As shown in the past we need to verify we actually can load the native as otherwise we may introduce regressions.
Modifications:
- Add new maven module which tests loading of native modules
- Add job that will also test loading on aarch64
Result:
Less likely to introduce regressions related to loading native code in the future
