Permalink
Commits on Jan 21, 2017
  1. 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.
    Christopher Exell committed with Scottmitch Jan 20, 2017
  2. Ensure calling ReferenceCountedOpenSslEngine.wrap(...) after closeOut…

    …bound() 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].
    normanmaurer committed Jan 20, 2017
Commits on Jan 20, 2017
  1. Update netty-tcnative

    Motivation:
    
    We released a new netty-tcnative version as a memory leak was fixed.
    
    Modifications:
    
    Update netty-tcnative.
    
    Result:
    
    Fixes [#6249].
    normanmaurer committed Jan 20, 2017
Commits on Jan 19, 2017
  1. 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.
    normanmaurer committed Jan 12, 2017
  2. Use less memory during writes when using SslHandler with SslProvider.…

    …OpenSsl
    
    Motivation:
    
    In commit fc3c9c9 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 fc3c9c9.
    normanmaurer committed Jan 19, 2017
  3. 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.
    tbrooks8 committed with normanmaurer Dec 19, 2016
  4. 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 #6209.
    Scottmitch committed Jan 14, 2017
  5. 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.
    normanmaurer committed Jan 19, 2017
  6. 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].
    normanmaurer committed Jan 19, 2017
  7. 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
    normanmaurer committed Jan 13, 2017
  8. Add SslCloseCompletionEvent that is fired once a close_notify was rec…

    …eived
    
    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.
    normanmaurer committed Jan 19, 2017
  9. Level initialization cleanup.

    doom369 committed with Scottmitch Jan 15, 2017
  10. PlatformDependent#getClassLoader fails in restrictive classloader env…

    …ironment
    
    Motivation:
    #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 #6246.
    Scottmitch committed Jan 18, 2017
  11. Typo error: Method invoker() no longer exists

    Method invoker() no longer exists
    zhoucen committed with normanmaurer Jan 19, 2017
  12. Removed unnecessary pattern matching during number paring and unneces…

    …sary 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.
    doom369 committed with normanmaurer Jan 18, 2017
  13. 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]
    normanmaurer committed Jan 17, 2017
  14. Follow-up cleanup of 0c48265

    normanmaurer committed Jan 19, 2017
  15. 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.
    normanmaurer committed Jan 17, 2017
  16. 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.
    normanmaurer committed Jan 16, 2017
  17. 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.
    normanmaurer committed Jan 11, 2017
  18. 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.
    normanmaurer committed Jan 13, 2017
  19. Fix possible IOOBE when calling ReferenceCountedSslEngine.unwrap(...)…

    … with heap buffers.
    
    Motivation:
    
    fc3c9c9 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.
    normanmaurer committed Jan 12, 2017
  20. 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 #6210.
    Scottmitch committed Jan 13, 2017
Commits on Jan 18, 2017
  1. 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.
    jasobrown committed with Scottmitch Oct 14, 2016
  2. 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.
    Scottmitch committed with normanmaurer Jan 17, 2017
Commits on Jan 12, 2017
  1. [#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.
    normanmaurer committed Dec 19, 2016
  2. 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.
    normanmaurer committed Jan 7, 2017
  3. 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].
    normanmaurer committed Dec 22, 2016
  4. Add unit test that shows LineBasedFrameDelimiter correctly handles fr…

    …agmented data.
    
    Motivation:
    
    Verify everything works as expected.
    
    Modifications:
    
    Added testcase.
    
    Result:
    
    More test-coverage.
    johnou committed with normanmaurer Jan 11, 2017
Commits on Jan 11, 2017
  1. 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 #6165.
    Scottmitch committed Jan 7, 2017
  2. 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 #6163
    Scottmitch committed Dec 31, 2016