Motivation:
As we use different execution path in our SSLEngine implementation depending on if heap, direct or mixed buffers are used we should run the tests with all of them.
Modification:
Ensure we run all tests with different buffer types.
Result:
Better test-coverage
Motivation:
For the completion of a handshake we already fire a SslHandshakeCompletionEvent which the user can intercept. We should do the same for the receiving of close_notify.
Modifications:
Add SslCloseCompletionEvent and test-case.
Result:
More consistent API.
Motivation:
https://github.com/netty/netty/pull/6042 only addressed PlatformDependent#getSystemClassLoader but getClassLoader is also called in an optional manner in some common code paths but fails to catch a general enough exception to continue working.
Modifications:
- Calls to getClassLoader which can continue if results fail should catch Throwable
Result:
More resilient code in the presense of restrictive class loaders.
Fixes https://github.com/netty/netty/issues/6246.
Motivation:
Pattern matching not necessary for number parsing.
Modification:
Removed pattern matching for number parsing and removed unnecessary toLowerCase() operation.
Result:
No static variable with pattern, removed unnecessary matching operation and toLowerCase() operation.
Motivation:
We not warned about not-supported ChannelOptions when set the options for the ServerChannel.
Modifications:
- Share code for setting ChannelOptions during bootstrap
Result:
Warning is logged when a ChannelOption is used that is not supported during bootstrap a Channel. See also [#6192]
Motivation:
PlatformDependent* contains some methods that are not used and some other things that can be cleaned-up.
Modifications:
- Remove unused methods
- cleanup
Result:
Code cleanup.
Motivation:
We miss checking if DnsCache is null in DnsNameResolver constructor which will later then lead to a NPE. Better fail fast here.
Modifications:
Check for null and if so throw a NPE.
Result:
Fail fast.
Motivation:
The SslHandler.sslCloseFuture() may not be notified when the Channel is closed before a closify_notify is received.
Modifications:
Ensure we try to fail the sslCloseFuture() when the Channel is closed.
Result:
Correctly notify the ssl close future.
Motivation:
The JDK implementation of SSLEngine allows to have unwrap(...) / wrap(...) called even after closeInbound() and closeOutbound() were called. We need to support the same in ReferenceCountedSslEngine.
Modification:
- Allow calling ReferenceCountedSslEngine.unwrap(...) / wrap(...) after the engine was closed
- Modify unit test to ensure correct behaviour.
Result:
Implementation works as expected.
Motivation:
fc3c9c9523 introduced a bug which will have ReferenceCountedSslEngine.unwrap(...) produce an IOOBE when be called with an BŷteBuffer as src that contains multiple SSLRecords and has a position != 0.
Modification:
- Correctly set the limit on the ByteBuffer and so fix the IOOBE.
- Add test-case to verify the fix
Result:
Correctly handle heap buffers as well.
Motivation:
- Decoder#decodeULE128 has a bounds bug and cannot decode Integer.MAX_VALUE
- Decoder#decodeULE128 doesn't support values greater than can be represented with Java's int data type. This is a problem because there are cases that require at least unsigned 32 bits (max header table size).
- Decoder#decodeULE128 treats overflowing the data type and invalid input the same. This can be misleading when inspecting the error that is thrown.
- Encoder#encodeInteger doesn't support values greater than can be represented with Java's int data type. This is a problem because there are cases that require at least unsigned 32 bits (max header table size).
Modifications:
- Correct the above issues and add unit tests.
Result:
Fixes https://github.com/netty/netty/issues/6210.
Motivation:
LZ4FrameEncoder maintains an internal buffer of incoming data compress, and only writes out compressed data when a size threshold is reached. LZ4FrameEncoder does not override the flush() method, and thus the only way to flush data down the pipeline is via more data or close the channel.
Modifications:
Override the flush() function to flush on demand. Also overrode the allocateBuffer() function so we can more accurately size the output buffer (instead of needing to potatntially realloc via buffer.ensureWritable()).
Result:
Implementation works as described.
Motivation:
Build failures have been observed with 2 second timeouts on the CI servers. We should make the timeouts longer to reduce false positive test failures due to tests timing out prematurely.
Modifications:
- Increase timeouts from 2 and 3 seconds to 5 seconds.
Result:
Less false positive test failures.
Motivation:
Openssl provider should behave same as JDK provider when mutual authentication is required and a specific set of trusted Certificate Authorities are specified. The SSL handshake should return back to the connected peer the same list of configured Certificate Authorities.
Modifications:
Correctly set the CA list.
Result:
Correct and same behaviour as the JDK implementation.
Motivation:
We need to ensure we not swallow the close_notify that should be send back to the remote peer. See [#6167]
Modifications:
- Only call shutdown() in closeInbound() if there is nothing pending that should be send back to the remote peer.
- Return the correct HandshakeStatus when the close_notify was received.
- Only shutdown() when close_notify was received after closeOutbound() was called.
Result:
close_notify is correctly send back to the remote peer and handled when received.
Motivation:
The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host.
Modifications:
- Correctly format the IPV6 address.
- Add unit tests
Result:
HttpProxyHandler works with IPV6 as well. Fixes [#6152].
Motivation:
The HTTP/2 helloworld client example has 2 bugs:
1. HttpResponseHandler has a map which is accessed from multiple threads, but the map is not thread safe.
2. Requests are flushed and maybe completely written and the responses may be received/processed by Netty before an element is inserted into the HttpResponseHandler map. This may result in an 'unexpected message' error even though the message has actually been sent.
Modifications:
- HttpResponseHandler should use a thread safe map
- Http2Client shouldn't flush until entries are added to the HttpResponseHandler map
Result:
Fixes https://github.com/netty/netty/issues/6165.
Motivation:
When DefaultHttp2Connection removes a stream it iterates over all children and adds them as children to the parent of the stream being removed. This process may remove elements from the child map while iterating without using the iterator's remove() method. This is generally unsafe and may result in an undefined iteration.
Modifications:
- We should use the Iterator's remove() method while iterating over the child map
Result:
Fixes https://github.com/netty/netty/issues/6163
Motivation:
[#6153] reports an endless loop that existed in the Recycler, while this was fixed adding a few asserts to ensure this remains fixed is a good thing. Beside this we also should ensure this can not escape the constructor to avoid unsafe publication.
Modifications:
- Add asserts
- Fix unsafe publication
Result:
More correct code.
Motivation:
Thought there may be a bug so added a testcase to verify everything works as expected.
Modifications:
Added testcase
Result:
More test-coverage.
Motivation:
EpollRecvByteAllocatorHandle will read unconditionally if EPOLLRDHUP has been received. However we can just treat this the same was we do as data maybe pending in ET mode, and let LT mode notify us if we haven't read all data.
Modifications:
- EpollRecvByteAllocatorHandle should not always force a read just because EPOLLRDHUP has been received, but just treated as an indicator that there maybe more data to read in ET mode
Result:
Fixes https://github.com/netty/netty/issues/6173.
Motivation:
ByteBufUtil.compare uses long arithmetic but doesn't check for underflow on when converting from long to int to satisfy the Comparable interface. This will result in incorrect comparisons and violate the Comparable interface contract.
Modifications:
- ByteBufUtil.compare should protect against int underflow
Result:
Fixes https://github.com/netty/netty/issues/6169
Motivation:
The comment on AbstractChannelHandlerContext.invokeHandler() is incorrect and missleading. See [#6177]
Modifications:
Change true to false to correct the comment.
Result:
Fix missleading and incorrect comment.
Motivation:
`scavengeSome()` has a corner case: when setting `cursor` to `head`, `this.prev` may point to the tail of the `WeakOrderQueue` linked list. Then it's possible that the following while loop will link the tail to the head, and cause endless loop.
I made a reproducer in 36522e7b72 . The unit test will just run forever. Unfortunately, I cannot change it to a unit test because it needs to add some codes to `scavengeSome` to control the execution flow.
Modification:
Set `prev` to null when setting `cursor` to `head` in `scavengeSome`
Result:
Fixes#6153.
Motivation:
InternalThreadLocalMap.arrayList returns a new ArrayList every time it's called that defeats the purpose of having a reusable ArrayList.
Modification:
Modified InternalThreadLocalMap.arrayList to create an ArrayList only if arrayList field is NULL.
Result:
InternalThreadLocalMap.arrayList now creates a reusable ArrayList only if arrayList field is NULL.
Motivation
The IdleStateHandler tracks write() idleness on message granularity but does not take into consideration that the client may be just slow and has managed to consume a subset of the message's bytes in the configured period of time.
Modifications
Adding an optional configuration parameter to IdleStateHandler which tells it to observe ChannelOutboundBuffer's state.
Result
Fixes https://github.com/netty/netty/issues/6150
Motivation:
In Netty, currently, the HttpPostRequestEncoder only supports POST, PUT, PATCH and OPTIONS, while the RFC 7231 allows with a warning that GET, HEAD, DELETE and CONNECT use a body too (but not TRACE where it is explicitely not allowed).
The RFC in chapter 4.3 says:
"A payload within a XXX request message has no defined semantics;
sending a payload body on a XXX request might cause some existing
implementations to reject the request."
where XXX can be replaced by one of GET, HEAD, DELETE or CONNECT.
Current usages, on particular in REST mode, tend to use those extra HttpMethods for such queries.
So this PR proposes to remove the current restrictions, leaving only TRACE as explicitely not supported.
Modification:
In the constructor, where the test is done, replacing all by checking only against TRACE, and adding one test to check that all methods are supported or not.
Result:
Fixes#6138.
Motivation:
cb139043f3 introduced special handling of response to HEAD requests. Due a bug we failed to handle FullHttpResponse correctly.
Modifications:
Correctly handle FullHttpResponse for HEAD requests.
Result:
Works as expected.
Motivation:
`SimpleChannelPool` subclasses are likely to override the `connectChannel` method, and are likely to clobber the cloned `Bootstrap` handler in the process. To allow subclasses to properly notify the pool listener of new connections, we should expose (at least) the `handler` property of the pool to subclasses.
Modifications:
Expose `SimpleChannelPool` properties to subclasses via `protected` getters.
Result:
Subclasses can now use the bootstrap, handler, health checker, and health-check-on-release preoperties from their superclass.
Motivation:
We used a MPSC queue in ThreadDeathWatcher and checked if it empty via isEmpty() from multiple threads if very unlucky. Depending on the implementation this is not safe and may even produce things like live-locks.
Modifications:
Change to use a MPMC queue.
Result:
No more risk to run into issues when multiple threads call watch(...) / unwatch(...) concurrently.
Motivation:
DefaultChannelId provides a regular expression which validates if a user provided MAC address is valid. This regular expression may allow invalid MAC addresses and also not allow valid MAC addresses.
Modifications:
- Introduce a MacAddressUtil#parseMac method which can parse and validate the MAC address at the same time. The regular expression check before hand is additional overhead if we have to parse the MAC address.
Result:
Fixes https://github.com/netty/netty/issues/6132.
Motivation:
We should have a unit test which explicitly tests a HTTP message being split between multiple ByteBuf objects.
Modifications:
- Add a unit test to HttpRequestDecoderTest which splits a request between 2 ByteBuf objects
Result:
More unit test coverage for HttpObjectDecoder.
Motivation:
`PromiseCombiner` is really handy, but it's not obvious how to use it from its existing documentation/method signatures.
Modification:
- Added javadoc comments to explain the theory of operation of `PromiseCombiner`.
- Generalized `PromiseCombiner` to work with `Futures` so it's clearer that the things for which it's listening won't be modified.
Result:
`PromiseCombiner` is easier to understand.
Motivation:
Enables optional .startsWith() matching of req.uri() with websocketPath.
Modifications:
New checkStartsWith boolean option with default false value added to both WebSocketServerProtocolHandler and WebSocketServerProtocolHandshakeHandler. req.uri() matching is based on this option.
Result:
By default old behavior matching via .equal() is preserved. To use checkStartsWith use constructor shortcut: new WebSocketServerProtocolHandler(websocketPath, true) or fill this flag on full form of constructor among other options.
Motivation:
On some platforms the PID my be bigger then 4194304 so we should not limit it to 4194304.
Modifications:
Only check that the PID is a valid Integer
Result:
No more warnings on systems where the PID is bigger then 4194304.
Motivation:
When profiling it is sometimes needed to still have the native library file avaible. We should allow to disable the explicit deletion and just delete it when the JVM stops.
This is related to #6110
Modifications:
Add io.netty.native.deleteLibAfterLoading system property which allows to disable the explicit delete after laoding
Result:
Possible to profile native libraries better.
Motivation:
Our ReferenceCountedOpenSslEngine does not support compression so we should explicit disable it.
This is related to #3722.
Modifications:
Set SSL_OP_NO_COMPRESSION option.
Result:
Not use compression.
Motivation:
Currently clients attempting to send headers that are too large recieve
a RST frame. This makes it harder than needed for implementations on top
of netty to handle this in a graceful way.
Modifications:
When the Decoder throws a StreamError of type FRAME_SIZE_ERROR, the
Http2ConnectionHandler will now attempt to send an Http2Header with
status 431 and endOfStream=true
Result:
Implementations now do not have to subclass parts of netty to handle
431s
Motivation:
In later Java8 versions our Atomic*FieldUpdater are slower then the JDK implementations so we should not use ours anymore. Even worse the JDK implementations provide for example an optimized version of addAndGet(...) using intrinsics which makes it a lot faster for this use-case.
Modifications:
- Remove methods that return our own Atomic*FieldUpdaters.
- Use the JDK implementations everywhere.
Result:
Faster code.
