Commit Graph

9897 Commits

Author SHA1 Message Date
Stephane Landelle
6240a4b03f Upgrade jctools from 3.0.0 to 3.1.0 (#10456)
Motivation:

JCTools 3.1.0 is out and includes several fixes, see https://github.com/JCTools/JCTools/releases/tag/v3.1.0

Modification:

Upgrade jctools-core version in pom.xml

Result:

Netty ships latest version of jctools.
2020-08-10 11:16:01 +02:00
Norman Maurer
6fd5550ad7 Add whitelist entry to BlockHound config to workaround issue when TLS… (#10459)
Motivation:

SSLEngineImpl.unwrap(...) may call FileInputStream.read(...) internally when TLS1.3 is used. This will cause an BlockingOperationError when BlockHound is enabled.

For more details see https://mail.openjdk.java.net/pipermail/security-dev/2020-August/022271.html

Modifications:

- Add whitelist entry to BlockHound config for now
- Update NettyBlockHoundIntegrationTest to include testing for this workaround

Result:

No BlockingOperationError when TLS1.3 is used with JDK SSL implementation and BlockHound is enabled
2020-08-10 11:12:52 +02:00
Andrey Mizurov
2b73c93581 Fix #10449, buffer.getBytes(...) not change a file channel position (#10453)
Motivation:

Regression appeared after making changes in fix #10360 .
The main problem here that `buffer.getBytes(buffer.readerIndex(), fileChannel, fileChannel.position(), localsize)`
doesn't change channel position after writes.

Modification:

Manually set position according to the written bytes.

Result:

Fixes #10449 .
2020-08-07 14:02:07 +02:00
Idel Pivnitskiy
e305b6af5e Create a new h2 stream only if the parent channel is still active (#10444)
Motivation:

If a request to open a new h2 stream was made from outside of the
EventLoop it will be scheduled for future execution on the EventLoop.
However, during the time before the `open0` task will be executed the
parent channel may already be closed. As the result,
`Http2MultiplexHandler#newOutboundStream()` will throw an
`IllegalStateException` with the message that is hard to
interpret correctly for this use-case: "Http2FrameCodec not found. Has
the handler been added to a pipeline?".

Modifications:

- Check that the parent h2 `Channel` is still active before creating a
new stream when `open0` task is picked up by EventLoop;

Result:

Users see a correct `ClosedChannelException` in case the parent h2
`Channel` was closed concurrently with a request for a new stream.
2020-08-06 14:04:31 +02:00
skyguard1
9c5dbfd1b6 Replace Class.getClassLoader with io.netty.util.internal.PlatformDependent.getClassLoader in Openssl (#10454)
Motivation:

Replace Class.getClassLoader with io.netty.util.internal.PlatformDependent.getClassLoader in Openssl so it also works when a SecurityManager is in place

Modification:

Replace Class.getClassLoader with io.netty.util.internal.PlatformDependent.getClassLoader in Openssl

Result:

No issues when a SecurityManager is in place
2020-08-06 09:03:01 +02:00
Idel Pivnitskiy
10354b62c8 New H2 stream promise may never complete (#10443)
Motivation:

`Http2StreamChannelBootstrap#open0` invokes
`Http2MultiplexHandler#newOutboundStream()` which may throw an
`IllegalStateException`. In this case, it will never complete
the passed promise.

Modifications:

- `try-catch` all invocations of `newOutboundStream()` and fail
promise in case of any exception;

Result:

New H2 stream promise always completes.
2020-08-06 08:34:44 +02:00
Roman Puchkovskiy
ff3858df36 Do not try to use Unsafe.staticFieldOffset() method under a native image. (#10428)
Motivation:

GraalVM's native images built with native-image tool do not support Unsafe.staticFieldOffset() method (at least, currently). If an application using Netty (and causing initialization of io.netty.util.internal.PlatformDependent0 class) is built into a native image and run, this results in the following error thrown during initialization:

Exception in thread "main" com.oracle.svm.core.jdk.UnsupportedFeatureError: Unsupported method of Unsafe
	at com.oracle.svm.core.util.VMError.unsupportedFeature(VMError.java:86)
	at jdk.internal.misc.Unsafe.staticFieldOffset(Unsafe.java:230)
	at sun.misc.Unsafe.staticFieldOffset(Unsafe.java:662)
	at io.netty.util.internal.PlatformDependent0$5.run(PlatformDependent0.java:294)
	at java.security.AccessController.doPrivileged(AccessController.java:83)
	at io.netty.util.internal.PlatformDependent0.<clinit>(PlatformDependent0.java:279)

This seems to be the reason of the behavior described in #10051.

Modification:

The idea of this commit is to only invoke Unsafe.staticFieldOffset() is we are not in a native image; if we are, behave like if we could not find the field at all.

GraalDetector is borrowed from Spring framework.

Result:

Fixes #10051
2020-08-03 19:50:42 +02:00
Koji Lin
1be1523a9e Fix DnsNameResolver may have LEAK ByteBuf after cancelling the returned future (#10448)
Motivation:

If we cancel the returned future of resolve query, we may get LEAK. Try to release the ByteBuf if netty can't pass the DnsRawRecord to the caller.

Modification:

Using debug mode I saw there are two places that don't handle trySuccess with release. Try to release there.

Result:

Fixes #10447.
2020-08-03 07:58:20 +02:00
Idel Pivnitskiy
e919708edc Make DefaultHttp2FrameStream.stream private (#10442)
Motivation:

`DefaultHttp2FrameStream.stream` is not used outside of its class and
therefore can be private.

Modifications:

- Make `DefaultHttp2FrameStream.stream` private;

Result:

Correct visibility scoping for `DefaultHttp2FrameStream.stream`.
2020-08-03 07:55:29 +02:00
Ruwei
f23704736d Fix bug in Http2FrameClient (#10427)
Motivation:
This request only has headers frame, it should set endOfStream flag, or
it will never get a response.

Modifications:
Set endOfStream=true in header frame.

Result:
Http2FrameClient can get a response now.
2020-08-03 07:54:44 +02:00
Idel Pivnitskiy
300fc243a4 Ignore .shelf/ folder generated by IntelliJ IDEA (#10445)
Motivation:

IntelliJ IDEA may generate a local folder `.shelf/` for version control.
For more information, see
https://www.jetbrains.com/help/idea/shelving-and-unshelving-changes.html

Modifications:

- Add `.shelf/` folder to the `.gitignore` file;

Result:

IntelliJ IDEA's `.shelf/` folder is ignored by git.
2020-08-03 07:52:18 +02:00
Esun Kim
c5de752fa3 Making AES_128_GCM_SHA256 have a higher priority over CHACHA20_POLY1305_SHA256 on HTTP2 (#10418)
Motivation:

From the recent benchmark using gRPC-Java based on Netty's HTTP2, it appears that it prefers `ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256` over `ECDHE_ECDSA_WITH_AES_128_GCM_SHA256` since it uses the Netty HTTPS Cipher list as is. Both are considered safe but `ECDHE_ECDSA_WITH_AES_128_GCM_SHA256` has a good chance to get more optimized implementation. (e.g. AES-NI) When running both on GCP Intel Haswell VM, `TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256` spent 3x CPU time than `TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256`. (Note that this VM supports AES-NI)

From the cipher suites listed on `Intermediate compatibility (recommended)` of [Security/Server Side TLS](https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility), they have a cipher preference which is aligned with this PR.

```
0x13,0x01  -  TLS_AES_128_GCM_SHA256         TLSv1.3  Kx=any   Au=any    Enc=AESGCM(128)             Mac=AEAD
0x13,0x02  -  TLS_AES_256_GCM_SHA384         TLSv1.3  Kx=any   Au=any    Enc=AESGCM(256)             Mac=AEAD
0x13,0x03  -  TLS_CHACHA20_POLY1305_SHA256   TLSv1.3  Kx=any   Au=any    Enc=CHACHA20/POLY1305(256)  Mac=AEAD
0xC0,0x2B  -  ECDHE-ECDSA-AES128-GCM-SHA256  TLSv1.2  Kx=ECDH  Au=ECDSA  Enc=AESGCM(128)             Mac=AEAD
0xC0,0x2F  -  ECDHE-RSA-AES128-GCM-SHA256    TLSv1.2  Kx=ECDH  Au=RSA    Enc=AESGCM(128)             Mac=AEAD
0xC0,0x2C  -  ECDHE-ECDSA-AES256-GCM-SHA384  TLSv1.2  Kx=ECDH  Au=ECDSA  Enc=AESGCM(256)             Mac=AEAD
0xC0,0x30  -  ECDHE-RSA-AES256-GCM-SHA384    TLSv1.2  Kx=ECDH  Au=RSA    Enc=AESGCM(256)             Mac=AEAD
0xCC,0xA9  -  ECDHE-ECDSA-CHACHA20-POLY1305  TLSv1.2  Kx=ECDH  Au=ECDSA  Enc=CHACHA20/POLY1305(256)  Mac=AEAD
0xCC,0xA8  -  ECDHE-RSA-CHACHA20-POLY1305    TLSv1.2  Kx=ECDH  Au=RSA    Enc=CHACHA20/POLY1305(256)  Mac=AEAD
0x00,0x9E  -  DHE-RSA-AES128-GCM-SHA256      TLSv1.2  Kx=DH    Au=RSA    Enc=AESGCM(128)             Mac=AEAD
0x00,0x9F  -  DHE-RSA-AES256-GCM-SHA384      TLSv1.2  Kx=DH    Au=RSA    Enc=AESGCM(256)             Mac=AEAD
```

Modification:

Moving up `AES_128_GCM_SHA256` in the CIPHERS of HTTPS so that it gets priority.

Result:

When connecting to the server supporting both `ECDHE_ECDSA_WITH_AES_128_GCM_SHA256` and `ECDSA_WITH_CHACHA20_POLY1305_SHA256` and respecting the client priority of cipher suites, it will be able to save significant cpu time when running it on machines with AES-NI support.
2020-08-03 07:50:55 +02:00
Norman Maurer
220995f155 Make the TLSv1.3 check more robust and not depend on the Java version… (#10409)
Motivation:

TLSv1.3 is not strictly limited to Java11+ anymore as different vendors backported TLSv1.3 to Java8 as well. We should ensure we make the detection of if TLSv1.3 is supported not depend on the Java version that is used.

Modifications:

- Add SslProvider.isTlsv13Supported(...) and use it in tests to detect if we should run tests against TLSv1.3 as well
- Adjust testcase to work on latest JDK 8 release as well

Result:

Correct detection of TLSv1.3 support even if Java version < 11.
2020-07-17 07:17:56 +02:00
Norman Maurer
8c0f1428af Reduce the scope of synchronized block in PoolArena (#10410)
Motivation:

We shouldn't call incSmallAllocation() in a synchronized block as its backed by a concurrent datastructure

Modifications:

Move call of incSmallAllocation() out of synchronized block

Result:

Minimize scope of synchronized block
2020-07-16 19:41:02 +02:00
skyguard1
72758f7587 Add IndexOutOfBoundsException error message (#10405)
Motivation:

We should provide details about why an IOOBE was thrown

Modification:

Add IndexOutOfBoundsException error information in io.netty.util.internal.AppendableCharSequence and io.netty.handler.codec.CodecOutputList class

Result:

Easier to debug
2020-07-16 11:37:14 +02:00
skyguard1
c78c3bead0 Add default handling for switch statement (#10408)
Motivation:

When a switch statement is used we should always define a `default:` so we don't introduce bugs due fall-through.

Modification:

Add missing `default:`s

Result:

Less error-prone code
2020-07-16 11:35:34 +02:00
Ruwei
0d701d7c3c Review PooledByteBufAllocator in respect of jemalloc 4.x changes and update allocate algorithm.(#10267)
Motivation:

For size from 512 bytes to chunkSize, we use a buddy algorithm. The
drawback is that it has a large internal fragmentation.

Modifications:

1. add SizeClassesMetric and SizeClasses
2. remove tiny size, now we have small, normal and huge size
3. rewrite the structure of PoolChunk
4. rewrite pooled allocate algorithm in PoolChunk
5. when allocate subpage, using lowest common multiple of pageSize and
   elemSize instead of pageSize.
   6. add more tests in PooledByteBufAllocatorTest and PoolArenaTest

Result:
Reduce internal fragmentation and closes #3910
2020-07-16 08:24:27 +02:00
Norman Maurer
7bf1ffb2d4 Support session cache for client and server when using native SSLEngine implementation (#10331)
Motivation:

At the moment we don't support session caching for client side when using native SSLEngine implementation and our implementation of SSLSessionContext is incomplete.

Modification:

- Consume netty-tcnative changes to be able to cache session in an external cache
- Add and adjust unit tests to test session caching
- Add an in memory session cache that is hooked into native SSLEngine

Result:

Support session caching on the client and server side
2020-07-14 15:20:44 +02:00
Gene
6035188246 Simple fix typo (#10403)
Motivation:

Wrong typo in annotation at line 925.

Modifications:

Fix typo. *then -> than.

Result:

Fix typo.
2020-07-14 10:59:25 +02:00
feijermu
eec46183f0 Eliminate a redundant method call in HpackDynamicTable.add(...) (#10399)
Motivation:

The result of `header.size()` is already cached in `headerSize`. There is no need to call it again actually.

Modification:

Replace the second `header.size()` with `headerSize` directly.

Result:

Improve performance slightly.
2020-07-13 17:34:50 +02:00
Norman Maurer
47bbc590ee jdk.tls.client.enableSessionTicketExtension must be respected by OPENSSL and OPENSSL_REFCNT SslProviders (#10401)
Motivation:

jdk.tls.client.enableSessionTicketExtension property must be respect by OPENSSL and OPENSSL_REFCNT SslProvider to ensure a consistent behavior. Due a bug this was not the case and it only worked for OPENSSL_REFCNT but not for OPENSSL.

Modifications:

Move the property check into static method that is used by both

Result:

Correctly respect jdk.tls.client.enableSessionTicketExtension
2020-07-13 16:17:41 +02:00
Norman Maurer
165ade5d9f Correctly return NEED_WRAP if we produced some data even if we could not consume any during SSLEngine.wrap(...) (#10396)
Motivation:

At the moment we may report BUFFER_OVERFLOW when wrap(...) fails to consume data but still prodce some. This is not correct and we should better report NEED_WRAP as we already have produced some data (for example tickets). This way the user will try again without increasing the buffer size which is more correct and may reduce the number of allocations

Modifications:

Return NEED_WRAP when we produced some data but not consumed any.

Result:

Fix ReferenceCountedOpenSslEngine.wrap(...) state machine
2020-07-09 08:52:30 +02:00
Daniel Zou
fca62510f3 Modify OpenSSL native library loading to accommodate GraalVM (#10395)
**Motivation:**

We are interested in building Netty libraries with Ahead-of-time compilation with GraalVM. We saw there was [prior work done on this](https://github.com/netty/netty/search?q=graalvm&unscoped_q=graalvm). We want to introduce a change which will unblock GraalVM support for applications using netty and `netty-tcnative`.

This solves the error [that some others encounter](https://github.com/oracle/graal/search?q=UnsatisfiedLinkError+sslOpCipherServerPreference&type=Issues):

```
Exception in thread "main" java.lang.UnsatisfiedLinkError: io.grpc.netty.shaded.io.netty.internal.tcnative.NativeStaticallyReferencedJniMethods.sslOpCipherServerPreference()I [symbol: Java_io_grpc_netty_shaded_io_netty_internal_tcnative_NativeStaticallyReferencedJniMethods_sslOpCipherServerPreference or Java_io_grpc_netty_shaded_io_netty_internal_tcnative_NativeStaticallyReferencedJniMethods_sslOpCipherServerPreference__]
```

**Modification:**

The root cause of the issue is that in the tcnative libraries, the [`SSL.java` class](783a8b6b69/openssl-dynamic/src/main/java/io/netty/internal/tcnative/SSL.java (L67)) references a native call in the static initialization of the class - the method `sslOpCipherServerPreference()` on line 67 is used to initialize the static variable. But you see that `OpenSsl` also uses[ `SSL.class` to check if `netty-tcnative` is on the classpath before loading the native library](cbe238a95b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java (L123)). 

So this is the problem because in ahead-of-time compilation, when it references the SSL class, it will try to load those static initializers and make the native library call, but it cannot do so because the native library has not been loaded yet since the `SSL` class is being referenced to check if the library should be loaded in the first place.

**Solution:** So the proposed solution here is to just choose a different class in the `tcnative` package which does not make a native library call during static initialization. I just chose `SSLContext` if this is OK.

This should have no negative effects other than unblocking the GraalVM use-case.

**Result:**

It fixes the unsatisfied link error. It will fix error for future users; it is a common error people encounter.
2020-07-08 10:47:10 +02:00
Norman Maurer
0c79863db5 Update to netty-tcnative 2.0.31.Final and make SslErrorTest more robust (#10392)
Motivation:

There was a new netty-tcnative release which we should use. Beside this the SSLErrorTest was quite fragile and so should be adjusted.

Modifications:

Update netty-tcnative and adjust test

Result:

Use latest netty-tcnative release
2020-07-07 10:56:42 +02:00
Bennett Lynch
f7fa9fce72 Add option to HttpObjectDecoder to allow duplicate Content-Lengths (#10349)
Motivation:

Since https://github.com/netty/netty/pull/9865 (Netty 4.1.44) the
default behavior of the HttpObjectDecoder has been to reject any HTTP
message that is found to have multiple Content-Length headers when
decoding. This behavior is well-justified as per the risks outlined in
https://github.com/netty/netty/issues/9861, however, we can see from the
cited RFC section that there are multiple possible options offered for
responding to this scenario:

> If a message is received that has multiple Content-Length header
> fields with field-values consisting of the same decimal value, or a
> single Content-Length header field with a field value containing a
> list of identical decimal values (e.g., "Content-Length: 42, 42"),
> indicating that duplicate Content-Length header fields have been
> generated or combined by an upstream message processor, then the
> recipient MUST either reject the message as invalid or replace the
> duplicated field-values with a single valid Content-Length field
> containing that decimal value prior to determining the message body
> length or forwarding the message.

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

Netty opted for the first option (rejecting as invalid), which seems
like the safest, but the second option (replacing duplicate values with
a single value) is also valid behavior.

Modifications:

* Introduce "allowDuplicateContentLengths" parameter to
HttpObjectDecoder (defaulting to false).
* When set to true, will allow multiple Content-Length headers only if
they are all the same value. The duplicated field-values will be
replaced with a single valid Content-Length field.
* Add new parameterized test class for testing different variations of
multiple Content-Length headers.

Result:

This is a backwards-compatible change with no functional change to the
existing behavior.

Note that the existing logic would result in NumberFormatExceptions
for header values like "Content-Length: 42, 42". The new logic correctly
reports these as IllegalArgumentException with the proper error message.

Additionally note that this behavior is only applied to HTTP/1.1, but I
suspect that we may want to expand that to include HTTP/1.0 as well...
That behavior is not modified here to minimize the scope of this change.
2020-07-06 14:50:15 +02:00
Norman Maurer
f8b3a388ff
Fix compile errors introduced by bad cherry-picks (#10391) 2020-07-06 13:22:31 +02:00
feijermu
9955ef563e Add detailed error message corresponding to the IndexOutOfBoundsException while calling getEntry(...) (#10386)
Motivation:
`getEntry(...)` may throw an IndexOutOfBoundsException without any error messages.


Modification:

Add detailed error message corresponding to the IndexOutOfBoundsException while calling `getEntry(...)` in HpackDynamicTable.java.

Result: 

Easier to debug
2020-07-06 10:19:37 +02:00
violetagg
f9e8c9ca11 Do not report ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify as blocking call (#10387)
Motivation:

When BlockHound is installed,
ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify
is reported as blocking call.

Modifications:

Add allowBlockingCallsInside configuration for
ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify

Result:

Fixes #10384
2020-07-06 09:36:00 +02:00
feijermu
ced38117a2 Add unit test for HpackDynamicTable. (#10389)
Motivation:

`HpackDynamicTable` needs some test cases to ensure bug-free.

Modification:

Add unit test for `HpackDynamicTable`.

Result:

Improve test coverage slightly.
2020-07-06 09:07:57 +02:00
Norman Maurer
8950144567 Correctly include TLS1.3 ciphers in the enabled ciphersuites when using BoringSSL (#10388)
Motivation:

BoringSSL behaves differently then OpenSSL and not include any TLS1.3 ciphers in the returned array when calling SSL_get_ciphers(...). This is due the fact that it also not allow to explicit configure which are supported and which not for TLS1.3. To mimic the behaviour expected by the SSLEngine API we should workaround this.

Modifications:

- Add a unit test that verifies enabled protocols / ciphers
- Add special handling for BoringSSL and tls1.3

Result:

Make behaviour consistent
2020-07-02 21:37:04 +02:00
skyguard1
95f9694150 Fix #10378,ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) ignores  sampling interval (#10383)
Motivation:

newResourceLeakDetector(...) did not correctly pass the samplingInterval parameter and so it was ignored.

Modification:

ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) use the second parameter as the sampling interval of the newly created ResourceLeakDetector.

Result:

Fixes #10378
2020-07-01 10:25:30 +02:00
Norman Maurer
91c75a4a8f Ensure we preserve the original cause of the SSLException in all case… (#10372)
Motivation:

We did not correctly preserve the original cause of the SSLException when all the following are true:

 * SSL_read failed
 * handshake was finished
 * some outbound data was produced durigin SSL_read (for example ssl alert) that needs to be picked up by wrap(...)

Modifications:

Ensure we correctly preserve the original cause of the SSLException and rethrow it once we produced all data via wrap(...)

Result:

Be able to understand why we see an error in all cases
2020-06-25 22:08:57 +02:00
Francesco Nigro
8ff7c99dd2 The bounded Mpsc Queue for no Unsafe users behave differently from Unsafe ones (#10377)
Motivation:

Unsafe users are getting MpscChunkedArrayQueue while no Unsafe ones MpscGrowableAtomicArrayQueue

Modifications:

MpscChunkedAtomicArrayQueue should be used for no Unsafe users (matching MpscChunkedArrayQueue behaviour)

Result:

no Unsafe users uses MpscChunkedAtomicArrayQueue while allocating bounded Mpsc Queues
2020-06-25 21:55:34 +02:00
Norman Maurer
b2726c4919 X509TrustManager with OPENSSL provider is not wrapped with hostname verification if Conscrypt is inserted in the first place (#10375)
Motivation:

Modifications:

Directly specify the provider which is used to create the SSLContext

Result:

Fixes https://github.com/netty/netty/issues/10374
2020-06-25 20:40:49 +02:00
Norman Maurer
163c2fc220 Ensure we feed all data to the SSLEngine during handshaking in our tests (#10373)
Motivation:

Due a bug in our test we may dropped data on the floor which are generated during handshaking (or slightly after). This could lead to corrupt state in the engine itself and so fail tests. This is especially true for TLS1.3 which generates the sessions on the server after the "actual handshake" is done.

Modifications:

Contine with wrap / unwrap until all data was consumed

Result:

Correctly feed all data to the engine during testing
2020-06-25 14:56:54 +02:00
Norman Maurer
f051b0c297 Ensure ApplicationProtocolNegotiationHandler does handle handshake fa… (#10363)
Motivation:

When ApplicationProtocolNegotiationHandler is in the pipeline we should expect that its handshakeFailure(...) method will be able to completly handle the handshake error. At the moment this is not the case as it only handled SslHandshakeCompletionEvent but not the exceptionCaught(...) that is also triggered in this case

Modifications:

- Call handshakeFailure(...) in exceptionCaught and so fix double notification.
- Add testcases

Result:

Fixes https://github.com/netty/netty/issues/10342
2020-06-24 08:47:31 +02:00
feijermu
0185ccc157 Fix a javadoc mistake. (#10364)
Motivation:

There exists a `javadoc` mistake in `HttpHeaderValues.java`.

Modification:

Just correct this `javadoc` mistake...
2020-06-23 09:24:48 +02:00
Norman Maurer
4dd6f82624 Fix memory leak in AbstractDiskHttpData when CompositeByteBuf is used (#10360)
Motivation:

AbstractDiskHttpData may cause a memory leak when a CompositeByteBuf is used. This happened because we may call copy() but actually never release the newly created ByteBuf.

Modifications:

- Remove copy() call and just use ByteBuf.getBytes(...) which will internally handle the writing to the FileChannel without any extra copies that need to be released later on.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10354
2020-06-22 13:55:02 +02:00
离诌
26d6eda0de version: remove Duplicating managed version (#10329)
Motivation:

remove Duplicating managed version, cause it is already defined in the parent project.

Modification:

- origin 
```
<dependency>
      <groupId>io.netty</groupId>
      <artifactId>netty-dev-tools</artifactId>
      <scope>test</scope>
      <version>${project.version}</version>
      <optional>true</optional>
</dependency>

<plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>build-helper-maven-plugin</artifactId>
        <version>1.10</version>
</plugin>
```

- after modify

```
<dependency>
      <groupId>io.netty</groupId>
      <artifactId>netty-dev-tools</artifactId>
      <scope>test</scope>
      <optional>true</optional>
</dependency>

<plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>build-helper-maven-plugin</artifactId>
</plugin>
```

Result:

remove Duplicating managed version
2020-06-12 11:22:57 +02:00
Norman Maurer
e76fc0a577 Fix compilation error in test due bad cherry-pick
Motivation:

I did not correctly adjust the code before cherry-pick, causing a compilation error in the test

Modifications:

Use ChannelHandler

Result:

No more compilation error
2020-06-12 11:21:47 +02:00
Bennett Lynch
f945cfbd66 Consolidate HttpObjectDecoder default values into constants (#10344)
Motivation

HttpObjectDecoder and its associated classes make frequent use of
default values for maxInitialLineLength, maxHeaderSize, maxChunkSize,
etc. Today, these defaults are defined in-line in constructors and
duplicated across many classes. This repetition is more prone to error
and inconsistencies.

Furthermore, due to the current lack of builder support, if a user wants
to change just one of these values (e.g., maxHeaderSize), they are also
required to know and repeat the other default values (e.g.,
maxInitialLineLength and maxChunkSize).

The primary motivation for this change is as we are considering adding
another constructor parameter (for multiple content length behavior),
appending this parameter may require some users to have prior knowledge
of the default initialBufferSize, and it would be cleaner to allow them
to reference the default constant.

Modifications

* Consolidate the HttpObjectDecoder default values into public constants
* Reference these constants where possible

Result

No functional change. Additional telescoping constructors will be easier
and safer to write. Users may have an easier experience changing single
parameters.
2020-06-12 08:44:39 +02:00
Kareem Ali
121daab927 Motivation: (#10326)
The current FLowControlHandler keeps a flag to track whether a read() call is pending.
This could lead to a scenario where you call read multiple times when the queue is empty,
and when the FlowControlHandler Queue starts getting messages, channelRead will be fired only once,
when we should've fired x many times, once for each time the handlers downstream called read().

Modifications:

Minor change to replace the boolean flag with a counter and adding a unit test for this scenario.

Result:

I used TDD, so I wrote the test, made sure it's failing, then updated the code and re-ran the test
to make sure it's successful after the changes.

Co-authored-by: Kareem Ali <kali@localhost.localdomain>
2020-06-04 19:18:23 +02:00
Andrey Mizurov
5276089867 Fix #10261 stomp can be chunked, so implement StompWebSocketFrameEncoder (#10274)
Motivation:

Current implementation `StompSubframeEncoder` can encode `StompFrame` into several separate chunks or encode separately `StompHeadersSubframe` and `StompContentSubframe`. But some client libraries (e.g. stomp.js) do not support aggregation.

Modification:

Add StompWebSocketFrameEncoder for integration between origin stomp suframe encoder and `ContinuationWebSocketFrame` to support  chunks on transport level.

Result:

Fixes #10261
2020-06-04 19:14:46 +02:00
Norman Maurer
040419be8a Fix possible StackOverflowError when try to resolve authorative names… (#10337)
Motivation:

There is a possibility to end up with a StackOverflowError when trying to resolve authorative nameservers because of incorrect wrapping the AuthoritativeDnsServerCache.

Modifications:

Ensure we don't end up with an overflow due wrapping

Result:

Fixes https://github.com/netty/netty/issues/10246
2020-06-04 19:14:29 +02:00
Norman Maurer
feeb3ee920 Update test to directly check for SslHandshakeTimeoutException (#10339)
Motivation:

9b7e091 added a special SSLHandshakeException sub-class to signal handshake timeouts but we missed to update a testcase to directly assert the type of the exception.

Modifications:

Assert directly that SslHandshakeTimeoutException is used

Result:

Test cleanup
2020-06-04 18:30:25 +02:00
feijermu
8bbd89d72a Fix a test case problem: testSwallowedReadComplete(...) may fail with an AssertionError sometimes. (#10313)
Motivation:

It seems that `testSwallowedReadComplete(...)` may fail with an AssertionError sometimes after my tests. The relevant stack trace is as follows:

```
java.lang.AssertionError: expected:<IdleStateEvent(READER_IDLE, first)> but was:<null>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:144)
	at io.netty.handler.flow.FlowControlHandlerTest.testSwallowedReadComplete(FlowControlHandlerTest.java:478)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:542)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:770)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:464)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)

```
Obviously the `readerIdleTime` of `IdleStateHandler` and the thread sleep time before `EmbeddedChannel.runPendingTasks` are both 100ms. And if `userEvents.poll()` happened before `userEvents.add(...)` or no `IdleStateEvent` fired at all, this test case would fail.

Modification:

Sleep for a little more time before running all pending tasks in the `EmbeddedChannel`.

Result:

Fix the problem of small probability of failure.
2020-06-03 09:19:54 +02:00
Lin Gao
4b6ceb2ca0 More values other than chunked defined in Transfer-Encoding header leads to decode failure (#10321)
Motivation:

`containsValue()` will check if there are multiple values defined in the specific header name, we need to use this method instead of `contains()` for the `Transfer-Encoding` header to cover the case that multiple values defined, like: `Transfer-Encoding: gzip, chunked`

Modification:

Change from `contains()` to `containsValue()` in `HttpUtil.isTransferEncodingChunked()` method.

Result:

Fixes #10320
2020-06-02 14:38:11 +02:00
Andrey Mizurov
4c7e017199 Set (and override) reserved websocket handshake response headers after custom to avoid duplication (#10319)
Motivation:
Currently we passing custom websocket handshaker response headers to a `WebSocketServerHandshaker` but they can contain a reserved headers (e.g. Connection, Upgrade, Sec-Websocket-Accept) what lead to duplication because we use response.headers().add(..) instead of response.headers().set(..).

Modification:
In each `WebSocketServerHandshaker00`, ... `WebSocketServerHandshaker13` implementation replace the method add(..) to set(..) for reserved response headers.

Result:

Less error-prone
2020-06-02 11:56:47 +02:00
Josef Grieb
311feaa20d Update GraalVM with JDK 8 and add GraalVM with JDK 11 (#10333)
Motivation:

A new GraalVM with JDK 11 was released and GraalVM adds Java 11 support

Modification:

- Update GraalVM JDK 8 version
- Add GraalVM JDK 11 support

Result:

Build with GraalVM JDK 11 and use latest GraalVM JDK 8 version
2020-06-02 11:53:18 +02:00
Norman Maurer
4dfe541167 Include more details if we throw an IllegalArgumentException because of overflow (#10330)
Motivation:

We should include as much details as possible when throwing an IllegalArgumentException because of overflow in CompositeByteBuf

Modifications:

Add more details and factor out check into a static method to share code

Result:

Make it more clear why an operations failed
2020-06-02 10:08:06 +02:00