Motivation:
Unnecessary object allocation is currently done during wrap/unwrap while a handshake is still in progress.
Modifications:
Use static instances when possible.
Result:
Less object creations.
Motivation:
Sometimes it's useful to get informations about the available OpenSSL library that is used for the OpenSslEngine.
Modifications:
Add two new methods which allows to get the available OpenSSL version as either
an int or an String.
Result:
Easy to access details about OpenSSL version.
Motivation:
Other implementations of FullHttpMessage allow .toString to be called after the Message has been released
This brings AggregatedFullHttpMessage into line with those impls.
Modifications:
- Changed AggregatedFullHttpMessage to no longer be a sub-class of DefaultByteBufHolder
- Changes AggregatedFullHttpMessage to implement ByteBufHolder
- Hold the content buffer internally to AggregatedFullHttpMessage
- Implement the required content() and release() methods that were missing
- Do not check refcnt when accessing content() (similar to DefaultFullHttpMessage)
Result:
A released AggregatedFullHttpMessage can have .toString called without throwing an exception
Motivation:
Attribute.getAndRemove() will return the value but also remove the AttributeKey itself from the AttributeMap. This may not
what you want as you may want to keep an instance of it and just set it later again. Document the contract so the user know what to expect.
Modifications:
- Make it clear when to use AttributeKey.getAndRemove() / AttributeKey.remove() and when AttributeKey.getAndSet(null) / AttributeKey.set(null).
Result:
Less suprising behaviour.
Motivation:
Sometimes it's useful to use EC keys and not DSA or RSA. We should support it.
Modifications:
Support EC keys and share the code between JDK and Openssl impl.
Result:
It's possible to use EC keys now.
Motivation:
SslContext factory methods have gotten out of control; it's past time to
swap to a builder.
Modifications:
New Builder class. The existing factory methods must be left as-is for
backward compatibility.
Result:
Fixes#3531
Motivation:
As we missed to correctly handle EPOLLRDHUP we produce an IOException which is unnessary. This leads
to have exceptionCaught(...) methods called.
Modifications:
When EPOLLRDHUP was received just close the socket and fail all pending writes.
Result:
Correctly handle of EPOLLRDHUP and so not miss-leading exceptions.
Motivation:
When create NormalMemoryRegionCache for PoolThreadCache, we overbooked
cache array size. This means unnecessary overhead for thread local cache
as we will create multi cache enties for each element in cache array.
Modifications:
change:
int arraySize = Math.max(1, max / area.pageSize);
to:
int arraySize = Math.max(1, log2(max / area.pageSize) + 1);
Result:
Now arraySize won't introduce unnecessary overhead.
Changes to be committed:
modified: buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
Motivation:
CompositeByteBuf has an iterator() method but fails to implement Iterable
Modifications:
Let CompositeByteBuf implement Iterable<ByteBuf>
Result:
Easier usage
Motivation:
The ReplayingDecoderBuffer does not match the naming scheme we use for ByteBuf types.
Modifications:
Rename to ReplayingDecoderByteBuf to match naming scheme
Result:
Consistent naming
Motivation:
We missed to dereference the chunk and tmpNioBuf when calling deallocate(). This means the GC can not collect these as we still hold a reference while have the PooledByteBuf in the recycler stack.
Modifications:
Dereference chunk and tmpNioBuf.
Result:
GC can collect things.
Motivation:
We missed to flush the channel when using HttpChunkedInput (this is done when using SSL). This will result in a stale.
Modifications:
Replace ctx.write(...) with ctx.writeAndFlush(...)
Result:
Correctly working example.
Motiviation:
At the moment we use FIFO for the PoolThreadCache which is sub-optimal as this may reduce the changes to have the cached memory actual still in the cpu-cache.
Modification:
- Change to use LIFO as this increase the chance to be able to serve buffers from the cpu-cache
Results:
Faster allocation out of the ThreadLocal cache.
Before the commit:
[xxx wrk]$ ./wrk -H 'Connection: keep-alive' -d 120 -c 256 -t 16 -s scripts/pipeline-many.lua http://xxx:8080/plaintext
Running 2m test @ http://xxx:8080/plaintext
16 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 14.69ms 10.06ms 131.43ms 80.10%
Req/Sec 283.89k 40.37k 433.69k 66.81%
533859742 requests in 2.00m, 72.09GB read
Requests/sec: 4449510.51
Transfer/sec: 615.29MB
After the commit:
[xxx wrk]$ ./wrk -H 'Connection: keep-alive' -d 120 -c 256 -t 16 -s scripts/pipeline-many.lua http://xxx:8080/plaintext
Running 2m test @ http://xxx:8080/plaintext
16 threads and 256 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 16.38ms 26.32ms 734.06ms 97.38%
Req/Sec 283.86k 39.31k 361.69k 83.38%
540836511 requests in 2.00m, 73.04GB read
Requests/sec: 4508150.18
Transfer/sec: 623.40MB
Motivation:
To support HTTP2 we need APLN support. This was not provided before when using OpenSslEngine, so SSLEngine (JDK one) was the only bet.
Beside this CipherSuiteFilter was not supported
Modifications:
- Upgrade netty-tcnative and make use of new features to support ALPN and NPN in server and client mode.
- Guard against segfaults after the ssl pointer is freed
- support correctly different failure behaviours
- add support for CipherSuiteFilter
Result:
Be able to use OpenSslEngine for ALPN / NPN for server and client.
In TrafficCounter, a recent change makes the contract of the API (the
constructor) wrong and lead to issue with GlobalChannelTrafficCounter
where executor must be null.
Motivation:
TrafficCounter executor argument in constructor might be null, as
explained in the API, for some particular cases where no executor are
needed (relevant tasks being taken by the caller as in
GlobalChannelTrafficCounter).
A null pointer exception is raised while it should not since it is
legal.
Modifications:
Remove the 2 null checking for this particular attribute.
Note that when null, the attribute is not reached nor used (a null
checking condition later on is applied).
Result:
No more null exception raized while it should not.
This shall be made also to 4.0, 4.1 (present) and master. 3.10 is not
concerned.
Related: #3567
Motivation:
SslHandler.channelReadComplete() forgets to call
super.channelReadComplete(), which discards read bytes from the
cumulative buffer. As a result, the cumulative buffer can expand its
capacity unboundedly.
Modifications:
Call super.channelReadComplete() instead of calling
ctx.fireChannelReadComplete()
Result:
Fixes#3567
Related: #3368
Motivation:
ChunkedWriteHandler checks if the return value of
ChunkedInput.isEndOfInput() after calling ChunkedInput.close().
This makes ChunkedStream.isEndOfInput() trigger an IOException, which is
originally triggered by PushBackInputStream.read().
By contract, ChunkedInput.isEndOfInput() should not raise an IOException
even when the underlying stream is closed.
Modifications:
Add a boolean flag that keeps track of whether the underlying stream has
been closed or not, so that ChunkedStream.isEndOfInput() does not
propagate the IOException from PushBackInputStream.
Result:
Fixes#3368
Motivation:
For some use cases X509ExtendedTrustManager is needed as it allows to also access the SslEngine during validation.
Modifications:
Add support for X509ExtendedTrustManager on java >= 7
Result:
It's now possible to use X509ExtendedTrustManager with OpenSslEngine
Motivations:
JDK 1.8 adds default methods to collections classes that reference
classes that don't exist in JDK 7. That's binary compatible,
but not source compatible.
Modifications:
Enforce JDK version to be 1.7.* when releasing
Result:
Fixes#3548
Motivation:
There are new versions of the ALPN and NPN dependencies. There was also some backport misses in the pom file related to ALPN/NPN.
Modifications:
- Add new versions for ALPN/NPN dependencies.
- Backport missed pieces from pom.xml.
Result:
Updated version of ALPN/NPN versions.
(Ported @luciferous's changes against 3.10)
Motivation:
The current implementation of the encoder writes each character of the
String as a single byte to the buffer, however not all characters are
mappable to a single byte.
Modifications:
If a character is outside the ASCII range, it's converted to '?'.
Result:
A safer encoder for String to ASCII, which substitutes unmappable
Related: #3504
Motivation:
There are two places in the SocksAuthRequest constructor where an
IllegalArgumentException is thrown with a password as part of the
exception message.
This constitutes mishandling of confidential information, which can
compromise user privacy and is flagged as critical by security scanners.
Modifications:
Mask the password in the exception messages
Result:
No unexpected password leak
Motivation:
With the current implementation the client protocol preference list
takes precedence over the one of the server, since the select method
will return the first item, in the client list, that matches any of the
protocols supported by the server. This violates the recommendation of
http://tools.ietf.org/html/rfc7301#section-3.2.
It will also fail with the current implementation of Chrome, which
sends back Extension application_layer_protocol_negotiation, protocols:
[http/1.1, spdy/3.1, h2-14]
Modifications:
Changed the protocol negotiator to prefer server’s list. Added a test
case that demonstrates the issue and that is fixed with the
modifications of this commit.
Result:
Server’s preference list is used.
Motivation:
While the LengthFieldBasedFrameDecoder supports a byte order the LengthFieldPrepender does not.
That means that I can simply add a LengthFieldBasedFrameDecoder with ByteOrder.LITTLE_ENDIAN to my pipeline
but have to write my own Encoder to write length fields in little endian byte order.
Modifications:
Added a constructor that takes a byte order and all other parameters.
All other constructors delegate to this one with ByteOrder.BIG_ENDIAN.
LengthFieldPrepender.encode() uses this byte order to write the length field.
Result:
LengthFieldPrepender will write the length field in the defined byte order.
Motivation:
When an error happens during loading the native library it may try to generate a new RuntimeException before the RuntimeException is loaded.
Modifications:
- Load RuntimeException as first
Result:
No more segfaults possible
Motivation:
On a system where ipv4 and ipv6 are supported a user may want to use -Djava.net.preferIPv4Stack=true to restrict it to use ipv4 only.
This is currently ignored with the epoll transport.
Modifications:
Respect java.net.preferIPv4Stack system property.
Result:
-Djava.net.preferIPv4Stack=true will have the effect the user is looking for.
Motivation:
When remove0() is called multiple times for an DefaultAttribute it can cause corruption of the internal linked-list structure.
Modifications:
- Ensure remove0() can not cause corruption by null out prev and next references.
Result:
No more corruption possible
Related: #3476
Motivation:
Some users use TrafficCounter for other uses than we originally
intended, such as implementing their own traffic shaper. In such a
case, a user does not want to specify an AbstractTrafficShapingHandler.
Modifications:
- Add a new constructor that does not require an
AbstractTrafficShapingHandler, so that a user can use it without it.
- Simplify TrafficMonitoringTask
- Javadoc cleanup
Result:
We open the possibility of using TrafficCounter for other purposes than
just using it with AbstractTrafficShapingHandler. Eventually, we could
generalize it a little bit more, so that we can potentially use it for
other uses.
Motivation:
Not knowing which unit is used for the maxContentLength of the HttpObjectAggregator when reading the Javadoc is annoying and can be a source of bugs.
Modifications:
Added the mention "in bytes"
Result:
Javadoc is clear.
Related: #3464
Motivation:
When a connection attempt is failed,
NioSocketChannelUnsafe.closeExecutor() triggers a SocketException,
suppressing the channelUnregistered() event.
Modification:
Do not attempt to get SO_LINGER value when a socket is not open yet.
Result:
One less bug
Motivation:
Due a a regression that was introduced by b898bdd we failed to set the localAddress if the connect did not success directly.
Modifications:
Correct set localAddress in doConnect(...)
Result:
Be able to get the localAddress in all cases.
Related: #3445
Motivation:
HttpObjectDecoder.HeaderParser does not reset its counter (the size
field) when it failed to find the end of line. If a header is split
into multiple fragments, the counter is increased as many times as the
number of fragments, resulting an unexpected TooLongFrameException.
Modifications:
- Add test cases that reproduces the problem
- Reset the HeaderParser.size field when no EOL is found.
Result:
One less bug
Motivation:
`Unpooled` javadoc's mentioned the generation of hex dump and swapping an integer's byte order,
which are actually provided by `ByteBufUtil`.
Modifications:
Sentence moved to `ByteBufUtil` javadoc.
Result:
`Unpooled` javadoc is correct.
Motivation:
During 6b941e9bdb I introduced a regression that could cause an IllegalStateException.
A non-proper fix was commited as part of #3443. This commit add a proper fix.
Modifications:
Remove FileDescriptor.INVALID and add FileDescriptor.isOpen() as replacement. Once FileDescriptor.close() is called isOpen() will return false.
Result:
No more IllegalStateException caused by a close channel.
