Commit Graph

8846 Commits

Author SHA1 Message Date
Scott Mitchell
5257ec49ec DnsNameResolverContext reuse of DnsServerAddressStream without duplicate
Motivation:
DnsServerAddressStream provides an iterator like interface but maybe expected to start at a specific point upon each new usage. If a DnsServerAddressStream is re-used in multiple independent iterations the order of iteration maybe incorrect. DnsNameResolverContext has a fallback DnsServerAddressStream reference if the cache doesn't contain a hit, but it is shared across multiple independent iterations. This may lead to undesirable DNS query order.

Modifications:
- DnsNameResolverContext#getNameServers should duplicate the default DnsServerAddressStream

Result:
Consistent iteration over the default DnsServerAddressStream in DnsNameResolverContext.
2018-02-02 07:24:11 +01:00
Scott Mitchell
3f3d309a28
JdkSslContext supported cipher suites incorrect
Motivation:
JdkSslContext builds the list of supported cipher suites, but assumes that ciphers prefixed with SSL_ and TLS_ will be interchangeable. However this is not the case and only applies to a small subset of ciphers. This results in the JdkSslContext attempting to use unsupported ciphers.

Modifications:
- When building the list of ciphers in JdkSslContext we should first check if the engine supports the TLS_ prefix cipher.

Result:
Fixes https://github.com/netty/netty/issues/7673
2018-02-01 09:34:44 -08:00
Scott Mitchell
978a46cc0a SslHandler unwrap out of order promise/event notificaiton
Motivation:
SslHandler#decode methods catch any exceptions and attempt to wrap
before shutting down the engine. The intention is to write any alerts
which the engine may have pending. However the wrap process may also
attempt to write user data, and may also complete the associated
promises. If this is the case, and a promise listener closes the channel
then SslHandler may later propagate a SslHandshakeCompletionEvent user
event through the pipeline. Since the channel has already been closed
the user may no longer be paying attention to user events.

Modifications:
- Sslhandler#decode should first fail the associated handshake promise
and propagate the SslHandshakeCompletionEvent before attempting to wrap

Result:
Fixes https://github.com/netty/netty/issues/7639
2018-02-01 09:25:40 -08:00
Scott Mitchell
fe5c69bdd9 DnsNameResolverContext#followCname only uses first name server
Motivation:
When following a CNAME it is possible there are multiple name servers to query against. However DnsNameResolverContext#followCname explicitly only uses the first name server address when attempting the query. This may lead to resolution failures because we didn't try all the available name servers.

Modifications:
DnsNameResolverContext#followCname should not just try the first name server, but it should try all name servers

Result:
More complete CNAME resolution.
2018-02-01 09:31:07 +01:00
Norman Maurer
95b9b0af5c Increase timeout and decrement number of operations in AbstractByteBufTest.testToStringMultipleThreads
Motivation:

We saw some timeouts on the CI when the leak detection is enabled.

Modifications:

- Use smaller number of operations in test
- Increase timeout

Result:

CI not times out.
2018-01-31 14:57:38 +01:00
Norman Maurer
b874edbf65 DefaultDnsCache should expire all records per hostname when one TTL is reached.
Motivation:

At the moment DefaultDnsCache will expire each record dependong on its own TTL. This may result in unexpected results for the end-user especially if the user for example uses IPV4_PREFERED but the cached AAAA records has a higher TTL then the A records and so the A record was removed. In this case we would only return the AAAA record and not even try to refresh.

Modifications:

Always expire all records for a hostname when one TTL is reached.

Result:

