Netty

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Ensure we only add OpenSslEngine to the OpenSslEngineMap when handshake is started

Motivation:

We need to ensure we only add the OpenSslEngine to the OpenSslEngineMap when the handshake is started as otherwise we may produce a memory leak when the OpenSslEngine is created but not actually used. This can for example happen if we encounter a connection refused from the remote peer. In this case we will never remove the OpenSslEngine from the OpenSslEngineMap and so it will never be collected (as we hold a reference). This has as affect that the finalizer will never be run as well.

Modifications:

- Lazy add the OpenSslEngine to the OpenSslEngineMap to elimate possible leak.

- Call OpenSslEngine.shutdown() when SslHandler is removed from the ChannelPipeline to free memory asap in all cases.

Result:

No more memory leak with OpenSslEngine if connection is refused.

Throw exception if KeyManagerFactory is used with OpenSslServerContext

Motivation:

We currently not supported using KeyManagerFactory with OpenSslServerContext and so should throw an exception if the user tries to do so. This will at least not give suprising and hard to debug problems later.

Modifications:

Throw exception if a user tries to construct a OpenSslServerContext with a KeyManagerFactory

Result:

Fail fast if the user tries to use something that is not supported.

Respect ClientAuth set via OpenSslEngine constructor

Motivation:

When ClientAuth is set via SslContextBuilder we pass it into the OpenSslEngine constructor. Due a bug we missed to call the correct native methods and so never enabled ClientAuth in this case.

Modifications:

Correctly call setClientAuth(...) in the constructor if needed.

Result:

client auth also works when configured via the SslContextBuilder and OPENSSL is used.

Moving KObjectHashMapTest to propert directory

Motivation:

The KObjectHashMapTest is in a directory called "io.netty.util.collection" rather than "io/netty/util/collection". This causes the generated tests to be created in the wrong directory as well.

Modifications:

Moved the file.

Result:

Fixes #4546

Extract SocketAdress logic from NameResolver

Motivation:

As discussed in #4529, NameResolver design shouldn't be resolving SocketAddresses (or String name + port) and return InetSocketAddresses. It should resolve String names and return InetAddresses.

This SocketAddress to InetSocketAddresses resolution is actually a different concern, used by Bootstrap.

Modifications:

Extract SocketAddress to InetSocketAddresses resolution concern to a new class hierarchy named AddressResolver.

These AddressResolvers delegate to NameResolvers.

Result:

Better separation of concerns.

Note that new AddressResolvers generate a bit more allocations because of the intermediate Promise and List<InetAddress>.

  1. … 6 more files in changeset.
Fix AsciiString.contentEqualsIgnoreCase

Motivation:

Related to issue #4564.

AsciiString.contentEqualsIgnoreCase fails when comparing two AsciiStrings of the same length

Modifications:

Compare the values of the first AsciiString to the second AsciiString

Result:

AsciiString.contentEqualsIgnoreCase works as expected

DefaultPromiseTest dead code removal

Motivation:

DefaultPromiseTest has dead code which was left over from a code restructure. Shared code between 2 tests was moved into a common method, but some code which was not cleaned up in each of these methods after the code was moved.

Modifications:

- Delete dead code in DefaultPromiseTest

Result:

Less dead code

Fix compile error introduced by e25a29a180360a942faee8d09b6ec8f045b44745

Move Hex dump related util from ByteBufUtil to inner class

Motivation:

Initialisation of the ByteBufUtil class, a class frequently used is

delayed because a significant number of String operations is performed to

fill a HEXDUMP_ROWPREFIXES array. This array also sticks to the Strings

forever.

It is quite likely that applications never use the hexdump facility.

Modification:

Moved the static initialisation and references to a static inner class.

This delays initialisation (and memory usage) until actually needed.

The API is kept as is.

Result:

Faster startup time, less memory usage for most netty using applications.

AbstractFuture should not wrap CancellationException

Motivation:

AbstractFuture currently wraps CancellationException in a ExecutionException. However the interface of Future says that this exception should be directly thrown.

Modifications:

- Throw CancellationException from AbstractFuture.get

Result:

Interface contract for CancellationException is honored in AbstractFuture.

Make DnsNameResolverGroup non-final and overridable

Motivation:

There's no way to override the default settings of the DnsNameResolvers

created by DnsNameResolverGroup because DnsNameResolverGroup is final.

Modifications:

- Make DnsNameResolverGroup non-final

- Add a new overridable protected method 'newResolver()' so that a user

can override it to create an alternative DnsNameResolver instance or

set the non-default properties

Result:

A user can configure the DnsNameResolver.

Don't cycle DNS servers while cycling DNS record types.

Motivation:

Each server should be checked for every record type. Currently, if there

