Commit Graph

7892 Commits

Author SHA1 Message Date
Norman Maurer
5cd8133477 Add unit test that shows we correctly return BUFFER_UNDERFLOW
Motivation:

We should test that we correctly return BUFFER_UNDERFLOW if the src buffer not contains enough data to unwrap it.

Modification:

Add unit test to verify behaviour.

Result:

Better test coverrage of SSLEngine implementations.
2017-01-26 15:04:10 +01:00
Norman Maurer
b69c7723f2 Update jetty-alpn-agent to support latest JDK releases.
Motivation:

We need to update jetty-alpn-agent to support latest JDK releases.

Modifications:

Update jetty-alpn-agent to 2.0.6

Result:

Be able to run tests with latest JDK releases.
2017-01-26 12:18:46 +01:00
ming.ma
44add3c525 Log correct value for useCacheForAllThreads
Motivation:

Log about "-Dio.netty.allocator.useCacheForAllThreads" is missing log placeholder, and so can't output correct value.

Modification:

- Add placeholder

Result:

Fixes #6265 .
2017-01-25 08:01:23 +01:00
Chris Conroy
9bec25a6eb Set the Transfer-Encoding header instead of adding
Motivation:

HttpUtil.setTransferEncodingChunked could add a second Transfer-Encoding
header if one was already present. While this is technically valid, it
does not appear to be the intent of the method.

Result:

Only one Transfer-Encoding header is present after calling this method.
2017-01-25 07:53:53 +01:00
Christopher Exell
d509365974 Move location of where oversized headers that don't exceed the go away limit is done
so that the check occurs after all headers have been read
2017-01-24 15:19:46 -08:00
Norman Maurer
a416b79d86 DnsNameResolver.resolve*(...) never notifies the Future when empty hostname is used.
Motivation:

When an empty hostname is used in DnsNameResolver.resolve*(...) it will never notify the future / promise. The root cause is that we not correctly guard against errors of IDN.toASCII(...) which will throw an IllegalArgumentException when it can not parse its input. That said we should also handle an empty hostname the same way as the JDK does and just use "localhost" when this happens.

Modifications:

- If the try to resolve an empty hostname we use localhost
- Correctly guard against errors raised by IDN.toASCII(...) so we will always noify the future / promise
- Add unit test.

Result:

DnsNameResolver.resolve*(...) will always notify the future.
2017-01-24 21:21:49 +01:00
Norman Maurer
640ef615be Allow to configure SslHandler to wait for close_notify response before closing the Channel and fix racy flush close_notify timeout scheduling.
Motivation:

SslHandler closed the channel as soon as it was able to write out the close_notify message. This may not be what the user want as it may make sense to only close it after the actual response to the close_notify was received in order to guarantee a clean-shutdown of the connection in all cases.

Beside this closeNotifyFlushTimeoutMillis is volatile so may change between two reads. We need to cache it in a local variable to ensure it not change int between. Beside this we also need to check if the flush promise was complete the schedule timeout as this may happened but we were not able to cancel the timeout yet. Otherwise we will produce an missleading log message.

Modifications:

- Add new setter / getter to SslHandler which allows to specify the behavior (old behavior is preserved as default)
- Added unit test.
- Cache volatile closeNotifyTimeoutMillis.
- Correctly check if flush promise was complete before we try to forcibly close the Channel and log a warning.
- Add missing javadocs.

Result:

More clean shutdown of connection possible when using SSL and fix racy way of schedule close_notify flush timeouts and javadocs.
2017-01-24 10:51:16 +01:00
Christopher Exell
907726988d Update hpack Decoder CTOR to allow for overflow in maxHeaderList size, as we do when we apply our ack'ed settings
This prevents us from having the first request, that hasn't ack'ed the setting causing a GOAWAY when we'd would
be under the maxHeaderListSizeGoAway that would have been set after the settings ack.
2017-01-21 10:20:49 -08:00
Norman Maurer
e9fa40d770 Ensure calling ReferenceCountedOpenSslEngine.wrap(...) after closeOutbound() was called will not throw an SSLException
Motivation:

