Motivation:
Invoking the javax.net.ssl.SSLEngine.closeInbound() method will send a
fatal alert and invalidate the SSL session if a close_notify alert has
not been received.
From the javadoc:
If the application initiated the closing process by calling
closeOutbound(), under some circumstances it is not required that the
initiator wait for the peer's corresponding close message. (See section
7.2.1 of the TLS specification (RFC 2246) for more information on
waiting for closure alerts.) In such cases, this method need not be
called.
Always invoking the closeInbound() method without regard to whether or
not the closeOutbound() method has been invoked could lead to
invalidating perfectly valid SSL sessions.
Modifications:
Added an instance variable to track whether the
SSLEngine.closeOutbound() method has been invoked. When the instance
variable is true, the SSLEngine.closeInbound() method doesn't need to be
invoked.
Result:
SSL sessions will not be invalidated if the outbound side has been
closed but a close_notify alert hasn't been received.
Motivation:
When SpdyHttpEncoder attempts to create an SpdyHeadersFrame from a HttpResponse an IllegalArgumentException is thrown if the original HttpResponse contains a header that includes uppercase characters. The IllegalArgumentException is thrown due to the additional validation check introduced by #4047.
Previous versions of the SPDY codec would handle this by converting the HTTP header name to lowercase before adding the header to the SpdyHeadersFrame.
Modifications:
Convert the header name to lowercase before adding it to SpdyHeaders
Result:
SpdyHttpEncoder can now convert a valid HttpResponse into a valid SpdyFrame
Motivation:
RecvByteBufAllocator.DelegatingHandle does not provide an accessor to get the delegate handle. This may be useful for classes that extend DelegatingHandle.
Modifications:
- add delegate() method to DelegatingHandle
Result:
Classes which inherit from DelegatingHandle can now access the delegate Handle.
Motivation:
The javadoc comments on Http2Headers.iterator() are incorrect.
Modifications:
- Correct and clarify the javadoc for Http2Headers.iterator()
Result:
Javadoc for Http2Headers.iterator() is more correct.
Motivation:
There are protocols (BGP, SXP), which are typically deployed with TCP
MD5 authentication to protect sessions from being hijacked/torn down by
third parties. This facility is not available on most operating systems,
but is typically present on Linux.
Modifications:
- add a new EpollChannelOption, which is write-only
- teach Epoll(Server)SocketChannel to track which addresses have keys
associated
- teach Native how to set the MD5 signature keys for a socket
Result:
Users of the native-epoll transport can set MD5 signature keys and thus
leverage RFC-2385 protection on TCP connections.
Motivation:
If LocalChannel doWrite executes while the peer's state changes from CONNECTED to CLOSED it is possible that some promise's won't be completed and buffers will be leaked.
Modifications:
- Check the peer's state in doWrite to avoid a race condition
Result:
All write operations should release, and the associated promise should be completed.
Motivation:
InboundHttp2ToHttpAdapterTest.bootstrapEnv does not wait for the serverConnectedChannel to be initialized before returning. Some methods rely only this behavior and throw a NPE because it may not be set.
Modifications:
- Add a CountDownLatch to ensure the serverConnectedChannel is initialized
Result:
No more NPE.
Motivation:
The SimplePromiseAggregator.setFailure allows a failure to occur before newPromise is called, but tryFailure doesn't. These methods should be consistent.
Modifications:
- tryFailure should use the same logic as setFailure
Result:
Consistent failure routines.
Motivation:
The configurable property value recently added was not logged like others properties.
Modifications:
Added debug log with effective value applied.
Result:
Consistent with other properties
Motivation:
See #4174.
Modifications:
Modify transport-native-epoll to allow setting TCP_USER_TIMEOUT.
Result:
Hanging connections that are written into will get timeouted.
Conflicts:
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java
Motivation:
Leak detector, when it detects a leak, will print the last 5 stack
traces that touched the ByteBuf. In some cases that might not be enough
to identify the root cause of the leak.
Also, sometimes users might not be interested in tracing all the
operations on the buffer, but just the ones that are affecting the
reference count.
Modifications:
Added command line properties to override default values:
* Allow to configure max number of stack traces to collect
* Allow to only record retain/release operation on buffers
Result:
Users can increase the number of stack traces to debug buffer leaks
with lot of retain/release operations.
Motivation:
DNS lookups in DnsNameResolverTest can take longer than expected due to
retries. The hard limit of 5 seconds is being applied to
testNegativeTtl(), making the first uncached lookup cause a timeout.
Modifications:
Do not use JUnit's Timeout annotation but implement simple timeout
mechanism that apples only to cached lookups.
Result:
testNegativeTtl() should not fail when an initial negative lookup
requires a retry.
Motivation:
As all methods in the ChannelHandler are executed by the same thread there is no need to use synchronized.
Modifications:
Remove synchronized keyword.
Result:
No more unnessary synchronized in SpdySessionHandler.
Motivation:
In NIO and OIO we throw a ChannelException if a ChannelConfig operation fails. We should do the same with epoll to be consistent.
Modifications:
Use ChannelException
Result:
Consistent behaviour across different transport implementations.
Motivation:
https://github.com/netty/netty/pull/4143 addressed a few ordering issues but an ordering issue still remained if the Promise for a write completes, and a listener of that promise does a write on a peer channel. The ordering was subject to how potentially 2 different executors would run a task, but it should be coordinated such that the first write is read first.
Modifications:
- Keep track of the finishPeerRead task run on the executor if necessary and ensure it completes before current channel read occurs
Result:
Ordering of events for echo type situations is preserved.
Motivation:
When a LocalChannel write operation occurs, the promise associated with the write operation is marked successful when it is added to the peer's queue, but before the peer has actually received the data. If the promise callback closes the channel then a race condition exists where the close event may occur before the data is delivered. We should preserve ordering of events.
Modifications:
- LocalChannel should track when a write is in progress, and if a close operation happens make sure the peer gets all pending read operations.
Result:
LocalChannel preserves order of operations.
Fixes https://github.com/netty/netty/issues/4118
Motivation:
The alpn / npn dependency versions are dependent on java version. If a java version 1.8+ is used that is not explicitly listed in the pom file then ALPN tests will fail because the java 1.7 version of alpn will be loaded by out pom file.
Modifications:
- Ensure there is a latest version to fall back up for npn 1.7+
- Ensure there is a latest version to fall back upon from alpn 1.8+
Result:
Build can complete despite having a newer jdk which is not listed in our pom file.
Motivation:
DefaultPropertyKey.index is currently private and accessed outside the class's scope.
Modifications:
- Change access level to package private
Result:
No chance of synthetic method generation for accessing this field
Motivation:
The latches in InboundHttp2ToHttpAdapterTest were volatile and reset during the tests. This resulted in race conditions and sometimes the tests would be waiting on old latches that were not the same latches being counted down when messages were received.
Modifications:
- Remove volatile latches from tests
Result:
More reliable tests with less race conditions.
Motivation:
for debugging and metrics reasons its sometimes useful to be able to get details of the the Thread that powers a SingleThreadEventExecutor.
Modifications:
- Expose ThreadProperties
- Add unit test.
Result:
It's now possible to get details of the Thread that powers a SingleThreadEventExecutor.
Motivation:
Sometimes it is useful to disable recycling completely if memory constraints are very tight.
Modifications:
Allow to use -Dio.netty.recycler.maxCapacity=0 to disable recycling completely.
Result:
It's possible to disable recycling now.
Motivation:
When try to get SO_LINGER from a fd that is closed an Exception is thrown. We should only try to get SO_LINGER if the fd is still open otherwise an Exception is thrown that can be ignored anyway.
Modifications:
First check if the fd is still open before try to obtain SO_LINGER setting when get the closeExecutor. This is also the same that we do in the NIO transport.
Result:
No more exception when calling unsafe.close() on a channel that has a closed file descriptor.
Motivation:
OioSctpChannel.doReadMessages is iterating over the selected keys, and ignoring each selected key. It is not known why this is needed and no other channel implementation does this.
Modifications:
- Stop iterating over selected keys, and just read like other channels
Result:
No unnecessary iteration in OioSctpChannel.doReadMessages.
Fixes https://github.com/netty/netty/issues/3884
Motivation:
There currently exists http.HttpUtil, http2.HttpUtil, and http.HttpHeaderUtil. Having 2 HttpUtil methods can be confusing and the utilty methods in the http package could be consolidated.
Modifications:
- Rename http2.HttpUtil to http2.HttpConversionUtil
- Move http.HttpHeaderUtil methods into http.HttpUtil
Result:
Consolidated utilities whose names don't overlap.
Fixes https://github.com/netty/netty/issues/4120
Motivation:
Doc of ChannelGroup says, that it can be used to manage server and child channels at once.
However, in DefaultChannelGroup, there is a race condition. When a server channel accepts a child, it schedules its
registration on an event loop, which takes some time. If the ChannelGroup, which is supposed
to close server and child channels at once, is closed after the child channel has been scheduled
for registration and before this registration actually happens, this child channel is not closed
and remains connected. This could lead to connection leaks.
Modifications:
To fix this, the DefaultChannelGroup is changed to has a closed flag.
This flag is set to true, just before the close() method is actually closing channels.
The add() method checks after adding a new channel, if this flag has been set to true.
If yes, the new channel is closed. If not, we have the guarantee, that this channel will be
closed by the ChannelGroup, because setting the closed flag to true happens-before closing any channels.
This behaviour can be activated by two new constructors. The old constructors are still there and behave like before.
Therefore, no existing code should be affected directly.
Result:
If activating this feature, the DefaultChannelGroup can be used, for managing server and child channels at once.
But this activating this feature means also, that a ChannelGroup cannot be reused after calling close().
Motivation:
The javadocs are incorrect and so give false impressions of use-pattern.
Modifications:
- Fix javadocs of which operations are allowed from multiple threads concurrently.
- Let isEmpty() work concurrently.
Result:
Correctly document usage-patterns.
Motivation:
The method implementions for setSoLinger(...) and setTrafficClass(...) were swapped by mistake.
Modifications:
Use the correct implementation for setSoLinger(...) and setTrafficClass(...)
Result:
Correct behaviour when setSoLinger(...) and setTrafficClass(...) are used with the epoll transport.
Motivation:
- DNS lookup sometimes hang because it does not call
tryToFinishResolve()
- Comodo Secure DNS handles negative lookup incorrectly.
Modifications:
- Add missing tryToFinishResolve()
- Remove Comodo Secure DNS servers from the list
Result
- DNS lookup does not hang on non-existent domain names
- More reliable DnsNameResolverTest
Motivation:
ByteToMessageDecoder may call decode after channelInactive is called. This will lead to a NPE.
Modifications:
- Call super.channelInactive() before we process the event in Http2ConnectionHandler
Result:
No more NPE in decode.
Motivation:
Currently the "derived" buffer will only ever be recycled if the release call is made on the "derived" object, and the "wrapped" buffer ends up being "fully released" (aka refcount goes to 0). From my experience this is not the common use case and thus the "derived" buffers will not be recycled.
Modifications:
- revert https://github.com/netty/netty/pull/3788
Result:
Less complexity, and less code to create new objects in majority of cases.
Related: #4065
Motivation:
DnsNameResolver was using a special Iterable/Iterator implementation
that yields an infinite stream of DNS server addresses. However, this
seems to cause confusion.
Modifications:
- Make DnsServerAddresses an abstract class with an abstract stream()
method that returns DnsServerAddressStream
- Add DnsServerAddressStream that yields DNS server address infinitely
- Remove DnsServerResolver(Group) constructors that accept only a single
server address, which wasn't very useful in practice
- Extract the DnsServerAddresses implementations to top level
- DnsServerAddresses.defaultAddresses() now returns DnsServerAddresses.
- Add DnsServerAddresses.defaultAddressList() instead
Result:
Less confusion and more explicitness
Motivation:
Commit 0d8ce23c83 failed to fix the Host header processing. Host is not a URI but is instead defined in https://tools.ietf.org/html/rfc3986#section-3.2.2 as host = IP-literal / IPv4address / reg-name
Modifications:
- Host should not be treated as a URI.
- We should be more explicit about required fields, and unexpected input by throwing exceptions.
Result:
Translation from HTTP/1.x to HTTP/2 is more correct.
Motivation:
The HttpRequestEncoder.encodeInitialLine can now be consistent with the master branch after 85c79dbbe4
Modifications:
- Use the AsciiString and ByteBufUtil.copy methods
Result:
Consistent behavior/code between 4.1 and master branches.
Motivation:
If a SimpleChannelPromiseAggregator is failed before any new promises are generated, the failure is not propegated through to the aggregated promise.
Modifications:
- Failures should be allowed to occur even if no new promises have been generated
Result:
Failures are always allowed.