are only two configured servers and the first is down, it is impossible

to query for IPv4 addresses because the second server is only ever

queried for type AAAA.

Modifications:

Do not cycle DNS servers while cycling DNS record types (A and AAAA)

Result:

Name resolution is less fragile when the number of available DNS servers

is 2.

HttpConversionUtil does not account for COOKIE compression

Motivation:

The HTTP/2 RFC allows for COOKIE values to be split into individual header elements to get more benefit from compression (https://tools.ietf.org/html/rfc7540#section-8.1.2.5). HttpConversionUtil was not accounting for this behavior.

Modifications:

- Modify HttpConversionUtil to support compressing and decompressing the COOKIE values

Result:

HttpConversionUtil is compatible with https://tools.ietf.org/html/rfc7540#section-8.1.2.5)

Fixes https://github.com/netty/netty/issues/4457

Fix the incorrect usage/value of 'Connection: upgrade'

Motivation:

HttpClientUpgradeHandler uses HttpHeaderNames.UPGRADE as the value of

the 'Connection' header, which is incorrect. It should use

HttpHeaderValues.UPGRADE instead (note Names vs Values.)

Also, HttpHeaderValues.UPGRADE should be 'upgrade' rather than

'Upgrade', as defined in:

- https://tools.ietf.org/html/rfc7230#section-6.7

Modifications:

- Use HttpHeaderValues.UPGRADE for a 'Connection' header

- Lowercase the value of HttpHeaderValues.UPGRADE

Result:

- Fixes #4508

- Correct behavior

Fix IllegalReferenceCountException caused by HttpClientCodec.upgradeFrom()

Motivation:

On a successful protocol upgrade in HTTP, HttpClientUpgradeHandler calls

HttpClientCodec.upgradeFrom(), which removed both the HTTP encoder and

decoder from the pipeline immediately.

However, because the decoder is in the middle of the decode loop,

removing it from the pipeline immediately will cause the cumulation

buffer to be released prematurely.

This often leads to an IllegalReferenceCountException or missing first

response after the upgrade response.

Modifications:

- Remove the decoder *after* the decode loop is done

Result:

Fixes #4504

Relax the sanity check in HttpClientUpgradeHandler

Motivation:

HttpClientUpgradeHandler currently throws an IllegalStateException when

the server sends a '101 Switching Protocols' response that has no

'Upgrade' header.

Some servers do not send the 'Upgrade' header on a successful protocol

upgrade and we could safely assume that the server accepted the

requested protocol upgrade in such a case, looking from the response

status code (101)

Modifications:

- Do not throw an IllegalStateException when the server responded 101

without a 'Upgrade' header

- Note that we still check the equality of the 'Upgrade' header when it

is present.

Result:

- Fixes #4523

- Better interoperability

[#4449] Remove registered events from eventloop before close

Motivation:

We need to remove all registered events for a Channel from the EventLoop before doing the actual close to ensure we not produce a cpu spin when the actual close operation is delayed or executed outside of the EventLoop.

Modifications:

Deregister for events for NIO and EPOLL socket implementations when SO_LINGER is used.

Result:

No more cpu spin.

Remove unused method in SslContext

Motivation:

We missed to remove a method in SslContext while refactored the implementation. We should remove the method to keep things clean.

Modifications:

Remove unused method.

Result:

Code cleanup.

Use ByteBuf.*LE methods for write and read LE

Motivation:

We recently added methods to ByteBuf to directly write and read LE values. We should use these in the Snappy implementation and so reduce duplication.

Modifications:

Replace manually swapping of values with LE write and read methods of ByteBuf.

Result:

Cleaner code with less duplication.

[#4505] Correctly handle whitespaces in websocket uri's.

Motivation:

If a uri contains whitespaces we need to ensure we correctly escape these when creating the request for the handshake.

Modifications:

- Correctly encode path for uri

- Add tests

Result:

Correctly handle whitespaces when doing websocket upgrade requests.

Ensure we retain the original hostname when connect to a remote peer when using epoll transport.

Motivation:

We should retain the original hostname when connect to a remote peer so the user can still query the origin hostname if getHostString() is used.

Modifications:

Compute a InetSocketAddress from the original remote address and the one returned by the Os.

Result:

Same behavior when using epoll transport and nio transport.

Make cookie encoding conform better to RFC 6265 in STRICT mode.

Motivation:

- On the client, cookies should be sorted in decreasing order of path

length. From RFC 6265:

5.4.2. The user agent SHOULD sort the cookie-list in the following

order:

* Cookies with longer paths are listed before cookies with

shorter paths.

* Among cookies that have equal-length path fields, cookies with

earlier creation-times are listed before cookies with later

creation-times.

NOTE: Not all user agents sort the cookie-list in this order, but

this order reflects common practice when this document was

written, and, historically, there have been servers that

(erroneously) depended on this order.

Note that the RFC does not define the path length of cookies without a

path. We sort pathless cookies before cookies with the longest path,

since pathless cookies inherit the request path (and setting a path

that is longer than the request path is of limited use, since it cannot

be read from the context in which it is written).

- On the server, if there are multiple cookies of the same name, only one

of them should be encoded. RFC 6265 says:

Servers SHOULD NOT include more than one Set-Cookie header field in

the same response with the same cookie-name.

Note that the RFC does not define which cookie should be set in the case

of multiple cookies with the same name; we arbitrarily pick the last one.

Modifications:

- Changed the visibility of the 'strict' field to 'protected' in

CookieEncoder.

- Modified ClientCookieEncoder to sort cookies in decreasing order of path

length when in strict mode.

- Modified ServerCookieEncoder to return only the last cookie of a given

name when in strict mode.

- Added a fast path for both strict mode in both client and server code

for cases with only one cookie, in order avoid the overhead of sorting

and memory allocation.

- Added unit tests for the new cases.

Result:

- Cookie generation on client and server is now more conformant to RFC 6265.

Update JDK SSL Tests to use SSL Context Builder.

Motivation:

Use new / non-deprecated APIs for creating SSL Context

in tests, in order to be able to implement OpenSsl

tests with maximum code reuse.

Modifications:

Use `SslContextBuilder.(forServer|forClient)` instead

of deprecated `JdkSslServerContext` constructor.

Use `ApplicationProtocolConfig` instead of Protocol

Negotiator.

Use custom exception type for skipping tests to avoid

swallowing exceptions arising from tests.

Result:

Exceptions from tests aren't swallowed.

Using new APIs allows reusing same test code for

OpenSsl tests.

DefaultChannelConfig maxMessagesPerRead default not always set

Motivation:

ChannelMetadata has a field minMaxMessagesPerRead which can be confusing. There are also some cases where static instances are used and the default value for channel type is not being applied.

Modifications:

- use a default value which is set unconditionally to simplify

- make sure static instances of MaxMessagesRecvByteBufAllocator are not used if the intention is that the default maxMessagesPerRead should be derived from the channel type.

Result:

Less confusing interfaces in ChannelMetadata and ChannelConfig. Default maxMessagesPerRead is correctly applied.

Fixing spammy logging for CoalescingBufferQueueTest

Motivation:

The CoalescingBufferQueueTest is somewhat relaxed with its releasing of test buffers, using safeRelease to generically deal with tests that may or may not release the buffers. SafeRelease generates logs, however, when the release fails.

Modifications:

Tightened up the individual test methods to verify that the buffers are released properly.

Result:

Fixes #4497

Add Http2HeadersEncoder.ALWAYS_SENSITIVE instance

Motivation:

We already provide a NEVER_SENSITIVE instance,we should add ALWAYS_SENSITIVE as well.

Modifications:

Add ALWAYS_SENSITIVE instance which will always return true when check for sesitive.

Result:

User can reuse code.

Fix race-condition when closing a NioSocketChannel or EpollSocketChannel

Motivation:

Fix a race-condition when closing NioSocketChannel or EpollSocketChannel while try to detect if a close executor should be used and the underlying socket was already closed. This could lead to an exception that then leave the channel / in an invalid state and so could lead to side-effects like heavy CPU usage.

Modifications:

Catch possible socket exception while try to get the SO_LINGER options from the underlying socket.

Result:

No more race-condition when closing the channel is possible with bad side-effects.

Allow to set Http2HeaderEncoder.SensitivityDetector in the Http2ConnectionHandler

Motivation:

Some times the user wants to set a Http2HeaderEncoder.SensitivityDetector when building a Http2ConnectionHandler.

Modifications:

Allow to set Http2HeaderEncoder.SensitivityDetector via builder.

Result:

More flexible building of Http2ConnectionHandler possible.

update pom due to alpn provided

Motiviation:

According to jetty docs the alpn-api should use the provided scope.

Modificaitons:

- change scope to provided for alpn-api

- update for new jdk

Result:

Users of Netty don't run into alpn version conflicts.

Fixes https://github.com/netty/netty/issues/4480

Require RHEL/CentOS 6.7 for releasing Netty

Motivation:

6.7 is the latest stable release in RHEL/CentOS 6 line. Given that most

RHEL/CentOS users have upgraded to 6.7 via yum upgrade, we should bump

our requirement.

Modification:

s/6.6/6.7/g

Result:

'mvn release:*' must be run on RHEL/CentOS 6.7 instead of 6.6.