Fixes [#7329]
2018-01-31 14:39:31 +01:00
Norman Maurer
d2bd36fc4c ByteBufUtil.isText method should be safe to be called concurrently
Motivation:

ByteBufUtil.isText(...) may produce unexpected results if called concurrently on the same ByteBuffer.

Modifications:

- Don't use internalNioBuffer where it is not safe.
- Add unit test.

Result:

ByteBufUtil.isText is thread-safe.
2018-01-31 13:47:49 +01:00
Norman Maurer
e72c197aa3 Reflective setAccessible(true) will produce scary warnings on the console when using java9+, dont do it
Motivation:

Reflective setAccessible(true) will produce scary warnings on the console when using java9+, while netty still works. That said users may feel uncomfortable with these warnings, we should not try to do it by default when using java9+.

Modifications:

Add io.netty.tryReflectionSetAccessible  system property which controls if setAccessible(...) will be used. By default it will bet set to false when using java9+.

Result:

Fixes [#7254].
2018-01-30 12:18:34 +01:00
Trustin Lee
bda6cb01c6 Add the NOTICE of the forked portion of Apache Harmony
Motivation:

According to the section 4d of ASLv2:

If the Work includes a "NOTICE" text file as part of its distribution, then any Derivative Works that You distribute must include a readable copy of the attribution notices contained within such NOTICE file, excluding those notices that do not pertain to any part of the Derivative Works, in at least one of the following places: within a NOTICE text file distributed as part of the Derivative Works; within the Source form or documentation, if provided along with the Derivative Works; or, within a display generated by the Derivative Works, if and wherever such third-party notices normally appear. The contents of the NOTICE file are for informational purposes only and do not modify the License. You may add Your own attribution notices within Derivative Works that You distribute, alongside or as an addendum to the NOTICE text from the Work, provided that such additional attribution notices cannot be construed as modifying the License.

Modifications:

- Add a subset of NOTICE.txt of Apache Harmony.

Result:

- Fixes #7588
2018-01-30 11:22:51 +01:00
Julien Hoarau
3e6b54bb59 Fix failing h2spec tests 8.1.2.1 related to pseudo-headers validation
Motivation:

According to the spec:
All pseudo-header fields MUST appear in the header block before regular
header fields. Any request or response that contains a pseudo-header
field that appears in a header block after
a regular header field MUST be treated as malformed (Section 8.1.2.6).

Pseudo-header fields are only valid in the context in which they are defined.
Pseudo-header fields defined for requests MUST NOT appear in responses;
pseudo-header fields defined for responses MUST NOT appear in requests.
Pseudo-header fields MUST NOT appear in trailers.
Endpoints MUST treat a request or response that contains undefined or
invalid pseudo-header fields as malformed (Section 8.1.2.6).

Clients MUST NOT accept a malformed response. Note that these requirements
are intended to protect against several types of common attacks against HTTP;
they are deliberately strict because being permissive can expose
implementations to these vulnerabilities.

Modifications:

- Introduce validation in HPackDecoder

Result:

- Requests with unknown pseudo-field headers are rejected
- Requests with containing response specific pseudo-headers are rejected
- Requests where pseudo-header appear after regular header are rejected
- h2spec 8.1.2.1 pass
2018-01-29 19:42:56 -08:00
Norman Maurer
c795e8897b Convert Http2Error.STREAM_CLOSED to ClosedChannelException when using child channels
Motivation:

We should convert Http2Exceptions that are produced because of STREAM_CLOSED to ClosedChannelException when hand-over to the child channel to make it more consistent with other transports.

Modifications:

- Check if STREAM_CLOSED is used and if so create a new ClosedChannelException (while preserve the original exception as cause) and use it in the child channel
- Ensure STREAM_CLOSED is used in DefaultHttp2RemoteFlowController when writes are failed because of a closed stream.
- Add testcase

Result:

More consistent and correct exception usage.
2018-01-29 17:50:29 -08:00
Norman Maurer
6e6edb59e7 Remove unused variable in DefaultHttp2StreamChannel
Motivation:

We should remove unused variable (was never read).

Modifications:

Remove unused variable (was never read).

Result:

Cleanup.
2018-01-29 11:32:22 +01:00
Jason
9dd5c928f3 Add java-doc for implemented methods of io.netty.util.concurrent.Future#cancel(boolean mayInterruptIfRunning)
Motivation:

The methods implement io.netty.util.concurrent.Future#cancel(boolean mayInterruptIfRunning) which actually ignored the param mayInterruptIfRunning.We need to add comments for the `mayInterruptIfRunning` param.

Modifications:

Add comments for the `mayInterruptIfRunning` param.

Result:

People who call the `cancel` method will be more clear about the effect of `mayInterruptIfRunning` param.
2018-01-29 11:19:52 +01:00
kashike
1c7125750b Provide an Automatic-Module-Name for the netty-all artifact fixes #7644
Motivation:

The netty-all artifact doesn't have a Automatic-Module-Name defined in the manifest like the rest of the projects do, resulting in requires netty.all.

Modification:

Add Automatic-Module-Name

Result:

Correctly work as java9 module.
2018-01-27 20:31:16 +01:00
Thomas Segismont
7f23c34b55 ReadOnlyHttp2Headers.contains always ignores case for values
Motivation:

When checking if a value is present, ReadOnlyHttp2Headers always ignores
case for values.

RFC 7540 says: https://tools.ietf.org/html/rfc7540#section-8.1.2
"header field names are strings of ASCII characters that are compared in a case-insensitive fashion"

But there is no such constraint on header values

Modifications:

Updated ReadOnlyHttp2Headers.contains to compare header value in a
case-sensitive way.

Result:

ReadOnlyHttp2Headers compares header names in a case-insensitive way,
values in a case-sensitive way.
2018-01-27 20:29:40 +01:00
Thomas Segismont
0db652277f DefaultHttp2Headers#contains(CharSequence, CharSequence) does not work with String
Motivation:

If you test a header value providing a String, contains() returns false.
This is due to the implementation inherited from DefaultHeaders using
the JAVA_HASHER.

JAVA_HASHER.equals returns false because a is a String and b an
AsciiString.

Modifications:

DefaultHttp2Headers overrides contains and uses CASE_SENSITIVE_HASHER.

Result:

You can test a header value with any CharSequence implementation.
2018-01-27 20:27:50 +01:00
Scott Mitchell
614b9e0f25 Add tests for Http2MultiplexChannel close promise completion consistency with AbstractChannel
Motivation:
The completion order of promises in Http2MultiplexChannel#close should be consistent with that of AbstractChannel. Otherwise this may result in Future listeners seeing incorrect channel state.

Modifications:
Add tests cases.

Result:
Ensure consistent behavior between Http2MultiplexChannel and AbstractChannel.
2018-01-27 08:57:17 +01:00
Norman Maurer
b423a35783 Correctly handle multiple calls to DefaultHttp2StreamChannel.Unsafe.close(...)
Motivation:

Calling DefaultHttp2StreamChannel.Unsafe.close(...) multiple times should not fail.

Modification:

- Correctly handle multiple calls to DefaultHttp2StreamChannel.Unsafe.close(...)
- Complete closePromise and promise that is given to close(...) in the correct order.
- Add unit test

Result:

Fixes [#7628] and [#7641]
2018-01-27 08:55:35 +01:00
Norman Maurer
b1695fe17d Ensure async failures are correctly propagated to Http2LifecycleManager.onError(...) in all cases.
Motivation:

If DefaultHttp2ConnectionEncoder process outbound operation it sometimes missed to call Http2LifecycleManager.onError(...) when the operation was executed asynchronously.

Modifications:

Make best effort to update flags but still ensure failures are propageted to Http2LifecycleManager.onError(...) in all cases.

Result:

More consistent handling of errors.
2018-01-27 08:46:23 +01:00
Norman Maurer
c43dc3364b Cleanup Http2MultiplexCodec by removing out-dated TODO
Motivation:

Http2MultiplexCodec contains some TODO that is outdated.

Modifications:

Remove TODO which is outdated

Result:

Cleaner code.
2018-01-27 08:42:17 +01:00
Carl Mastrangelo
9ba942e59b Swap header check in ReadOnlyHttp2Headers
Motivation:
Pseudo headers are checked less frequently than normal headers, so
it is more efficient to check the latter first.

Modifications:
Swap the order of the check, and fix minor formatting

Result:
Possibly more efficient header checks
2018-01-26 19:01:02 -08:00
Norman Maurer
e975c5f6fe Reduce objects by directly implement interface in internal implementations of DefaultHttp2RemoteFlowController
Motivation:

We can just implement the interfaces directly and so reduce object creation in DefaultHttp2RemoteFlowController.

Modifications:

Directly implement the interfaces.

Result:

Less object creation.
2018-01-26 18:33:58 -08:00
Norman Maurer
49d1db4113 Http2MultiplexCodec.DefaultHttpStreamChannel.isOpen() / isActive() shoule be false when fireChannelActive() is called
Motivation:

When part of a HTTP/2 StreamChannel the Http2StreamChannel.isOpen() / isActive() should report false within a call to a ChannelInboundHandlers channelInactive() method.

Modifications:

Fullfill promise before call fireChannelInactive()

Result:

Correctly update state / promise before notify handlers. Fixes [#7638]
2018-01-26 17:34:37 -08:00
Thomas Segismont
bed74d8380 Method to check if a Http2 header is present and has a given value
Motivation:

With HTTP1, it's very easy to check if a header is present and has a
given value: you can simply invoke
io.netty.handler.codec.http.HttpHeaders#contains(java.lang.CharSequence, java.lang.CharSequence, boolean)

It is not possible to do the same with HTTP2. You have to get the list
of all headers (returned as String) and then iterate over it invoking
String#equals or String#equalsIgnoreCase

Modifications:

I've added io.netty.handler.codec.http2.Http2Headers#contains and
implemented it in DefaultHttp2Headers, EmptyHttp2Headers and ReadOnlyHttp2Headers.

Result:

You can use AsciiString constants to check if a header is present in a
consice and efficient manner.
2018-01-26 08:33:49 -08:00
Norman Maurer
36304e1f05 Reduce object allocation by using same ChannelFutureListener instance.
Motivation:

When VoidChannelPromise.unvoid() was called we created a new ChannelFutureListener everytime. This is not needed as its stateless.

Modifications:

Reuse the ChannelFutureListener.

Result:

Less object allocations
2018-01-26 08:42:39 +01:00
amizurov
4afabd3f3c Fix StompFrame.copy() does not copy headers
Motivation:
Incorrect behavior for StompFrame.copy() method.

Modification:
Added copying of frame headers

Result:
When you call the StompFrame.copy() method, the headers are also copied.

Fixes [#7561].
2018-01-26 08:30:12 +01:00
Norman Maurer
1879433ae6 ObjectCleanerThread must be a deamon thread to ensure the JVM can always terminate.
Motivation:

The ObjectCleanerThread must be a daemon thread as otherwise we may block the JVM from exit. By using a daemon thread we basically give the same garantees as the JVM when it comes to cleanup of resources (as the GC threads are also daemon threads and the CleanerImpl uses a deamon thread as well in Java9+).

Modifications:

Change ObjectCleanThread to be a daemon thread.

Result:

JVM shutdown will always be able to complete. Fixed [#7617].
2018-01-26 08:25:42 +01:00
Abhijit Sarkar
f99a1985ce Include mvn wrapper to make setup of development env easier
Motivation:
Someone intending to contribute should be able to set up their development environment quickly and easily.

Modifications:
- Added a Maven wrapper such that a local Maven installation isn't necessary.
- Added a .gitattributes such that auto line-endings are enforced.

Result:
- ./mvnw is enough to build.
- Git line-endings are enforced.
- Fixes #7578.
2018-01-26 08:13:17 +01:00
Norman Maurer
1df5b02fd9 Do not fire outbound exception throught the pipeline when using Http2FrameCodec / Http2MultiplexCodec
Motivation:

Usually when using netty exceptions which happen for outbound operations should not be fired through the pipeline but only the ChannelPromise should be failed.

Modifications:

- Change Http2LifecycleManager.onError(...) to take also an boolean that indicate if the error was caused by an outbound operation
- Channel Http2ConnectionHandler.on*Error(...) methods to also take this boolean
- Change Http2FrameCodec to only fire exceptions through the pipeline if these are not outbound operations related
- Add unit test.

Result:

More consistent error handling when using Http2FrameCodec and Http2MultiplexCodec.
2018-01-25 13:42:28 -08:00
Bryce Anderson
8a095d0244 Http2MultiplexCodec should propagate unhandled Http2Frames down the pipeline
Motivation:

Http2MultiplexCodec swallows Http2PingFrames without releasing the payload, resulting in a memory leak.

Modification:

Send unhandled frames down the pipeline for consumption/disposal by another InboundChannelHandler.

Result:

Fixes #7607.
2018-01-25 13:26:45 -08:00
jaymode
f0c76cacc3 Replace reflective access of Throwable#addSuppressed with version guarded access
Motivation:

In environments with a security manager, the reflective access to get the reference to
Throwable#addSuppressed can cause issues that result in Netty failing to load. The main
motivation in this pull request is to remove the use of reflection to prevent issues in
these environments.

Modifications:

ThrowableUtil no longer uses Class#getDeclaredMembers to get the Method that references
Throwable#addSuppressed and instead guards the call to Throwable#addSuppressed with a
Java version check.

Additionally, a annotation was added that suppresses the animal sniffer java16 signature
check on the given method. The benefit of the annotation is that it limits the exclusion
of Throwable to just the ThrowableUtil class and has string text indicating the reason
for suppressing the java16 signature check.

Result:

Netty no longer requires the use of Class#getDeclaredMethod for ThrowableUtil and will
work in environments restricted by a security manager without needing to grant reflection
permissions.

Fixes #7614
2018-01-25 19:56:17 +01:00
Matteo Bertozzi
b640797de1 Fix HttpPostMultipartRequestDecoder.splitMultipartHeader() String index out of range: -1 with empty header
Motivation:

A Malformed empty header value (e.g. Content-Type: \r\n) will trigger a String index out of range
while trying to parse the multi-part request, using the HttpPostMultipartRequestDecoder.

Modification:

Ensure that the substring() method is called passing the endValue >= valueStart.
In case of an empty header value, the empty header value associated with the header key will be returned.

Result:

Fixes #7620
2018-01-25 14:03:35 +01:00
jaymode
c0e84070b0 Set thread context classloader in a doPrivileged block
Motivation:

In a few classes, Netty starts a thread and then sets the context classloader of these threads
to prevent classloader leaks. The Thread#setContextClassLoader method is a privileged method in
that it requires permissions to be executed when there is a security manager in place. Unless
these calls are wrapped in a doPrivileged block, they will fail in an environment with a security
manager and restrictive policy in place.

Modifications:

Wrap the calls to Thread#setContextClassLoader in a AccessController#doPrivileged block.

Result:

After this change, the threads can set the context classloader without any errors in an
environment with a security manager and restrictive policy in place.
2018-01-25 10:55:34 +01:00
Scott Mitchell
4921f62c8a
HttpResponseStatus object allocation reduction
Motivation:
Usages of HttpResponseStatus may result in more object allocation then necessary due to not looking for cached objects and the AsciiString parsing method not being used due to CharSequence method being used instead.

Modifications:
- HttpResponseDecoder should attempt to get the HttpResponseStatus from cache instead of allocating a new object
- HttpResponseStatus#parseLine(CharSequence) should check if the type is AsciiString and redirect to the AsciiString parsing method which may not require an additional toString call
- HttpResponseStatus#parseLine(AsciiString) can be optimized and doesn't require and may not require object allocation

Result:
Less allocations when dealing with HttpResponseStatus.
2018-01-24 22:01:52 -08:00
Norman Maurer
b769ec0934 Reduce overhead of cancel flowcontrolled writes.
Motivation:

When we cancel the flowcontrolled writes we did create a new StreamException for each write that was enqueued. Creating Exceptions is very expensive due of filling the stacktrace.

Modifications:

Only create the StreamException once and reuse the same for all the flowcontrolled writes (per stream).

Result:

Less expensive to cancel flowcontrolled writes.
2018-01-24 19:42:13 +01:00
Scott Mitchell
2d815fa752 DefaultChannelPipeline will not invoke handler if events are fired from handlerAdded
Motiviation:
DefaultChannelPipeline and AbstractChannelHandlerContext maintain state
which indicates if a ChannelHandler should be invoked or not. However
the state is updated to allow the handler to be invoked only after the
handlerAdded method completes. If the handlerAdded method generates
events which may result in other methods being invoked on that handler
they will be missed.

Modifications:
- DefaultChannelPipeline should set the state before calling
handlerAdded

Result:
DefaultChannelPipeline will allow events to be processed during the
handlerAdded process.
2018-01-24 10:32:48 +01:00
Henning Rohlfs
27ff15319c Reduce memory allocations in StompSubframeDecoder.readHeaders
Motivation:
When decoding stomp frames a lot of unnecessary character arrays are created when parsing headers.
For every header, an array is created to read the line into and then more when splitting the line at the colon.

Modifications:
Parse key and value of a header while reading the line instead of afterwards.
Reuse a single AppendableCharSequence.
Reduce initial size of AppendableCharSequence when reading the command as it is expected to be short.

Result:
Allocations when parsing stomp frames have dropped significantly.
2018-01-23 15:27:57 +01:00
Norman Maurer
4c1e0f596a Use FastThreadLocal for CodecOutputList
Motivation:

We used Recycler for the CodecOutputList which is not optimized for the use-case of access only from the same Thread all the time.

Modifications:

- Use FastThreadLocal for CodecOutputList
- Add benchmark

Result:

Less overhead in our codecs.
2018-01-23 11:34:28 +01:00
Norman Maurer
e305488c5a Fix race-condition when using DnsCache in DnsNameResolver
Motivation:

The usage of DnsCache in DnsNameResolver was racy in general. First of the isEmpty() was not called in a synchronized block while we depended on synchronized. The other problem was that this whole synchronization only worked if the DefaultDnsCache was used and the returned List was not wrapped by the user.

Modifications:

- Rewrite DefaultDnsCache to not depend on synchronization on the returned List by using a CoW approach.

Result:

Fixes [#7583] and other races.
2018-01-23 08:04:33 +01:00
Scott Mitchell
46dac128f7
Http2FrameCodec WindowUpdate bug
Motivation:
Http2FrameCodec increases the initialWindowSize when the user attempts to increase the connection flow control window. The initialWindowSize should only be touched as a result of a SETTINGS frame, and otherwise may result in flow control getting out of sync with our peer.

Modifications:
- Http2FrameCodec shouldn't update the initialWindowSize when a WindowUpdateFrame is written on the connection channel

Result:
More correct WindowUpdate processing.
2018-01-22 11:17:56 -08:00
Norman Maurer
949c515333 Provide a Docker image for reproducible builds
Motivation:

It would be good to provide a docker image for people that want to build netty on linux.

Modifications:

Add a docker file

Result:

People can more easily build netty. Fixes [#7585].
2018-01-22 20:02:17 +01:00
Norman Maurer
336bea9dc5 Fix ByteBuf.nioBuffer(...) and nioBuffers(...) docs to reflect reality.
Motivation:

Depending on the implementation of ByteBuf nioBuffer(...) and nioBuffers(...) may either share the content or return a ByteBuffer that contains a copy of the content.

Modifications:

Fix javadocs.

Result:

Correct docs.
2018-01-22 19:50:08 +01:00
Norman Maurer
ea58dc7ac7 [maven-release-plugin] prepare for next development iteration 2018-01-21 12:53:51 +00:00
Norman Maurer
96c7132dee [maven-release-plugin] prepare release netty-4.1.20.Final 2018-01-21 12:53:34 +00:00
Thomas Devanneaux
3ae57cf302 ByteBuf.toString(Charset) is not thread-safe
Motivation:

Calling ByteBuf.toString(Charset) on the same buffer from multiple threads at the same time produces unexpected results, such as various exceptions and/or corrupted output. This is because ByteBufUtil.decodeString(...) is taking the source ByteBuffer for CharsetDecoder.decode() from ByteBuf.internalNioBuffer(int, int), which is not thread-safe.

Modification:

Call ByteBuf.nioBuffer() instead of ByteBuf.internalNioBuffer() to get the source buffer to pass to CharsetDecoder.decode().

Result:

Fixes the possible race condition.
2018-01-21 09:02:42 +01:00
Scott Mitchell
031bad60dc ObjectCleaner should continue cleaning despite exceptions
Motivation:
ObjectCleaner inovkes a Runnable which may execute user code (FastThreadLocal#onRemoval) and therefore exceptions maybe thrown. If an exception is thrown the cleanup thread will exit prematurely and we may never finish cleaning up which will result in leaks.

Modifications:
- ObjectCleaner should suppress exceptions and continue cleaning

Result:
ObjectCleaner will reliably clean despite exceptions being thrown.
2018-01-19 20:09:20 +01:00
Scott Mitchell
f72f162e16 ObjectCleaner may indefinitely block on ReferenceQueue#poll
Motivation:
ObjectCleaner polls a ReferenceQueue which will block indefinitely. However it is possible there is a race condition between the live set of objects being empty due to the WeakReference being cleaned/cleared and polling the queue. If this situation occurs the cleanup thread may never unblock if no more objects are added to the live set, and may result in an application's failure to gracefully close.

Modifications:
- ReferenceQueue.remove should use a timeout to compensate for the race condition, and avoid dead lock

Result:
No more dead lock in ObjectCleaner when polling the ReferenceQueue.
2018-01-19 18:51:56 +01:00
Scott Mitchell
322a062185 HpackDecoderTest cleanup
Motivation:
public ExpectedException fields should be final.
setUp method signature has a throws, but it isn't necessary
2018-01-19 17:26:06 +01:00
Julien Hoarau
61dba95091 Fix h2spec test 4.3 about invalid header block
Motivation:

HPackDecoder works on entire header block, we shouldn't encounter
incomplete header fields. If we do we should treat it as
a decoding error and according to the specification:

A decoding error in a header block MUST be treated as
a connection error (Section 5.4.1) of type COMPRESSION_ERROR.

Modifications:

* Check final state in HpackDecoder once we've decoded all the data.

Result:

* Throw a connection error if we receive incomplete header fields
* H2spec 4.3 tests all passes
2018-01-18 11:01:20 -08:00
Norman Maurer
1740f366eb Fail fast when DefaultChannelPromise is constructed with null as Channel.
Motivation:

We should fail fast when DefaultChannelPromise is constructed with null as Channel as otherwise it will fail with a NPE once we call setSuccess / setFailure.

Modifications:

Add null check and test.

Result:

Fail fast.
2018-01-18 18:57:42 +00:00