Motivation:
As stated by https://tools.ietf.org/html/rfc7540#section-8.1.2.6 we should report a stream error if the content-length does not match the sum of all data frames.
Modifications:
- Verify that the sum of data frames match if a content-length header was send.
- Handle multiple content-length headers and also handle negative values
- Add io.netty.http2.validateContentLength system property which allows to disable the more strict validation
- Add unit tests
Result:
Correctly handle the case when the content-length header was included but not match what is send and also when content-length header is invalid
Motivation:
HttpObjectDecoder may throw an IllegalArgumentException if it encounters
a character that Character.isWhitespace() returns true for, but is not
one of the two valid OWS (optional whitespace) values. Such values may
not have friendly or readable toString() values. We should include the
hex value so that the illegal character can always be determined.
Modifications:
Add hex value as well
Result:
Easier to debug
Co-authored-by: Bennett Lynch <Bennett-Lynch@users.noreply.github.com>
Motivation:
These days we always include the OS in the library name. This means we also can simplify things
Modifications:
Adjust build configuration to address for libray name change
Result:
Simplify build
Motivation:
The DnsResolver default start address listen to "0.0.0.0", which may be not what the user wants.
Modification:
Add localAddress as a param of DnsNameResolver and its builder
Result:
The DnsNameResolver's bind address can be configured.
Motivation:
At the moment we don't support session caching on the client side at all when using the native SSL implementation. We should at least allow to enable it.
Modification:
Allow to enable session cache for client side but disable ti by default due a JDK bug atm.
Result:
Be able to cache sessions on the client side when using native SSL implementation .
Motivation:
In WriteTimeoutHandler we did make the assumption that the executor which is used to schedule the timeout is the same that is backing the write promise. This may not be true which will cause concurrency issues
Modifications:
Ensure we are on the right thread when try to modify the doubly-linked-list and if not schedule it on the right thread.
Result:
Fixes https://github.com/netty/netty/issues/11053
Motivation:
At the moment its only possible to create a PendingWriteQueue via a ChannelHandlerContext.
Modifications:
Add another constructor
Result:
More flexible usage of PendingWriteQueue
Motivation:
We need to ensure that we call queue.remove() before we cal writeAndFlush() as this operation may cause an event that also touches the queue and remove from it. If we miss to do so we may see NoSuchElementExceptions.
Modifications:
- Call queue.remove() before calling writeAndFlush(...)
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/11046
Motivation:
Channels need to have their configurations applied before we can call out to user-code via handlerAdded and initChannel.
Modification:
This adds tests for this behaviour, which already works correctly.
Result:
Better test coverage.
Motivation:
For protocols like QUIC using UDP_SEGMENT (GSO) can help to reduce the
overhead quite a bit. We should support it.
Modifications:
- Add a SegmentedDatagramPacket which can be used to use UDP_SEGMENT
- Add unit test
Result:
Be able to make use of UDP_SEGMENT
Motivation:
- Underlying buffer usages might be erroneous when releasing them internaly
in HttpPostMultipartRequestDecoder.
2 bugs occurs:
1) Final File upload seems not to be of the right size.
2) Memory, even in Disk mode, is increasing continuously, while it shouldn't.
- Method `getByte(position)` is too often called within the current implementation
of the HttpPostMultipartRequestDecoder.
This implies too much activities which is visible when PARANOID mode is active.
This is also true in standard mode.
Apply the same fix on buffer from HttpPostMultipartRequestDecoder to HttpPostStandardRequestDecoder
made previously.
Finally in order to ensure we do not rewrite already decoded HttpData when decoding
next ones within multipart, we must ensure the buffers are copied and not a retained slice.
Modification:
- Add some tests to check consistency for HttpPostMultipartRequestDecoder.
Add a package protected method for testing purpose only.
- Use the `bytesBefore(...)` method instead of `getByte(pos)` in order to limit the external
access to the underlying buffer by retrieving iteratively the beginning of a correct start
position.
It is used to find both LF/CRLF and delimiter.
2 methods in HttpPostBodyUtil were created for that.
The undecodedChunk is copied when adding a chunk to a DataMultipart is loaded.
The same buffer is also rewritten in order to release the copied memory part.
Result:
Just for note, for both Memory or Disk or Mixed mode factories, the release has to be done as:
for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
httpData.release();
factory.removeHttpDataFromClean(request, httpData);
}
factory.cleanAllHttpData();
decoder.destroy();
The memory used is minimal in Disk or Mixed mode. In Memory mode, a big file is still
in memory but not more in the undecodedChunk but its own buffer (copied).
In terms of benchmarking, the results are:
Original code Benchmark Mode Cnt Score Error Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel thrpt 6 0,152 ± 0,100 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel thrpt 6 0,543 ± 0,218 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel thrpt 6 0,001 ± 0,001 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel thrpt 6 0,615 ± 0,070 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel thrpt 6 0,114 ± 0,063 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel thrpt 6 0,664 ± 0,034 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel thrpt 6 0,001 ± 0,001 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel thrpt 6 0,620 ± 0,140 ops/ms
New code Benchmark Mode Cnt Score Error Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel thrpt 6 4,037 ± 0,358 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel thrpt 6 4,226 ± 0,471 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel thrpt 6 0,875 ± 0,029 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel thrpt 6 4,346 ± 0,275 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel thrpt 6 2,044 ± 0,020 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel thrpt 6 2,278 ± 0,159 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel thrpt 6 0,174 ± 0,004 ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel thrpt 6 2,370 ± 0,065 ops/ms
In short, using big file transfers, this is about 7 times faster with new code, while
using high number of HttpData, this is about 4 times faster with new code when using Simple Level.
When using Paranoid Level, using big file transfers, this is about 800 times faster with new code, while
using high number of HttpData, this is about 170 times faster with new code.
Motivation:
It is possible for two separate threads to race on recycling an object.
If this happens, the object might be added to a WeakOrderQueue when it shouldn't be.
The end result of this is that an object could be acquired multiple times, without a recycle in between.
Effectively, it ends up in circulation twice.
Modification:
We fix this by making the update to the lastRecycledId field of the handle, an atomic state transition.
Only the thread that "wins" the race and succeeds in their state transition will be allowed to recycle the object.
The others will bail out on their recycling.
We use weakCompareAndSet because we only need the atomicity guarantee, and the program order within each thread is sufficient.
Also, spurious failures just means we won't recycle that particular object, which is fine.
Result:
Objects no longer risk circulating twice due to a recycle race.
This fixes#10986
Motivation:
It is not uncommon to run Netty on OS X without the specific
`MacOSDnsServerAddressStreamProvider`. The current log message is too
verbose because it prints a full stack trace on the console while a
simple logging message would have been enough.
Modifications:
- Print a `WARN` message when `MacOSDnsServerAddressStreamProvider`
class is not found;
- Print a `ERROR` message with a stack trace when the class was found
but could not be loaded due to some other reasons;
Result:
Less noise in logs.
Motivation:
We should use a higher timeout as sometimes the verification process in oss.sonatype.org is very slow.
Modifications:
Bump up timeout to 10 minutes
Result:
Less likely to see timeouts
Motivation:
When finish the release process we need to give the id of the staged release. Let's add a script for that
Modifications:
Add script which allows to show all staged releases
Result:
No need to login into sonatype anymore
Motivation:
If two different headers end up in the same hash bucket, and you are iterating the header that is not the first in the bucket, and you use the iterator to remove the first element returned from the iterator, then you would get a NullPointerException.
Modification:
Change the DefaultHeaders iterator remove method, to re-iterate the hash bucket and unlink the entry once found, if we don't have any existing iteration starting point.
Also made DefaultHeaders.remove0 package private to avoid a synthetic method indirection.
Result:
Removing from iterators from DefaultHeaders is now robust towards hash collisions.
Motivation:
When TLSv1.3 is used (or TLS_FALSE_START) together with mTLS the handshake is considered successful before the server actually did verify the key material that was provided by the client. If the verification fails we currently will just close the stream without any extra information which makes it very hard to debug on the client side.
Modifications:
- Propagate SSLExceptions to the active streams
- Add unit test
Result:
Better visibility into why a stream was closed
Motivation:
We can just close and release the staging repository at the end and so remove the extra manual steps
Modification:
Execute extra goals for close and release
Result:
Less manual actions needed for release
Motivation:
We should fail the script if the tag already exists locally to ensure we really deploy the right code
Modifications:
Check if the tag already exists and if so print an error and exit
Result:
No risks to deploy wrong code as part of the release
Motivation:
#10995
when `io.netty.channel.unix.Socket` is ipv6 and join a multicast group with ipv4 address will cause `io.netty.channel.ChannelException: setsockopt() failed: Invalid argument` (at least in `Linux centos.dev 4.18.0-240.10.1.el8_3.x86_64 #1 SMP Mon Jan 18 17:05:51 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux`)
Modification:
check if target group address is ipv6 before call `io.netty.channel.epoll.LinuxSocket#joinGroup(java.net.InetAddress, java.net.NetworkInterface, java.net.InetAddress)`
I'm not sure if this modification is currect, but i checked source code of java NIO
```
Java_sun_nio_ch_Net_canJoin6WithIPv4Group0(JNIEnv* env, jclass cl)
{
#if defined(__APPLE__)
/* IPV6_ADD_MEMBERSHIP can be used to join IPv4 multicast groups */
return JNI_TRUE;
#else
/* IPV6_ADD_MEMBERSHIP cannot be used to join IPv4 multicast groups */
return JNI_FALSE;
#endif
}
```
seems ipv6 address can't join ipv4 group except osx
Result:
test on `Linux 3.10.0-514.el7.x86_64 #1 SMP Tue Nov 22 16:42:41 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux` exception ` setsockopt() failed: Invalid argument` has fixed
Fixes#10995
Motivation:
The `!fastOpen` part of `active || !fastOpen` is always false.
Modification:
- Remove `!fastOpen` and keep only `active` as a `flushAtEnd` flag for
`startHandshakeProcessing`;
- Update comment;
Result:
Simplified `flushAtEnd` flag computation in `SslHandler#handlerAdded`.
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
