Motivation:
The `AbstractTrafficShapingHandler` caches the `ReopenReadTimerTask` instance in the channel attribute. However, if this handler is removed from the channel pipeline, this `ReopenReadTimerTask` instance may not be released.
Modification:
Release the channel attribute `REOPEN_TASK` in `handlerRemoved` method.
Result:
Avoid a channel attribute leak.
Motivation:
Parameter maxCount needs the unit test.
Modifications:
1. Change the conditional statement to avoid the ineffective maxCount (enhance the robustness of the code merely).
2. Add the unit test for maxCount.
Result:
Enable this parameter to be tested.
Motivation:
We should update our optional bouncycastle dependency to ensure we use the latest which has all the security fixes
Modifications:
Update bouncycastle version
Result:
Fixes https://github.com/netty/netty/issues/10184
Motivation:
Only cipher suite is logged during handshake. Picked protocol is interesting too.
Modification:
Log protocol as well.
Result:
More interesting information when debugging TLS handshakes.
**Motivation:**
Following up on https://github.com/netty/netty/issues/10181
I was able to successfully setup a Netty server with TLS on Android, using Conscrypt, with the change proposed here. Conscrypt publishes an [Android specific version](https://github.com/google/conscrypt#android). But the current availability check prevents Netty from using it (the call `PlatformDependent.javaVersion()` returns `6` on Android).
**Modification:**
Check whether the java version is above 8, or if it's running on an Android device, to flag Conscrypt as available (besides the remaining checks).
**Result:**
Netty with TLS runs fine on Android 👍
Motivation:
Often people want to use `stomp-codec` with WebSocket transport or other but cannot figure out how can do this staff on Netty.
Modification:
Create example for demonstrating integration between STOMP and WebSocket.
Inspired by https://github.com/jmesnil/stomp-websocket
Result:
Fixes#9383
Motivation:
A user might want to cancel DNS resolution when it takes too long.
Currently, there's no way to cancel the internal DNS queries especially when there's a lot of search domains.
Modification:
- Stop sending a DNS query if the original `Promise`, which was passed calling `resolve()`, is canceled.
Result:
- You can now stop sending DNS queries by cancelling the `Promise`.
Motivation:
Related https://github.com/line/armeria/issues/2463
Here is an example that an NIC has only link local address for IPv6.
```
$ ipaddr
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
3: eth0@if18692: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1460 qdisc noqueue
link/ether 1a:5e:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
inet 10.xxx.xxx.xxx/24 scope global eth0
valid_lft forever preferred_lft forever
inet6 fe80::xxxx:xxxx:xxxx:xxxx/64 scope link
valid_lft forever preferred_lft forever
```
If the NICs have only local or link local addresses, We should not send IPv6 DNS queris.
Modification:
- Ignore link-local IPv6 addresses which may exist even on a machine without IPv6 network.
Result:
- `DnsNameResolver` does not send DNS queries for AAAA when IPv6 is not available.
Motivation:
An `IOException` may be thrown from `FileChannel.write` or `FileChannel.force`, and cause the fd leak.
Modification:
Close the file in a finally block.
Result:
Avoid fd leak.
Motivation:
PoolChunk requires a link to a PoolThreadCache to init ByteBuf. Currently the link is retrieved from a thread local: arena.parent.threadCache().
It has some performance cost. At the beginning of the allocation call the PoolThreadCache is already retrieved from the thread local. The reference can be propagated through the calls and used.
Modifications:
Replace second lookup of PoolThreadCache during ByteBuf init by propagation of a reference to PoolThreadCache down in the allocation stack explicitly
Result:
Improve performance of ByteBuf allocation
--Before--
Benchmark (size) (tokens) (useThreadCache) Mode Cnt Score Error Units
SimpleByteBufPooledAllocatorBenchmark.getAndRelease 123 0 true avgt 20 57.112 ± 1.004 ns/op
SimpleByteBufPooledAllocatorBenchmark.getAndRelease 123 100 true avgt 20 222.827 ± 1.307 ns/op
--After--
Benchmark (size) (tokens) (useThreadCache) Mode Cnt Score Error Units
SimpleByteBufPooledAllocatorBenchmark.getAndRelease 123 0 true avgt 20 50.732 ± 1.321 ns/op
SimpleByteBufPooledAllocatorBenchmark.getAndRelease 123 100 true avgt 20 216.892 ± 3.806 ns/op
Motivation:
An exception may occur between ByteBuf's allocation and release. So I think it's a good idea to place the release operation in a finally block.
Modification:
Release the temporary ByteBuf in finally blocks.
Result:
Avoid ByteBuf leak.
Motivation:
Different versions of the JDK use different TLS versions by default. We should define the versions explicit
Modifications:
Explicit specify TLSv1.2
Result:
Blockhound tests pass on JDK14 as well
Motivation:
If the SslHandler is removed from the pipeline we also need to ensure we fail the handshake / close promise if it was not notified before as otherwise we may never do so.
Modifications:
- Correctly fail promise and notify pipeline if handshake was not done yet when the SslHandler is removed
- Add unit test
Result:
Fix https://github.com/netty/netty/issues/10158
Motivation:
An IOException may be thrown from FileChannel.read, and cause the fd leak.
Modification:
Close the file when IOException occurs.
Result:
Avoid fd leak.
Motivation:
PoolChunk.usage() method has non-trivial computations. It is used currently in hot path methods invoked when an allocation and de-allocation are happened.
The idea is to replace usage() output comparison against percent thresholds by Chunk.freeBytes plain comparison against absolute thresholds. In such way the majority of computations from the threshold conditions are moved to init logic.
Modifications:
Replace PoolChunk.usage() conditions in PoolChunkList with equivalent conditions for PoolChunk.freeBytes()
Result:
Improve performance of allocation and de-allocation of ByteBuf from normal size cache pool
Motivation:
How we did wildcard matching was not correct according to RFC6125. Beside this our implementation was quite CPU heavy.
Modifications:
- Add new DomainWildcardMappingBuilder which correctly does wildcard matching. See https://tools.ietf.org/search/rfc6125#section-6.4
- Add unit tests
- Deprecate old implementations
Result:
Correctly implement wildcard matching and improve performance
Motivation:
Not always STOMP frames contain any payload some times it just headers. So we wan't allocate additional buffer with NULL content for this situation.
Modification:
Modify StompSubframeEncoder to check if content is readable or not. If content is not readable just add NULL byte to encoded header buffer.
Result:
Less allocations
Motivation:
JDK 14 was released and need some special settings to be able to build with. Also there seems to be one regression that we need to workaround for now.
Modifications:
- Add maven profile for JDK 14
- Update blockhound version to be able to work on JDK 14
- Add workaround for possible JDK 14 regression
Result:
Be able to build on JDK 14
Motivation:
Mqtt procotol defines packetId instead of messageId, replace deprecated method to match protocol definition
Modification:
Replace messageId() to packetId() in MqttEncoder
Result:
Match mqtt protocol definition and improve readability.
Motivation:
currently (http) disk based attributes or uploads are globally configured in a single directory and can also only globally be deleted on exit or not. it does not fit well multiple cases, in particular the case you have multiple servers in the same JVM.
Modification:
make it configurable per attribute/fileupload.
Result:
This PR duplicates Disk* constructor to add basedir and deleteonexit parameters and wires it in default http daa factory.
Motivation:
An IOException may be thrown from InputStream.read or checkSize method, and cause the ByteBuf leak.
Modification:
Release the ByteBuf when IOException occurs.
Result:
Avoid ByteBuf leak.
Motivation:
JDK 14 was released so we should include it in our build matrix. Beside this there was also a JDK 13 update
Modifications:
- Add docker-compose files for JDK 14
- Update JDK 13 version
Result:
Build with JDK 14 as well and use latest JDK 13 release
Motivation:
Before release 4.1.23, there was only ONE call to exceptionCaught method when an ssl handshake failure occurs, now we have two when using the JDK provider.
Modifications:
- Ensure we only propagate one exception fi we already failed the handshake and channelInactive(...) produce an exception again
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/10119
Motivation:
An exception may occur between ByteBuf's allocation and release. For example:
```java
java.lang.OutOfMemoryError: Java heap space
at java.lang.String.<init>(String.java:325)
at io.netty.buffer.ByteBufUtil.decodeString(ByteBufUtil.java:838)
at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1247)
at io.netty.buffer.AbstractByteBuf.toString(AbstractByteBuf.java:1242)
at io.netty.handler.proxy.HttpProxyHandler.<init>(HttpProxyHandler.java:105)
at io.netty.handler.proxy.HttpProxyHandler.<init>(HttpProxyHandler.java:90)
at io.netty.handler.proxy.HttpProxyHandler.<init>(HttpProxyHandler.java:85)
```
It may cause the ByteBuf variable authz and authzBase64's leak.
Modification:
Release the ByteBuf in a finally block as soon as possible.
Result:
Fix a potential ByteBuf leak.
Motivation:
The current implementation of log2 in PoolThreadCache uses a loop and less efficient than an version based on Integer.numberOfLeadingZeros (intrinsic).
Modifications:
Replace the current log2 implementation in PoolThreadCache with a version based on Integer.numberOfLeadingZeros
Result:
It can improve performance slightly during allocation and de-allocation of ByteBuf using pooled allocator.
Motivation:
In the code example of ReplayingDecoder, an input parameter List<Object> out is missing.
Modification:
Just add this parameter.
Result:
The right doc.
Motivation:
A new netty-tcnative version was just released which fixes a small memory leak.
Modifications:
Update to 2.0.30.Final
Result:
Small memory leak fixed
Motivation:
At the moment HttpObjectDecoder does not limit the number of controls chars. These should be counted with the initial line and so a limit should be exposed
Modifications:
- Change LineParser to also be used when skipping control chars and so enforce a limit
- Add various tests to ensure that limit is enforced
Result:
Fixes https://github.com/netty/netty/issues/10111
Motivation:
We need to ensure we always close the Channel when we see an exception during bootstrapping it.
Modification:
Ensure we correct close the Channel if we see an exception during retrieving the Resolver from the group while bootstrapping.
Result:
Fixes#10109
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Motivation:
HttpProxyHandler itself will add a HttpClientCodec into the ChannelPipeline and so confuse the WebSocket*Handshaker when it tries to modify the pipeline as it will replace the wrong HttpClientCodec.
Modifications:
Wrap the internal HttpClientCodec that is added by HttpProxyHandler so it will not be replaced by HttpProxyHandler.
Result:
Fixes https://github.com/netty/netty/issues/5201 and https://github.com/netty/netty/issues/5070
Motivation:
Netty's DefaultThreadFactory that creates FastThreadLocalThread instance is widely used in NioEventLoopGroup, EpollEventLoopGroup, etc, but not OioEventLoopGroup. Although oio is quite stale, I still think this change may be useful.
Modification:
Replace oio's default thread factory with netty's DefaultThreadFactory just like NioEventLoopGroup, EpollEventLoopGroup, etc.
Result:
Faster access to FastThreadLocal in oio.
Motivation:
Http2StreamChannelId is Serializable. A test case is needed to ensure that it works.
Modification:
Add a test case about serialization.
Result:
Improve test coverage slightly.
Motivation:
In our WebSocketClientHandshaker* implementations we "hardcode" the version number to use. This is error-prone, we should better use the WebSocketVersion so we dont need to maintain the value multiple times. Beside this we can also use an AsciiString to improve performance
Modifications:
- Use WebSocketVersion.toAsciiString
Result:
Less stuff to maintain and small performance win
Motivation:
The user may need to provide a specific HOST header. We should not override it when specified during handshake.
Modifications:
Check if a custom HOST header is already provided by the user and if so dont override it
Result:
Fixes https://github.com/netty/netty/issues/10101
Motivation:
Netty currently does not support the SameSite attribute for response cookies (see issue #8161 for discussion).
Modifications:
The attribute has been added to the DefaultCookie class as a quick fix since adding new methods to the Cookie interface would be backwards-incompatible.
ServerCookieEncoder and ClientCookieDecoder have been updated accordingly to process this value. No validation for allowed values (Lax, None, Strict) has been implemented.
Result:
Response cookies with the SameSite attribute set can be read or written by Netty.
Co-authored-by: David Latorre <a-dlatorre@hotels.com>
Motivation:
Since the LZF support non-compress and compress format, we can let LzfEncoder support length aware ability. It can let the user control compress.
Modification:
When the data length over compressThreshold, LzfEncoder use compress format to compress data. Otherwise, only use non-compress format. Whatever compress format the encoder use, the LzfDecoder can decompress data well.
Result:
Gives users control over compression capabilities
Motivation:
WebSocketClientHandshaker#upgradeUrl doesn't comperly compute relative url when path is empty and produces url such as `?access_token=foo` instead of `/?access_token=foo`.
Modifications:
* fix WebSocketClientHandshaker#upgradeUrl
* add tests for urls without path, with and without query
Result:
WebSocketClientHandshaker properly connects to url without path.
Motivation:
Magic numbers seem hard to read or understand.
Modification:
Replace several magic numbers with named constants.
Result:
Improve readability and make it easier to maintain.
**Motivation:**
When I was previously working on a project using Netty's HTTP/2 support, I used the newer frames approach but I struggled to find any good examples or documentation online. I did, however, see a few people ask the same (or similar) questions as me on StackOverflow and a couple of older Netty Github issues.
Reading issue [9733](https://github.com/netty/netty/issues/9733) therefore prompted me to pull together a few bits of code into this HTTP/2 frame client example.
**Modification:**
Populated the previously-empty `example/src/main/java/io/netty/example/http2/helloworld/frame/client/` folder with a HTTP/2 frame client example.
**Result:**
Gives a clear example of how the newer HTTP/2 support can be used for Netty clients.
Motivation:
ThrowableUtil.stackTraceToString is an expensive method call. So I think a log level check before this logging statement is quite needed especially in a environment with the warning log disabled.
Modification:
Add log level check simply before logging.
Result:
Improve performance in a environment with the warning log disabled.
Motivation:
939e928312 introduced MacOSDnsServerAddressStreamProvider which will ensure the right nameservers are selected when running on MacOS. To ensure this is done automatically on MacOS we should use it by default on these platforms.
Modifications:
Try to use MacOSDnsServerAddressStreamProvider when on MacOS via reflection and fallback if not possible
Result:
Ensure the right nameservers are used on MacOS even when a VPN (for example) is used.
Motivation:
I am receiving a mutlipart/form_data upload from postman. The filename contains Chinese, and so some invalid chars. We should ensure all of these are removed before trying to decode.
Modification:
Ensure all invalid characters are removed
Result:
Fixes#10087
Co-authored-by: liming.zhang <liming.zhang@luckincoffee.com>
Motivation:
In general, we will close the debug log in a product environment. However, logging without external level check may still affect performance as varargs will need to allocate an array.
Modification:
Add log level check simply before logging.
Result:
Improve performance slightly in a product environment.