PR [#6238] added guards to be able to call wrap(...) / unwrap(...) after the engine was shutdown. Unfortunally one case was missed which is when closeOutbound() was called and produced some data while closeInbound() was not called yet.

Modifications:

Correctly guard against SSLException when closeOutbound() was called, produced data and someone calls wrap(...) after it.

Result:

No more SSLException. Fixes [#6260].
2017-01-21 07:36:05 +01:00
Norman Maurer
4e04b63746 Update netty-tcnative
Motivation:

We released a new netty-tcnative version as a memory leak was fixed.

Modifications:

Update netty-tcnative.

Result:

Fixes [#6249].
2017-01-20 20:18:03 +01:00
Norman Maurer
1b31397249 Deprecate methods on SslHandler that have other replacements
Motivation:

SslHandler has multiple methods which have better replacements now or are obsolete. We should mark these as `@Deprecated`.

Modifications:

Mark methods as deprecated.

Result:

API cleanup preparation.
2017-01-19 21:34:22 +01:00
Norman Maurer
ac5f701a5c Use less memory during writes when using SslHandler with SslProvider.OpenSsl
Motivation:

In commit fc3c9c9523 I changes the way how we calculate the capacity of the needed ByteBuf for wrap operations that happen during writes when the SslHandler is used. This had the effect that the same capacity for ByteBufs is needed for the JDK implementation of SSLEngine but also for our SSLEngine implementation that uses OpenSSL / BoringSSL / LibreSSL. Unfortunally this had the side-effect that applications that used our SSLEngine implementation now need a lot more memory as bascially the JDK implementation always needs a 16kb buffer for each wrap while we can do a lot better for our SSLEngine implementation.

Modification:

- Resurrect code that calculate a better ByteBuf capacity when using our SSLEngine implementation and so be able to safe a lot of memory
- Add test-case to ensure it works as expected and is not removed again later on.

Result:

Memory footprint of applications that uses our SSLEngine implementation based on OpenSSL / BoringSSL / LibreSSL is back to the same amount of before commit fc3c9c9523.
2017-01-19 21:24:39 +01:00
Tim Brooks
3344cd21ac Wrap operations requiring SocketPermission with doPrivileged blocks
Motivation:

Currently Netty does not wrap socket connect, bind, or accept
operations in doPrivileged blocks. Nor does it wrap cases where a dns
lookup might happen.

This prevents an application utilizing the SecurityManager from
isolating SocketPermissions to Netty.

Modifications:

I have introduced a class (SocketUtils) that wraps operations
requiring SocketPermissions in doPrivileged blocks.

Result:

A user of Netty can grant SocketPermissions explicitly to the Netty
jar, without granting it to the rest of their application.
2017-01-19 21:12:52 +01:00
Dmitriy Dumanskiy
2d11331591 Typo fix in post encoder and replaced static hashmap with array. 2017-01-19 10:44:27 -08:00
Scott Mitchell
2fd42cfc6b HTTP/2 Max Header List Size Bug
Motivation:
If the HPACK Decoder detects that SETTINGS_MAX_HEADER_LIST_SIZE has been violated it aborts immediately and sends a RST_STREAM frame for what ever stream caused the issue. Because HPACK is stateful this means that the HPACK state may become out of sync between peers, and the issue won't be detected until the next headers frame. We should make a best effort to keep processing to keep the HPACK state in sync with our peer, or completely close the connection.
If the HPACK Encoder is configured to verify SETTINGS_MAX_HEADER_LIST_SIZE it checks the limit and encodes at the same time. This may result in modifying the HPACK local state but not sending the headers to the peer if SETTINGS_MAX_HEADER_LIST_SIZE is violated. This will also lead to an inconsistency in HPACK state that will be flagged at some later time.

Modifications:
- HPACK Decoder now has 2 levels of limits related to SETTINGS_MAX_HEADER_LIST_SIZE. The first will attempt to keep processing data and send a RST_STREAM after all data is processed. The second will send a GO_AWAY and close the entire connection.
- When the HPACK Encoder enforces SETTINGS_MAX_HEADER_LIST_SIZE it should not modify the HPACK state until the size has been checked.
- https://tools.ietf.org/html/rfc7540#section-6.5.2 states that the initial value of SETTINGS_MAX_HEADER_LIST_SIZE is "unlimited". We currently use 8k as a limit. We should honor the specifications default value so we don't unintentionally close a connection before the remote peer is aware of the local settings.
- Remove unnecessary object allocation in DefaultHttp2HeadersDecoder and DefaultHttp2HeadersEncoder.

Result:
Fixes https://github.com/netty/netty/issues/6209.
2017-01-19 10:42:43 -08:00
Norman Maurer
ead87b7df8 Respect resolvedAddressTypes when follow CNAME records.
Motivation:

When we follow CNAME records we should respect resolvedAddressTypes and only query A / AAAA depending on which address types are expected.

Modifications:

Check if we should query A / AAAA when follow CNAMEs depending on resolvedAddressTypes.

Result:

Correct behaviour when follow CNAMEs.
2017-01-19 19:36:56 +01:00
Norman Maurer
71efb0b39e Do not replace System.err during Slf4JLoggerFactory construction
Motivation:

Replacing System.err during Slf4JLoggerFactory construction is problematic as another class may optain the System.err reference before we set it back to the original value.

Modifications:

Remove code that temporary replaced System.err.

Result:

Fixes [#6212].
2017-01-19 19:28:01 +01:00
Norman Maurer
cf8e07f62f Run all tests in SSLEngineTest with heap, direct and mixed buffers
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
2017-01-19 19:18:31 +01:00
Norman Maurer
d55c321306 Add SslCloseCompletionEvent that is fired once a close_notify was received
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.
2017-01-19 19:15:24 +01:00
Norman Maurer
cd9008f95b Remove unnecessray for loop missed by fac0ca8319 2017-01-19 19:13:57 +01:00
Dmitriy Dumanskiy
0e654f77e2 Level initialization cleanup. 2017-01-19 10:11:38 -08:00
Scott Mitchell
9a4aa617f4 PlatformDependent#getClassLoader fails in restrictive classloader environment
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.
2017-01-19 09:01:36 -08:00
周岑
86020a2858 Typo error: Method invoker() no longer exists
Method invoker() no longer exists
2017-01-19 12:16:31 +01:00
Dmitriy Dumanskiy
141554f5d1 Removed unnecessary pattern matching during number paring and unnecessary toLowerCase() invocation.
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.
2017-01-19 10:11:55 +01:00
Norman Maurer
fac0ca8319 Warn about not-supported ChannelOption when bootstrap Channels.
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]
2017-01-19 08:27:19 +01:00
Norman Maurer
4f6ef69582 Follow-up cleanup of 0c4826586f 2017-01-19 08:01:14 +01:00
Norman Maurer
0c4826586f Cleanup PlatformDependent* code
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.
2017-01-19 07:55:18 +01:00
Norman Maurer
d7ff71a3d1 Check if DnsCache is null in DnsNameResolver constructor.
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.
2017-01-19 07:54:36 +01:00
Norman Maurer
d37702aa69 Ensure SslHandler.sslCloseFuture() is notified in all cases.
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.
2017-01-19 07:53:08 +01:00
Norman Maurer
91951a51ad Ensure calling ReferenceCountedSslEngine.unwrap(...) / wrap(...) can be called after it was closed
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.
2017-01-19 07:51:27 +01:00
Norman Maurer
821501717b Fix possible IOOBE when calling ReferenceCountedSslEngine.unwrap(...) with heap buffers.
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.
2017-01-19 07:47:12 +01:00
Scott Mitchell
b701da8d1c HTTP/2 HPACK Integer Encoding Bugs
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.
2017-01-18 18:36:47 -08:00
Jason Brown
3ea807e375 Flush LZ4FrameEncoder buffer when channel flush() is received.
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.
2017-01-18 10:57:21 -08:00
Scott Mitchell
c590e3bd63 HTTP/2 relax test timeouts
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.
2017-01-18 08:02:15 +01:00
Norman Maurer
7f01da8d0f [maven-release-plugin] prepare for next development iteration 2017-01-12 11:36:51 +01:00
Norman Maurer
7a21eb1178 [maven-release-plugin] prepare release netty-4.1.7.Final 2017-01-12 11:35:58 +01:00
Norman Maurer
be8c16cd0c [#6141] OpenSSLContext Mutual Auth does not announce acceptable CAs
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.
2017-01-12 08:05:15 +01:00
Norman Maurer
dd055c01c7 Ensure ReferenceCountedOpenSslEngine not swallow the close_notify
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.
2017-01-12 07:57:37 +01:00
Norman Maurer
eb5dc4bced Correctly handle IPV6 in HttpProxyHandler
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].
2017-01-12 07:51:37 +01:00
Johno Crawford
84410f97af Add unit test that shows LineBasedFrameDelimiter correctly handles fragmented data.
Motivation:

Verify everything works as expected.

Modifications:

Added testcase.

Result:

More test-coverage.
2017-01-12 07:50:31 +01:00
Scott Mitchell
d771526f8c HTTP/2 HelloWorld Client Example Bug
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.
2017-01-11 12:23:45 -08:00
Scott Mitchell
ec3d077e0d DefaultHttp2Connection modifying child map while iterating
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
2017-01-11 11:07:18 -08:00
Norman Maurer
3c5e677964 Add assert to ensure we not create an endless loop and fix unsafe publication
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.
2017-01-11 12:14:27 +01:00
Norman Maurer
7a4b0c3297 Add unit test that shows LineBasedFrameDelimiter correctly splits line.
Motivation:

Thought there may be a bug so added a testcase to verify everything works as expected.

Modifications:

Added testcase

Result:

More test-coverage.
2017-01-11 08:00:47 +01:00
Scott Mitchell
631077c793 EPOLL RDHUP processing
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.
2017-01-10 13:11:18 -08:00
Scott Mitchell
583a59abb1 ByteBufUtil.compare int underflow
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
2017-01-10 11:43:59 -08:00
Norman Maurer
2368f238ad Fix typo in inner-class name
Motivation:

There is a typo in the inner-class name.

Modifications:

Fix typo.

Result:

One typo less. Fixes [#6185].
2017-01-10 13:49:52 +01:00
Norman Maurer
72c9fc2084 Use href in WebSocketServer javadoc
Motivation:

We should use <a href=....> in the javadoc of WebSocketServer. See [#6172].

Modifications:

Use href.

Result:

Fix link in xref.
2017-01-10 13:34:44 +01:00
Norman Maurer
c1830c8b4e Fix missleading comment in AbstractChannelHandlerContext.invokeHandler()
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.
2017-01-10 12:05:24 +01:00
Shixiong Zhu
2457f386d8 Set prev to null when setting cursor to head in scavengeSome.
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.
2017-01-07 20:48:46 +01:00