Commit Graph

10506 Commits

Author SHA1 Message Date
Scott Mitchell
d0fea48604
ReferenceCountedOpenSslEngine unwrap handshake complete status fix (#11210)
Motivation:
ReferenceCountedOpenSslEngine may unwrap data and complete the handshake
in a single unwrap() call. However it may return HanshakeStatus of
HandshakeStatus of NEED_UNWRAP instead of FINISHED. This may result in
the SslHandler sending the unwrapped data up the pipeline before
notifying that the handshake has completed, and result in out-of-order
events.

Modifications:
- if ReferenceCountedOpenSslEngine handshake status is NEED_UNWRAP and
  produced data, or NEED_WRAP and consumed some data, we should call
  handshake() to get the current state.

Result:
ReferenceCountedOpenSslEngine correctly indicates when the handshake has
finished if at the same time data was produced or consumed.
2021-04-29 09:56:01 +02:00
skyguard1
c32c520edd
Before throwing TooLongFrameException,should skip the bytes to be read in MqttDecoder (#11204)
Motivation:

Before throwing TooLongFrameException, should call the skipBytes method to skip the bytes to be read

Modification:
- skip bytes before throw

Result:
Actually skip the bytes when we detect too much data

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-29 08:13:11 +02:00
skyguard1
d22a35628d
Give a choice for app to extend the length limitation of clientId even in mqtt v3.1 on the server side (#11205)
Motivation:

In the mqtt v3.1 protocol, the default maximum Client Identifier length is 23.However, in (#11114), there are many cases, the server may still receive a client ID with a length greater than 23. Perhaps should consider letting the user decide whether accept client id greater than 23 on the server side

Modification:

- Allow to specify max length.

Result:

Give a choice for app to extend the length limitation of clientId even in mqtt v3.1 on the server side.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-28 16:20:59 +02:00
Norman Maurer
def8a3f17d
Destroy HttpPostMultipartRequestDecoder if contructor throws (#11207)
Motivation:

We need to call destroy() if the constructor of HttpPostMultipartRequestDecoder throws as otherwise we may leak memory.

Modifications:

- Call destroy() if we throw
- Add unit test

Result:

No more leaks when constructor throws

Co-authored-by: Frederic Bregier <frederic.bregier@waarp.fr>
2021-04-28 12:27:12 +02:00
Norman Maurer
3657805711
Correctly throw ErrorDataDecoderException for errors while decoding (#11198)
Motivation:

We didn't correctly handle the case when no content-type header was found or if the charset was illegal and just did throw a NPE or ICE. We should in both cases throw an ErrorDataDecoderException to reflect what is documented in the javadocs.

Modifications:

- Throw correct exception
- Merge private method into the constructor as it is only used there
- Add unit tests

Result:

Throw expected exceptions on decoding errors
2021-04-27 16:37:37 +02:00
Norman Maurer
268f5a9108
Update run-on-arch-action (#11199)
Motivation:

In the past we did see problems sometime when run-on-arch-action was used. We are multiple releases behind, lets update and so maybe fix the problems.

Modifications:

Update to latest release

Result:

Use latest run-on-arch-action release
2021-04-27 13:48:51 +02:00
Norman Maurer
216cd38ff2
Re-enable running openssl (shared) tests on CI (#11197)
Motivation:

It turned out we didnt run the openssl tests on the CI when we used the non-static version of netty-tcnative.

Modifications:

- Upgrade netty-tcnative to fix segfault when using shared openssl
- Adjust tests to only run session cache tests when openssl supports it
- Fix some more tests to only depend on KeyManager if the underlying openssl version supports it

Result:

Run all openssl test on the CI even when shared library is used
2021-04-27 13:38:08 +02:00
Norman Maurer
48e2c0d5cd
Be more lenient when check the number of registered channels (#11194)
Motivation:

Some SingleEventLoop implementations may need some "time" to update the number of registered Channels. We should loop until we see the correct value or timeout if we did not observe the correct value.

Modifications:

- Loop until we see the correct value
- add test timeout

Result:

Fixes https://github.com/netty/netty/issues/11169
2021-04-26 11:31:09 +02:00
roy
c1b922b224
adjust validation logic when websocket server check starts with '/' (#11191)
Motivation:

When create a WebSocketServerProtocolConfig to check URI path starts from '/',
only '/' or '//subPath' can be passed by the checker,but '/subPath' should be
passed as well

Modifications:

in `WebSocketServerProtocolHandshakeHandler.isWebSocketPath()` treat '/' a special case

Result:
'/subPath' can be passed
2021-04-26 10:32:10 +02:00
skyguard1
a98c60283b
Enable Tlsv1.3 when using BouncyCastle ALPN support (#11193)
Motivation:

In the latest version of BouncyCastle, BCJSSE:'TLSv1.3' is now a supported protocol for both client and server. So should consider enabling TLSv1.3 when TLSv1.3 is available

Modification:

This pr is to enable TLSv1.3 when using BouncyCastle ALPN support, please review this pr,thanks

Result:

Enable TLSv1.3 when using BouncyCastle ALPN support

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-26 10:01:16 +02:00
Trustin Lee
183559ddb6
Add a new HTTP/2 pseudo header :protocol (#11192)
Motivation:

RFC 8411 defines a new HTTP/2 pseudo header called `:protocol`:

- https://datatracker.ietf.org/doc/rfc8441/

Netty currently raises an exception when validating an `Http2Headers`.

Modifications:

- Added `Http2Headers.PseudoHeaderNames.PROTOCOL` so that `:protocol`
  pseudo header is not rejected.

Result:

- A user can implement WebSockets with HTTP/2.
2021-04-26 09:28:35 +02:00
Boris Unckel
db2f60c870
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (codec-http) (#11170) (#11187)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-23 08:08:28 +02:00
Boris Unckel
4eafccf971
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (misc) (#11170) (#11186)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved in microbench and resolver-dns

Fixes #11170
2021-04-22 17:44:58 +02:00
Boris Unckel
d3fa33058d
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (codec*) (#11170) (#11185)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.
Fixes #11170
2021-04-22 16:01:28 +02:00
Boris Unckel
52ca64033a
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (example) (#11170) (#11183)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 15:03:13 +02:00
Boris Unckel
d5fabe78a7
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (buffer) (#11170) (#11182)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 14:20:44 +02:00
Boris Unckel
c3416d8ad2
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (transport*) (#11170) (#11181)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 14:13:14 +02:00
Boris Unckel
2f4beae8ec
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (handler) (#11170) (#11180)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 12:51:23 +02:00
Scott Mitchell
75c1134c0d
SimpleChannelPromiseAggregator use first exception instead of last (#11168)
Motivation:
SimpleChannelPromiseAggregator implements the promise API and allows for
multiple operations to share a common promise. It currently propagates
the last exception to occur, but this may mask the original exception
which lead to the last exception and make debugging more difficult.

Modifications:
- SimpleChannelPromiseAggregator propagates the first exception instead
  of the last exception.

Result:
Fixes https://github.com/netty/netty/issues/11161.
2021-04-22 12:17:47 +02:00
Boris Unckel
acd9b383bc
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (codec) (#11170) (#11179)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-22 10:57:40 +02:00
Norman Maurer
36dcaf0c50 Fix typo in exception message 2021-04-22 08:22:37 +02:00
skyguard1
1d802d9da5
Add default block to CompositeByteBuf (#11178)
Motivation:

Switch statements should always have a default block to ensure we not "fall-through" by mistake.

Modification:

Add default block

Result:

code cleanup.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-22 08:21:20 +02:00
Boris Unckel
2c757d11be
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (commons) (#11170) (#11172)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* Add some checks to ObjectUtil not present today but utilized in the code.
* Add unit test for ObjectUtil
* Update commmons internal usage with ObjectUtil

Result:

All checks needed are present, subsequent changes of usage of ObjectUtil are possible.

Fixes for https://github.com/netty/netty/issues/11170
2021-04-22 08:03:24 +02:00
Karsten Ohme
56ef371676
BouncyCastle ALPN support (#11157)
Motivation:

Under Android it was not possible to load a specific web page. It might be related to the (missing?) ALPN of the internal TLS implementation. BouncyCastle as a replacement works but this was not supported so far by Netty.
BouncyCastle also has the benefit to be a pure Java solution, all the other providers (OpenSSL, Conscrypt) require native libraries which are not available under Android at least.

Modification:

BouncyCastleAlpnSslEngine.java and support classes have been added. It is relying on the JDK code, hence some support classes had to be opened to prevent code duplication.

Result:

BouncyCastle can be used as TLS provider.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-04-21 16:18:34 +02:00
Liyuan
f7795c81a5 Remove duplicate HTTP2 payload length check against max frame size
Motivation:

In the method processHeaderState(), we have checked the http2 payload length against max frame size. But later for
different types of frames, we checked this again.

Modifications:

Removed the duplicate check in verify*() methods. And removed verifyPayloadLength() method, since it will not be used anymore.

Result:

Remove duplicate check and make the code cleaner.
2021-04-21 16:13:07 +02:00
Liyuan
8d7ffec081 Used already calculated SETTINGS frame payload length when allocating ByteBuf
Motivation:  We have already calculated the payload length. So no need to calculate again when allocating ByteBuf

Modification:  Used payloadLength variable instead of calculating the payload length again

Result:  Re-use the variable value and make the code cleaner
2021-04-21 16:13:07 +02:00
Norman Maurer
e27831d767
Fix support for IP_RECVORIGDSTADDR when using native epoll transport (#11173)
Motivation:

While adding support for GRO (b05fdf3ff8) we broke support for IP_RECVORIGDSTADDR when using the native transport. Beside this we also didnt correctly handle IP_RECVORIGDSTADDR when recvmmsg was used.

Modifications:

- Fix support for IP_RECVORIGDSTADDR when using the native epoll transport for normal reads (recvmsg) but also for scattering reads (recvmmsg)
- Remove code from unix code-base as the support is linux specific and we not need the code there anymore

Result:

Fixes https://github.com/netty/netty/issues/11141
2021-04-21 16:04:12 +02:00
Norman Maurer
697952e3e6
Add WebSocketClientHandshaker / WebSocketServerHandshaker close methods that take ChannelHandlerContext as parameter (#11171)
Motivation:

At the moment we only expose close(...) methods that take a Channel as paramater. This can be problematic as the write will start at the end of the pipeline which may contain ChannelOutboundHandler implementations that not expect WebSocketFrame objects. We should better also support to pass in a ChannelHandlerContext as starting point for the write which ensures that the WebSocketFrame objects will be handled correctly from this position of the pipeline.

Modifications:

- Add new close(...) methods that take a ChannelHandlerContext
- Add javadoc sentence to point users to the new methods.

Result:

Be able to "start" the close at the right position in the pipeline.
2021-04-21 15:16:10 +02:00
Violeta Georgieva
d342124390
Ensure DnsNameResolver resolves the host(computer) name on Windows (#11167)
Motivation:

On Windows DnsNameResolver is not able to resolve the host(computer) name as it is not in the hosts file and the DNS server is also not able to resolve it.
The exception below is the result of the resolution:
Caused by: java.net.UnknownHostException: failed to resolve 'host(computer)-name' after 2 queries
	at io.netty.resolver.dns.DnsResolveContext.finishResolve(DnsResolveContext.java:1013)
	at io.netty.resolver.dns.DnsResolveContext.tryToFinishResolve(DnsResolveContext.java:966)
	at io.netty.resolver.dns.DnsResolveContext.query(DnsResolveContext.java:414)
	at io.netty.resolver.dns.DnsResolveContext.tryToFinishResolve(DnsResolveContext.java:938)
	at io.netty.resolver.dns.DnsResolveContext.access$700(DnsResolveContext.java:63)
	at io.netty.resolver.dns.DnsResolveContext$2.operationComplete(DnsResolveContext.java:467)

Modifications:

On Windows DnsNameResolver maps host(computer) name to LOCALHOST

Result:

DnsNameResolver is able to resolve the host(computer) name on Windows

Fixes #11142
2021-04-20 08:18:32 +02:00
ZHANG Dapeng
e2daae9ac8
Fix StreamBufferingEncoder GOAWAY bug (#11144)
Motivation:

There is a bug in `StreamBufferingEncoder` such that when client receives GOWAY while there are pending streams due to MAX_CONCURRENT_STREAMS, we see the following error:
```
io.netty.handler.codec.http2.Http2Exception$StreamException: Maximum active streams violated for this endpoint.
        at io.netty.handler.codec.http2.Http2Exception.streamError(Http2Exception.java:147)
        at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.checkNewStreamAllowed(DefaultHttp2Connection.java:896)
        at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.createStream(DefaultHttp2Connection.java:748)
        at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.createStream(DefaultHttp2Connection.java:668)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.writeHeaders0(DefaultHttp2ConnectionEncoder.java:201)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.writeHeaders(DefaultHttp2ConnectionEncoder.java:167)
        at io.netty.handler.codec.http2.DecoratingHttp2FrameWriter.writeHeaders(DecoratingHttp2FrameWriter.java:53)
        at io.netty.handler.codec.http2.StreamBufferingEncoder.writeHeaders(StreamBufferingEncoder.java:153)
        at io.netty.handler.codec.http2.StreamBufferingEncoder.writeHeaders(StreamBufferingEncoder.java:141)
        at io.grpc.netty.NettyClientHandler.createStreamTraced(NettyClientHandler.java:584) 
        at io.grpc.netty.NettyClientHandler.createStream(NettyClientHandler.java:567)
        at io.grpc.netty.NettyClientHandler.write(NettyClientHandler.java:328)
        at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:717)
        at io.netty.channel.AbstractChannelHandlerContext.invokeWrite(AbstractChannelHandlerContext.java:709)
        at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:792)
        at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:702)
        at io.netty.channel.DefaultChannelPipeline.write(DefaultChannelPipeline.java:1015)
        at io.netty.channel.AbstractChannel.write(AbstractChannel.java:289)
        at io.grpc.netty.WriteQueue$AbstractQueuedCommand.run(WriteQueue.java:213)
        at io.grpc.netty.WriteQueue.flush(WriteQueue.java:128)
        at io.grpc.netty.WriteQueue.drainNow(WriteQueue.java:114)
        at io.grpc.netty.NettyClientHandler.goingAway(NettyClientHandler.java:783)
        at io.grpc.netty.NettyClientHandler.access$300(NettyClientHandler.java:91)
        at io.grpc.netty.NettyClientHandler$3.onGoAwayReceived(NettyClientHandler.java:280)
        at io.netty.handler.codec.http2.DefaultHttp2Connection.goAwayReceived(DefaultHttp2Connection.java:236)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder.onGoAwayRead0(DefaultHttp2ConnectionDecoder.java:218)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onGoAwayRead(DefaultHttp2ConnectionDecoder.java:551)
        at io.netty.handler.codec.http2.Http2InboundFrameLogger$1.onGoAwayRead(Http2InboundFrameLogger.java:119)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readGoAwayFrame(DefaultHttp2FrameReader.java:591)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.processPayloadState(DefaultHttp2FrameReader.java:272)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readFrame(DefaultHttp2FrameReader.java:160)
        at io.netty.handler.codec.http2.Http2InboundFrameLogger.readFrame(Http2InboundFrameLogger.java:41)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder.decodeFrame(DefaultHttp2ConnectionDecoder.java:174)
        at io.netty.handler.codec.http2.Http2ConnectionHandler$FrameDecoder.decode(Http2ConnectionHandler.java:378)
        at io.netty.handler.codec.http2.Http2ConnectionHandler.decode(Http2ConnectionHandler.java:438)
        at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:498)
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:437)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1486)
        at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1235)
        at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1282)
        at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:498)
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:437)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
        at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:792)
        at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:475)
        at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:378)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Unknown Source)
```

The bug should come from the way that `StreamBufferingEncoder.writeHeaders()` handles the condition `connection().goAwayReceived()`. The current behavior is to delegate to `super.writeHeaders()` and let the stream fail, but this will end up with `Http2Exception` with the message "Maximum active streams violated for this endpoint" which is horrible. See e5951d46fc/codec-http2/src/main/java/io/netty/handler/codec/http2/StreamBufferingEncoder.java (L152-L155)

Modification:

Abort new stream immediately if goaway received *and* MAX_CONCURRENT_STREAM reached in `StreamBufferingEncoder` rather than delegating to the `writeHeaders()` method of its super class.

Result:

In the situation when GOAWAY received as well as MAX_CONCURRENT_STREAM exceeded, the client will fail the buffered streams with `Http2Error.NO_ERROR` and message "GOAWAY received" instead of "Maximum active streams violated for this endpoint".

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-04-19 16:05:53 +02:00
Dmitriy Dumanskiy
3b89ac7cf0
Use ThreadLocalRandom instead of Math.random() (#11165)
Motivation:

`ThreadLocalRandom` doesn't cause contention. Also `nextInt()` generates only 4 random bytes while `Math.random()` generates 8 bytes.

Modification:

Replaced `(int) Math.random()` with `PlatformDependent.threadLocalRandom().nextInt()`

Result:

No possible contention when random numbers for WebSockets.
2021-04-19 13:56:16 +02:00
skyguard1
976486f021
[Clean] Remove useless code (#11154)
Motivation:
There is an unused field

Modifications:
Remove useless code

Result:
Code cleanup

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-19 10:09:40 +02:00
Chris Vest
1a5ebfb999
Log fewer stack traces from initialisation code (#11164)
Motivation:
We are increasingly running in environments where Unsafe, setAccessible, etc. are not available.
When debug logging is enabled, we log a complete stack trace every time one of these initialisations fail.
Seeing these stack traces can cause people unnecessary concern.
For instance, people might have alerts that are triggered by a stack trace showing up in logs, regardless of its log level.

Modification:
We continue to print debug log messages on the result of our initialisations, but now we only include the full stack trace is _trace_ logging (or FINEST, or equivalent in whatever logging framework is configured) is enabled.

Result:
We now only log these initialisation stack traces when the lowest possible log level is enabled.

Fixes #7817
2021-04-19 08:53:08 +02:00
Scott Mitchell
0ef2f05373
SslHandler LocalChannel read/unwrap reentry fix (#11156)
Motivation:
SslHandler invokes channel.read() during the handshake process. For some
channel implementations (e.g. LocalChannel) this may result in re-entry
conditions into unwrap. Unwrap currently defers updating the input
buffer indexes until the unwrap method returns to avoid intermediate
updates if not necessary, but this may result in unwrapping the same
contents multiple times which leads to handshake failures [1][2].

[1] ssl3_get_record:decryption failed or bad record mac
[2] ssl3_read_bytes:sslv3 alert bad record mac

Modifications:
- SslHandler#unwrap updates buffer indexes on each iteration so that if
  reentry scenario happens the correct indexes will be visible.

Result:
Fixes https://github.com/netty/netty/issues/11146
2021-04-16 08:21:40 -07:00
Frédéric Brégier
93f021141d
Fix behavior of HttpPostMultipartRequestDecoder for Memory based Factory (#11145)
Motivation:
When Memory based Factory is used, if the first chunk starts with Line Break, the HttpData
is not filled with the current available buffer if the delimiter is not found yet, while it may
add some.

Fix JavaDoc to note potential wrong usage of content() or getByteBuf() if HttpDatais has
a huge content with the risk of Out Of Memory Exception.

Fix JavaDoc to explain how to release properly the Factory, whatever it is in Memory,
Disk or Mixed mode.

Fix issue #11143

Modifications:
First, when the delimiter is not found, instead of searching Line Break from readerIndex(), we should search
from readerIndex() + readableBytes() - delimiter size, since this is the only part where usefull
Line Break could be searched for, except if readableBytes is less than delimiter size (then we search from
readerIndex).

Second, when a Memory HttpData is created, it should be assigned an empty buffer to be
consistent with the other implementations (Disk or Mixed mode).
We cannot change the default behavior of the content() or getByteBuf() of the Memory based HttpData
since the ByteBuf is supposed to be null when released, but not empty.
When a new ByteBuf is added, one more check verifies if the current ByteBuf is empty, and if so, it
is released and replaced by the new one, without creating a new CompositeByteBuf.

Result:
In the tests testBIgFileUploadDelimiterInMiddleChunkDecoderMemoryFactory and related for other modes,
the buffers are starting with a CRLF.
When we offer only the prefix part of the multipart (no data at all), the current Partial HttpData has
an empty buffer.
The first time we offer the data starting with CRLF to the decoder, it now
has a correct current Partial HttpData with a buffer not empty.

The Benchmark was re-run against this new version.

Old Benchmark                                                                       Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  4,037 ± 0,358  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  4,226 ± 0,471  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,875 ± 0,029  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  4,346 ± 0,275  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,044 ± 0,020  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  2,278 ± 0,159  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,174 ± 0,004  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,370 ± 0,065  ops/ms

New Benchmark                                                                       Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  5,604 ± 0,415  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  6,058 ± 0,111  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,914 ± 0,031  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  6,053 ± 0,051  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,636 ± 0,141  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  3,033 ± 0,181  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,178 ± 0,006  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,859 ± 0,189  ops/ms

So +20 to +40% improvement due to not searching for CRLF/LF into the full buffer when no delimiter is found,
but only from the end and delimiter size + 2 (CRLF).
2021-04-16 11:01:15 +02:00
Scott Mitchell
16b40d8a37
SslHandler consolidate state to save memory (#11160)
Motivation:
SslHandler has many independent boolean member variables. They can be
collapsed into a single variable to save memory.

Modifications:
- SslHandler boolean state consolidated into a single short variable.

Result:
Savings of 8 bytes per SslHandler (which is per connection) observed on
OpenJDK.
2021-04-15 08:18:54 -07:00
Idel Pivnitskiy
c93419ba57
Fire SslHandshakeCompletionEvent after the last decoded data chunk (#11148)
Motivation:

`SslHandler#unwrap` may produce `SslHandshakeCompletionEvent` if it
receives `close_notify` alert. This alert indicates that the engine is
closed and no more data are expected in the pipeline. However, it fires
the event before the last data chunk. As the result, further handlers
may loose data if they handle `SslHandshakeCompletionEvent`.
This issue was not visible before #11133 because we did not write
`close_notify` alert reliably.

Modifications:

- Add tests to reproduce described behavior;
- Move `notifyClosePromise` after fire of the last `decodeOut`;

Result:

`SslHandshakeCompletionEvent` correctly indicates that the engine is
closed and no more data are expected on the pipeline.
2021-04-15 14:06:42 +02:00
Chris Vest
df53de5b68
Less blocking in ChunkedStream (#11150)
Motivation:
We should avoid blocking in the event loop as much as possible.
The InputStream.read() is a blocking method, and we don't need to call it if available() returns a positive number.

Modification:
Bypass calling InputStream.read() if available() returns a positive number.

Result:
Fewer blocking calls in the event loop, in general, when ChunkedStream is used.
2021-04-12 13:40:14 +02:00
Netty Project Bot
6724786dcc [maven-release-plugin] prepare for next development iteration 2021-04-01 10:50:23 +00:00
Netty Project Bot
b78d8f2abd [maven-release-plugin] prepare release netty-4.1.63.Final 2021-04-01 10:50:18 +00:00
Scott Mitchell
6b48e690fd
SslHandler wrap reentry bug fix (#11133)
Motivation:
SslHandler's wrap method notifies the handshakeFuture and sends a
SslHandshakeCompletionEvent user event down the pipeline before writing
the plaintext that has just been wrapped. It is possible the application
may write as a result of these events and re-enter into wrap to write
more data. This will result in out of sequence data and result in alerts
such as SSLV3_ALERT_BAD_RECORD_MAC.

Modifications:
- SslHandler wrap should write any pending data before notifying
  promises, generating user events, or anything else that may create a
  re-entry scenario.

Result:
SslHandler will wrap/write data in the same order.
2021-04-01 10:00:18 +02:00
Netty Project Bot
e4dd6ee532 [maven-release-plugin] prepare for next development iteration 2021-03-30 17:22:25 +00:00
Netty Project Bot
573d1f4bf5 [maven-release-plugin] prepare release netty-4.1.62.Final 2021-03-30 17:22:18 +00:00
Violeta Georgieva
41ef4e4b02
AbstractKQueueChannel#writeFilter is invoked with the correct boolean depending on the ChannelOutboundBuffer state (#11128)
Motivation:

This is a regression caused by #11086

Modifications:

AbstractKQueueChannel#writeFilter should be invoked with `!in.isEmpty()`
- false - all messages are written
- true - there are still messages to be written

Result:

AbstractKQueueChannel#writeFilter is invoked with the correct boolean depending on the ChannelOutboundBuffer state
2021-03-30 19:16:03 +02:00
Norman Maurer
ff0c014ade
Skip deployment of testsuite jars (#11127)
Motivation:

We should skip the deployment of jars that are not meant to be consumed by the user as there is no public API.

Modifications:

Let's skip deployment for modules that are not useful for users

Result:

Build cleanup
2021-03-30 19:06:08 +02:00
Norman Maurer
3706b73d92 Also build modules we depend on when finish the release
Motivation:

We also need to compile the modules we depend on when finish up the release otherwise it may fail.

Modifications:

Add "-am" to the mvn args

Result:

Correctly also compile the modules we depend on
2021-03-30 12:03:14 +02:00
Netty Project Bot
7192ab0f0a [maven-release-plugin] prepare for next development iteration 2021-03-30 07:42:06 +00:00
Netty Project Bot
ccb3ff388f [maven-release-plugin] prepare release netty-4.1.61.Final 2021-03-30 07:42:01 +00:00
Norman Maurer
b0fa4d5aab
Merge pull request from GHSA-f256-j965-7f32
Motivation:

We also need to ensure that all the header validation is done when a single header with the endStream flag is received

Modifications:

- Adjust code to always enforce the validation
- Add more unit tests

Result:

Always correctly validate
2021-03-30 09:40:47 +02:00
Norman Maurer
5867de70d7
Update to netty-tcnative 2.0.38.Final (#11126) 2021-03-29 19:48:16 +02:00