Commit Graph

9961 Commits

Author SHA1 Message Date
Norman Maurer
fc656f605f Only set the keymaterial once and correctly handle errors during keymaterial setting on the client-side as well (#10613)
Motivation:

We should stop as soon as we were able to set the key material on the server side as otherwise we may select keymaterial that "belongs" to a less prefered cipher. Beside this it also is just useless work.
We also need to propagate the exception when it happens during key material selection on the client side so openssl will produce the right alert.

Modifications:

- Stop once we were able to select a key material on the server side
- Ensure we not call choose*Alias more often then needed
- Propagate exceptions during selection of the keymaterial on the client side.

Result:

Less overhead and more correct behaviour
2020-09-29 09:28:25 +02:00
Norman Maurer
ced5faa440 Correctly report back when we fail to select the key material and ens… (#10610)
Motivation:

We need to let openssl know that we failed to find the key material so it will produce an alert for the remote peer to consume. Beside this we also need to ensure we wrap(...) until we produced everything as otherwise the remote peer may see partial data when an alert is produced in multiple steps.

Modifications:

- Correctly throw if we could not find the keymaterial
- wrap until we produced everything
- Add test

Result:

Correctly handle the case when key material could not be found
2020-09-29 09:21:35 +02:00
Norman Maurer
d92d92a923 Filter out duplicates before trying to find the alias to use (#10612)
Motivation:

Calling chooseServerAlias(...) may be expensive so we should ensure we not call it multiple times for the same auth methods.

Modifications:

Remove duplicated from authMethods before trying to call chooseServerAlias(...)

Result:

Less performance overhead during key material selection
2020-09-25 19:13:33 +02:00
Yosfik Alqadri
c9e42106d1 Fix Javadoc Typo (#10603)
Motivation:

Following Javadoc standard

Modification:

Change from `@param KeyManager` to `@param keyManager`

Result:

The `@param` matches the actual parameter variable name
2020-09-24 15:28:30 +02:00
Norman Maurer
1663eb5965 Increase initial buffer size in AdaptiveRecvByteBufAllocator (#10600)
Motivation:

We should use an initial buffer size with is >= 1500 (which is a common setting for MTU) to reduce the need for memory copies when a new connection is established. This is especially interesting when SSL / TLS comes into the mix.

This was ported from swiftnio:

https://github.com/apple/swift-nio/pull/1641

Modifications:

Increase the initial size from 1024 to 2048.

Result:

Possible less memory copies on new connections
2020-09-22 17:27:45 +02:00
Paul Lysak
2a87b00984 Not truncate MQTT SUBACK reason codes (#10584)
Motivation:

Reason code for MQTT SUBACK messages is truncated, thus not allowing to get new codes supported by MQTT v.5

Modification:

Changed `MqttCodecTest.testUnsubAckMessageForMqtt5` to catch it then, removed truncation in `MqttDecoder.decodeSubackPayload` to make it pass.

Result:

Codec users can get all reason codes supported by MQTT5 now.
2020-09-22 09:04:45 +02:00
Norman Maurer
5bb535e78a Only create ConnectTimeoutException if really needed (#10595)
Motivation:

Creating exceptions is expensive so we should only do so if really needed.

Modifications:

Only create the ConnectTimeoutException if we really need it.

Result:

Less overhead
2020-09-21 21:40:25 +02:00
Divij Vaidya
58911c1482 Clear scheduled timeout if channel is closed with incomplete WebSocket handshake (#10510)
Motivation:

Consider a scenario when the client iniitiates a WebSocket handshake but before the handshake is complete,
the channel is closed due to some reason. In such scenario, the handshake timeout scheduled on the executor
is not cleared. The reason it is not cleared is because in such cases the handshakePromise is not completed.

Modifications:

This change completes the handshakePromise exceptinoally on channelInactive callback, if it has not been
completed so far. This triggers the callback on completion of the promise which clears the timeout scheduled
on the executor.

This PR also adds a test case which reproduces the scenario described above. The test case fails before the
fix is added and succeeds when the fix is applied.

Result:

After this change, the timeout scheduled on the executor will be cleared, thus freeing up thread resources.
2020-09-16 10:10:50 +02:00
Aayush Atharva
7d985d26b4 Call long as l instead of i (#10578)
Motivation:

Long should be called `l` instead of `i` because `i` is generally used for `int`.

Modification:

Changed `i` to `l`.

Result:
Better naming
2020-09-16 07:58:23 +02:00
Paul Lysak
2fca020be5 Make MQTT property value publicly accessible (#10577)
Motivation:

There was no way to read MQTT properties outside of `io.netty.handler.codec.mqtt` package

Modification:

Expose `MqttProperties.MqttProperty` fields `value` and `propertyId` via public methods

Result:

It's possible to read MQTT properties now
2020-09-16 07:56:31 +02:00
Kevin Wu
eef67e2f0e Fix DeleteOnExitHook cause memory leak (#10560)
Motivation:

If DeleteOnExitHook is in the open state and the program runs for a long time, the DeleteOnExitHook file keeps increasing.
This results in a memory leak

Modification:

I re-customized a DeleteOnExitHook hook. If DeleteOnExitHook is turned on, this hook will be added when creating a temporary file. After the request ends and the corresponding resources are released, the current file will be removed from this hook, so that it will not increase all the time.

Result:

Fixes https://github.com/netty/netty/issues/10351
2020-09-15 16:45:03 +02:00
dependabot[bot]
cc3a686357 Bump ant from 1.9.7 to 1.9.15 (#10573)
Bumps ant from 1.9.7 to 1.9.15.

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2020-09-15 15:31:15 +02:00
Paul Lysak
575c2d16de Fix NPE with MqttUnsubAckMessage - regression of MQTT5 support (#10557)
Recent changes for MQTT5 may cause NPE if UNSUBACK message is created with `MqttMessageFactory`
and client code isn't up-to-date.

Modifications:

Added default body in `MqttUnsubAckMessage` constructor if null body is passed,
added null checks in `encodeUnsubAckMessage`

Result:

`MqttUnsubAckMessage` created with `MqttMessageFactory` doesn't cause NPE
even if null body is supplied.
2020-09-15 15:14:45 +02:00
Aayush Atharva
379be5085f Add PcapWriteHandler Support (#10498)
Motivation:
Write TCP and UDP packets into Pcap `OutputStream` which helps a lot in debugging.

Modification:
Added TCP and UDP Pcap writer.

Result:
New handler can write packets into an `OutputStream`, e.g. a file that can be opened with Wireshark.

Fixes #10385.
2020-09-11 16:15:39 +02:00
Paul Lysak
dc66e8d67f Make RetainedHandlingPolicy enum public (#10565)
Motivation:

Impossible to specify retained messages handling policy because the corresponding enum
isn't public (https://github.com/netty/netty/issues/10562)

Modification:

Made `RetainedHandlingPolicy` public

Result:

Fixes #10562.
2020-09-11 08:33:39 +02:00
Chris Vest
7f423772ba Disable Maven build HTTP connection pooling (#10558)
Motivation:
 The builds often fail when downloading dependencies.
 This might be caused by the build taking a long time, and cause pooled connections to be closed
 by the remote end, if they are idle for too long.

Modification:
 Disable connection pooling. This should force Maven to reestablish the connection for each download,
 thus reducing the likelihood of the remote end closing connections we wish to use.

Result:
 I'll leave it up the statistis of our CI to confirm, but we should see more stable builds.
2020-09-09 17:08:03 +02:00
Chris Vest
1d7efbddd9 Fix compilation after forward port of #10368
Motivation: Code failed to compile because ByteBuf index marking has been removed.
Modification: Index marking wasn't really used anyway, so just set the relevant index to zero.
Result: Code compiles again.
2020-09-09 16:27:52 +02:00
Francesco Nigro
7f86f90646 Improve predictability of writeUtf8/writeAscii performance (#10368)
Motivation:

writeUtf8 can suffer from inlining issues and/or megamorphic call-sites on the hot path due to ByteBuf hierarchy

Modifications:

Duplicate and specialize the code paths to reduce the need of polymorphic calls

Result:

Performance are more stable in user code
2020-09-09 16:15:22 +02:00
Norman Maurer
5d418e7bd9 Make kernel version detection code in EpollReuseAddrTest more robust (#10556)
Motivation:

When we try to parse the kernel version we need to be careful what to
expect. Especially when a custom kernel is used we may get extra chars
in the version numbers. For example I had this one fail because of my
custom kernel that I built for io_uring:

5.8.7ioring-fixes+

Modifications:

- Try to be a bit more lenient when parsing
- If we cant parse the kernel version just use 0.0.0

Result:

Tests are more robust
2020-09-09 15:52:57 +02:00
Norman Maurer
6c5a6dba46 Fix regression when trying to bind an EpollDatagramChannel with port (#10552)
only

Motivation:

4b7dba1 introduced a change which was not 100 % complete and so
introduce a regression when a user specified to use
InetProtocolFamily.IPv4 and trying to bind to a port (without specify
the ip).

Modifications:

- Fix regression by respect the InetProtocolFamily
- Add unit test

Result:

Fix regression when binding to port explicit
2020-09-09 15:52:40 +02:00
Francesco Nigro
319a4bc3ba Reduce garbage on MQTT (#10509)
Reduce garbage on MQTT encoding

Motivation:

MQTT encoding and decoding is doing unnecessary object allocation in a number of places:
- MqttEncoder create many byte[] to encode Strings into UTF-8 bytes
- MqttProperties uses Integer keys instead of int
- Some enums valueOf create unnecessary arrays on the hot paths
- MqttDecoder was using unecessary Result<T>

Modification:

- ByteBufUtil::utf8Bytes and ByteBufUtil::reserveAndWriteUtf8 allows to perform the same operation GC-free
- MqttProperties uses a primitive key map
- Implemented GC free const table lookup/switch valueOf
- Use some bit-tricks to pack 2 ints into a single primitive long to store both result and numberOfBytesConsumed and use byte[].length to compute numberOfByteConsumed on fly. These changes allowed to save creating Result<T>.

Result:
Significantly less garbage produced in MQTT encoding/decoding
2020-09-04 18:31:53 +02:00
Graham Edgecombe
ed6ad17caa Fix ByteBufUtil.getBytes() incorrectly sharing the array in some cases (#10529)
Motivation:

If ByteBufUtil.getBytes() is called with copy=false, it does not
correctly check that the underlying array can be shared in some cases.

In particular:

* It does not check that the arrayOffset() is zero. This causes it to
  incorrectly return the underlying array if the other conditions are
  met. The returned array will be longer than requested, with additional
  unwanted bytes at its start.

* It assumes that the capacity() of the ByteBuf is equal to the backing
  array length. This is not true for some types of ByteBuf, such as
  PooledHeapByteBuf. This causes it to incorrectly return the underlying
  array if the other conditions are met. The returned array will be
  longer than requested, with additional unwanted bytes at its end.

Modifications:

This commit fixes the two bugs by:

* Checking that the arrayOffset() is zero before returning the
  underlying array.

* Comparing the requested length to the underlying array's length,
  rather than the ByteBuf's capacity, before returning the underlying
  array.

This commit also adds a series of test cases for ByteBufUtil.getBytes().

Result:

ByteBufUtil.getBytes() now correctly checks whether the underlying array
can be shared or not.

The test cases will ensure the bug is not reintroduced in the future.
2020-09-04 13:15:56 +02:00
Norman Maurer
338b7ce314 Revert "Support session cache for client and server when using native SSLEngine implementation (#10331)"
Motivation:

This reverts commit 7bf1ffb2d4 as it turns out it introduced a big performance regression.

Modifications:

Revert 7bf1ffb2d4

Result:

Performance of TLS is back to normal
2020-09-03 08:33:22 +02:00
Maksym Ostroverkhov
a7d4c010a6 Http2ConnectionHandlerBuilder: make initialSettings() public (#10526)
Motivation:
be able to modify Http2Settings of Http2ConnectionHandlerBuilder.

Modifications:
make initialSettings() of Http2ConnectionHandlerBuilder public.

Result:
public initialSettings() of Http2ConnectionHandlerBuilder allow Http2Settings modification.
2020-09-02 21:13:14 +02:00
Norman Maurer
07632a5a4c Create a stackless ClosedChannelException to reduce overhead when the… (#10523)
Motivation:

In some benchmarks closing the Channel attributes to a lot of overhead due the call of fillInStackTrace(). We should reduce this overhead.

Modifications:

- Create a StacklessClosedChannelException and use it to reduce overhead.
- Only call ChannelOutboundBuffer.failFlushed(...) when there was a flushed message at all.

Result:

Less performance overhead when closing the Channel
2020-09-01 15:47:10 +02:00
Aayush Atharva
a49afaef35 Binary search based IpSubnetFilter (#10492)
Motivation:

`IpSubnetFilter` uses Binary Search for IP Address search which is fast if we have large set of IP addresses to filter.

Modification:

Added `IpSubnetFilter` which takes `IpSubnetFilterRule` for filtering.

Result:
Faster IP address filter.
2020-09-01 11:15:19 +02:00
Aayush Atharva
890c261759 Fix JavaDoc of RuleBasedIpFilter (#10521)
Motivation:

`RuleBasedIpFilter` had JavaDoc `{@link #channelRejected(ChannelHandlerContext, SocketAddress)}` instead of `{@link AbstractRemoteAddressFilter#channelRejected(ChannelHandlerContext, SocketAddress)}`.

Modification:
Added `AbstractRemoteAddressFilter` reference.

Result:
Fixed JavaDoc error and made documentation more clear.
2020-09-01 11:02:52 +02:00
Chris Vest
0fd525f859
Use j.u.Consumer interface instead of bespoke copy (#10520)
Motivation:
 Fix a TODO that was due since the "master" branch is baselined on at least Java 8.

Modification:
 Remove our own copy of the Consumer interface and fix usage sites to use j.u.Consumer.
 Also some cleanup.

Result:
 Cleaner code.
2020-08-31 16:42:58 +02:00
Michael Nitschinger
571e61ab36 Allow to customize how SslMasterKeyHandler is enabled (#10513)
Motivation:

Right now after a SslMasterKeyHandler is added to the pipeline,
it also needs to be enabled via a system property explicitly. In
some environments where the handler is conditionally added to
the pipeline this is redundant and a bit confusing.

Modifications:

This changeset keeps the default behavior, but allows child
implementations to tweak the way on how it detects that it
should actually handle the event when it is being raised.

Also, removed a private static that is not used in the wireshark
handler.

Result:

Child implementations can be more flexible in deciding when
and how the handler should perform its work (without changing
any of the defaults).
2020-08-31 10:35:06 +02:00
Francesco Nigro
0a8c9192e5 Improve MqttMessageType::valueOf cost (#10400)
Motivation:

MqttMessageType::valueOf has O(N) cost

Modifications:

MqttMessageType::valueOf uses a const lookup table

Result:

MqttMessageType::valueOf has O(1) cost
2020-08-31 10:32:48 +02:00
Paul Lysak
fb4a57826b MQTT5 support for netty-codec-mqtt (#10483)
Motivation:

 MQTT Specification version 5 was released over a year ago,
 netty-codec-mqtt should be changed to support it.

Modifications:

  Added more message and header types in `io.netty.handler.codec.mqtt`
  package in `netty-coded-mqtt` subproject,
  changed `MqttEncoder` and `MqttDecoder` to handle them properly,
  added attribute `NETTY_CODEC_MQTT_VERSION` to track protocol version

Result:

  `netty-codec-mqtt` supports both MQTT5 and MQTT3 now.
2020-08-31 10:31:57 +02:00
Aayush Atharva
079b15eee1 Add ipv4AddressToInt(Inet4Address) in NetUtils (#10500)
Motivation:

We're converting `Inet4Address` to `Integer` quite frequently so it's a good idea to keep that code in `NetUtils`.

Modification:

Added ipv4AddressToInt(Inet4Address) in NetUtils

Result:
Easy conversion of  `Inet4Address` to `Integer`.
2020-08-31 09:13:32 +02:00
Norman Maurer
d6a723ab9d Use all configured nameservers when using DnsNameResolver in all cases (#10503)
Motivation:

Due a change introduced in 68105b257d we incorrectly skipped the usage of nameservers in some cases.

Modifications:

Only fetch a new stream of nameserver if the hostname not matches the original hostname in the query.

Result:

Use all configured nameservers. Fixes https://github.com/netty/netty/issues/10499
2020-08-31 09:12:40 +02:00
Norman Maurer
3b23e5a360 Adjust testsuite to pass on JDK15+ (#10511)
Motivation:

JDK15 is about to be released as GA, we should ensure netty works and builds on it. SSLSession#getPeerCertificateChain() throws UnsupportedOperationException in JDK15 and later as it was deprecated before and people should use SSLSession#getPeerCertificates(). We need to account for that in our tests

Modifications:

- Catch UnsupportedOperationException in our testsuite and ignore it when on JDK15+ while rethrowing it otherwise.

Result:

Testsuite passes on JDK15+
2020-08-31 09:05:24 +02:00
Chris Vest
56f32f1f8c
Remove javassist dependency (#10514)
Motivation:
 Avoid keeping unused dependencies around.

Modification:
 Remove all references to javassist dependency, since it does not appear to be used by anything.

Result:
 One less dependency to worry about.
2020-08-31 09:02:28 +02:00
Aayush Atharva
6e223b5427 Minor typo (#10518)
Motivation:
I was working on the transport part in Netty (ofc, solving a major issue) and I found this typo so thought to fix it.

Modification:
Fixed Typo

Result:
No more confusion between `us` and `use`.
2020-08-31 08:59:57 +02:00
Nick Hill
26993b0d9c Lazily construct contained DataOutputStream in ByteBufOutputStream (#10507)
Motivation

This is used solely for the DataOutput#writeUTF8() method, which may
often not be used.

Modifications

Lazily construct the contained DataOutputStream in ByteBufOutputStream.

Result

Saves an allocation in some common cases
2020-08-28 09:23:12 +02:00
Nick Hill
d7c1407d4c Use ByteBuf#isAccessible() in more places (#10506)
Motivation

ByteBuf has an isAccessible method which was introduced as part of ref
counting optimizations but there are some places still doing
accessibility checks by accessing the volatile refCnt() directly.

Modifications

- Have PooledNonRetained(Duplicate|Sliced)ByteBuf#isAccessible() use
their refcount delegate's isAccessible() method
- Add static isAccessible(buf) and ensureAccessible(buf) methods to
ByteBufUtil
(since ByteBuf#isAccessible() is package-private)
- Adjust DefaultByteBufHolder and similar classes to use these methods
rather than access refCnt() directly

Result

- More efficient accessibility checks in more places
2020-08-28 09:22:34 +02:00
Norman Maurer
a0905073d4 Don't try to remove the task when the underlying executor fails the execution of self (#10505)
Motivation:

It makes no sense to remove the task when the underlying executor fails as we may be able to pick it up later. Beside this the used Queue doesnt support remove(...) and so will throw.

Modifications:

Remove the queue.remove(...) call

Result:

Fixes https://github.com/netty/netty/issues/10501.
2020-08-27 08:22:58 +02:00
Violeta Georgieva
a399175b52 Add validation when constructing Http2FrameLogger (#10495)
Motivation:

There should be a validation for the input arguments when constructing Http2FrameLogger

Modification:

Check that the provided arguments are not null

Result:

Proper validation when constructing Http2FrameLogger
2020-08-21 16:11:12 +02:00
Norman Maurer
9ab75505f8 Update netty-tcnative to 2.0.34.Final (#10494)
Motivation:

A new netty-tcative was released

Modifications:

Update to latest version

Result:

Use latest version
2020-08-21 14:22:33 +02:00
Andrey Mizurov
ce132b15d5 Small fix that takes into account the remainder when assigning the size (see #10453) (#10491)
Motivation:

This is small fixes for #10453 PR according @njhill and @normanmaurer conversation.

Modification:

Simple refactor and takes into account remainder when calculate size.

Result:

Behavior is correct
2020-08-21 11:47:42 +02:00
Chris Vest
6150a0e3c5
Expose a LoggingDnsQueryLifeCycleObserverFactory (#10488)
Expose a LoggingDnsQueryLifeCycleObserverFactory

Motivation:
 There is a use case for having logging in the DnsNameResolver, similar to the LoggingHandler.
 Previously, one could set `traceEnabled` on the DnsNameResolverBuilder, but this is not very configurable.
 Specifically, the log level and the logger context cannot be changed.

Modification:
 Expose a LoggingDnsQueryLifeCycleObserverFactory, that permit changing the log-level
 and logger context.

Result:
 It is now possible to get logging in the DnsNameResolver at a custom log level and logger,
 without very much effort.

Fixes #10485
2020-08-19 17:38:29 +02:00
Norman Maurer
514d349e1f Enable SSL_MODE_ENABLE_FALSE_START if jdkCompatibilityMode is false (#10407)
Motivation:

To reduce latency and RTTs we should use TLS False Start when jdkCompatibilityMode is not required and its supported

Modifications:

Use SSL_MODE_ENABLE_FALSE_START when jdkCompatibilityMode is false

Result:

Less RTTs and so lower latency when TLS False Start is supported
2020-08-18 19:01:09 +02:00
Norman Maurer
68dbc7703a Correctly limit queries done to resolve unresolved nameservers (#10478)
Motivation:

We need limit the number of queries done to resolve unresolved nameserver as otherwise we may never fail the resolve when there is a missconfigured dns server that lead to a "resolve loop".

Modifications:

- When resolve unresolved nameservers ensure we use the allowedQueries as max upper limit for queries so it will eventually fail
- Add unit tests

Result:

No more possibility to fail in a loop while resolve. This is related to https://github.com/netty/netty/issues/10420
2020-08-14 16:09:40 +02:00
Violeta Georgieva
4ce6774336 AbstractDiskHttpData#getChunk closes fileChannel only if everything w… (#10481)
Motivation:
AbstractDiskHttpData#getChunk opens and closes fileChannel every time when it is invoked,
as a result the uploaded file is corrupted. This is a regression caused by #10270.

Modifications:

- Close the fileChannel only if everything was read or an exception is thrown
- Add unit test

Result:
AbstractDiskHttpData#getChunk closes fileChannel only if everything was read or an exception is thrown
2020-08-14 11:14:50 +02:00
Norman Maurer
3c49d09eb9 Include TLSv1.3 ciphers as recommented ciphers for HTTP2 (#10480)
Motivation:

We should include TLSv1.3 ciphers as well as recommented ciphers these days for HTTP/2. That is especially true as Java supports TLSv1.3 these days out of the box

Modifications:

- Add TLSv1.3 ciphers that are recommended by mozilla as was for HTTP/2
- Add unit test

Result:

Include TLSv1.3 ciphers as well
2020-08-13 20:33:21 +02:00
Norman Maurer
cc33da49aa DnsAddressResolverGroup should respect configured EventLoop (#10479)
Motivation:

DnsAddressResolverGroup allows to be constructed with a DnsNameResolverBuilder and so should respect its configured EventLoop.

Modifications:

- Correctly respect the configured EventLoop
- Ensure there are no thread-issues by calling copy()
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/10460
2020-08-13 20:32:24 +02:00
Ruwei
03b6be774d
Add example code for smtp client (#10345)
Motivation:

We don't have example code for smtp.

Modifications:

Add SmtpClient and SmtpClientHandler.

Result:

Provide an example for developers who want to write a smtp client.
2020-08-13 14:42:41 +02:00
Chris Vest
85b72923d9
Remove unused classes (#10476)
Motivation:
 Noticed we had some unused non-public classes.
 There is no reason to keep these around.

Modification:
 Remove unused non-public classes.

Result:
 Less code to worry about.
2020-08-13 11:31:41 +02:00