Motivation:
In JDK9 the Cleaner.clean method cannot be called as it is not exported
from `java.base`. `Runnable.run` should be called instead.
Modifications:
Pick Runnable.run if the cleaner implements Runnable. Otherwise try the
clean method on the class implementing the cleaner.
Result:
The cleaner for direct byte buffers is run on JDK9 as well as earlier
JDKs.
Motivation:
See #5054 and to fix remaining issue stopping Netty 3.10.6.Final being released.
Modification:
Copied IpToInt from Netty 4 and contains() functions.
Result:
Now issue #5054 shown examples are doing what they are expected to do.
Motivation:
To benefit from newer JDK CHM as the current internal CHM is the one from JDK 5.
Modification:
Removed old JDK 5 CHM from internal package.
Result:
If you are using JDK 7 or 8 there will be a slight performance benefit.
Motivation:
A rare issue was seen wherein a channel would become
persistently unwritable, even though the underlying
connection was working fine, as evidenced by the fact
that heartbeats were still being received from the
other side.
Examination of WriteRequestQueue and the threading
model around it revealed the following race:
T1: offer MessageEvent
go over high water mark
context switch before calling setUnwritable()
T2: poll multiple MessageEvent
go under low water mark
call setWritable()
T1: call setUnwritable()
At which point the channel is incorrectly marked as
unwritable and will remain so until another back-and-forth
across the low water mark, which may never happen if
the application uses writability to apply backpressure
on the writer.
The commit log shows that this issue has been present
since commit 2127c8eafe
i.e. version 3.10, when the introduction of user-defined
writability decoupled isWritable from the bufferSize.
Solution:
The use of a mutex was decided against due to the serious
performance implications.
To avoid subtle breakage in backward-compat, deferring the
call to setUnwritable() in the io thread was also decided
against.
Instead, the buffer size is re-checked against the water
mark immediately before firing the interestChanged event
and the writability change is rolled back if necessary.
In addition, a stronger invariant is enforced on poll()
to make it easier to reason about the possible water
mark/writability transitions. This requires minor changes
to NioChannelTest which manually calls poll().
Motivation
----------
Channels.write() is advertised to be thread-safe, however
if one thread starts a write and another closes the channel,
the first thread can find itself with a closed SSLEngine.
Throwing an unchecked exception will cause most programs to
crash, which is not acceptable for such an inconsequential
race.
Solution
--------
Throw SSLException or ClosedChannelException instead, which
are checked exceptions and should therefore be handled in a
non-crashy fashion by most applications.
Motivation:
Discovered during static code analysis. Found entries are overwritten
due to error in list instantiation. Also avoiding a possible NPE.
Modification:
Instantiating the list of ChannelFutures if it is not instantiated yet,
instead of overwriting an existing list.
Result:
SslHandler store all queued MessageEvents on channel closed. Fix for
https://github.com/netty/netty/issues/4296
Motivation:
1. Unfair of OrderedMemoryAwareThreadPoolExecutor
The task executed in OrderedMemoryAwareThreadPoolExecutor is unfair in
some situations.
For example, let's say there is only one executor thread that handle
the events from the two channels, and events are submitted in sequence:
Channel A (Event A1) , Channel B (Event B), Channel A (Event
A2) , ... , Channel A (Event An)
Then the events maybe executed in this unfair order:
-------------------> Timeline --------------------->
Channel A (Event A1) , Channel A (Event A2) , ... , Channel A (Event
An), Channel B (Event B)
As we see above, Channel B (Event B) maybe executed unfairly late.
Even more, if there are too much events come in Channel A, and
one-by-one closely, then Channel B (Event B) would be waiting for a long
while and become "hungry".
Modifications:
1. Fair of FairOrderedMemoryAwareThreadPoolExecutor
In the same case above ( one executor thread and two channels ) , this
implement will guarantee execution order as:
-------------------> Timeline -------------------->
Channel A (Event A1) , Channel B (Event B), Channel A (Event A2) ,
... ,Channel A (Event An)
NOTE: For convenience the case above use one single executor thread, but
the fair mechanism is suitable for multiple executor threads situations.
Result:
There is a fair-mode alternative of OrderedMemoryAwareThreadPoolExecutor
and OrderedDownstreamThreadPoolExecutor now.
Motivations:
When using HttpPostRequestEncoder and trying to set an attribute if a
charset is defined, currenlty implicit Charset.toStrng() is used, given
wrong format.
As in Android for UTF-16 = "com.ibm.icu4jni.charset.CharsetICU[UTF-16]".
Modifications:
Each time charset is used to be printed as its name, charset.name() is
used to get the canonical name.
Result:
Now get "UTF-16" instead.
(3.10 version)
Motivation:
RFC6265 specifies which characters are allowed in a cookie name and value.
Netty is currently too lax, which can used for HttpOnly escaping.
Modification:
Backport new RFC6265 compliant Cookie parsers in cookie subpackage.
Deprecate old Cookie encoders and decoders that will be dropped in 5.0.
Result:
The problem described in the motivation section is fixed.
Motivation:
This AE was seen in the wild at a non-negligible rate among AeroFS
clients (JDK 8, TLS 1.2, mutual auth with RSA certs).
Upon examination of SslHandler's code a few things became apparent:
- the AE is unnecessary given the contract of decode()
- the AE was introduced between 3.8 and 3.9
- the AE is no longer present in in 4.x and master
- branches that do not have the AE skip all the bytes being fed to
unwrap()
It is not entirely clear what sequence of SSL records can trip the
assert but it seems to happen before the handshake is completed. The
little detailed data we've been able to gather shows the assert being
triggered when
- SSLEngine.unwrap returns NEED_WRAP
- the remaining buffer is a TLS heartbeat record
Likewise, it is not entirely clear if skipping the remaining bytes is
the right thing to do or if they should be fed back to unwrap.
Modifications:
Mirror behavior in newer versions by removing the assert and skipping
bytes fed to unwrap()
Add logging in an effort to get a better understanding of this corner
case.
Result:
Avoid crashes
Motivation:
The following cookie key-value pair should not be decoded:
a='b;c=d
.. because it is invalid. Currently, a missing closing quote will make
the CookieDecoder decode "'b;c=d" as a value. This can be a problem
when 'c=d' is something you don't want to expose.
Modification:
Discard the cookie key-value pair when it has no matching quote.
Result:
The problem described in the motiviation section is fixed.
Motivation:
The documented thread model stipulates that all
upstream events should be fired from the IO thread,
which is crucial to avoid race conditions.
See #3546
Modifications:
Change AbstractNioChannel.WriteQueue to check if
the current thread is the IO thread of the worker
and call fireInterestChangedLater accordingly.
Result:
interestChanged event is always fired from IO
thread of channel thus honoring the documented
thread model and fixing race conditions.
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
characters with'?'.
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:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException when
trying to decode Content-Disposition header with filename containing ';'.
See issue #3326.
Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and use it in
splitMultipartHeader method, instead of StringUtils.split.
Result:
Filenames can contain semicolons.
Motivation:
Several issues were shown by various ticket (#2900#2956).
Also propose a similar improvement on writability user management than from #3036.
And finally add a mixte handler, both for Global and Channels, with
the advantages of being uniquely created and using less memory and
less shaping.
Issue #2900:
When a huge amount of data are written, the current behavior of the
TrafficShaping handler is to limit the delay to 15s, whatever the delay
the previous write has. This is wrong, and when a huge amount of writes
are done in a short time, the traffic is not correctly shapened.
Moreover, there is a high risk of OOM if one is not using in his/her own
handler for instance ChannelFuture.addListener() to handle the write
bufferisation in the TrafficShapingHandler.
This version includes "user-defined writability" capability added to the channel,
where writability could be managed, using isWritable() only from user handler side.
ChannelInterestChanged is also managed in order to provide compatibility
with other handlers as for instance ChunkedInput.
The "bandwidth" compute on write is only on "acquired" write orders, not
on "real" write orders, which is wrong from statistic point of view.
Issue #2956:
When using GlobalTrafficShaping, every write (and read) were
synchronized, thus leading to a drop of performance.
ChannelTrafficShaping is not touched by this issue since synchronzed is
then correct (handler is per channel, so the synchronized).
Modifications:
The current write delay computation takes into account the previous
write delay and time to check is the 15s delay (maxTime) is really
exceeded or not (using last scheduled write time). The algorithm is
simplified and in the same time more accurate.
A port of the "user-defined writability" as proposed in 4.X is included.
When the real write occurs, the statistics are update accordingly on a
new attribute (getRealWriteThroughput()).
To limit the synchronisations, all synchronized on
GlobalTrafficShapingHandler on submitWrite were removed. They are
replaced with a lock per channel (since synchronization is still needed
to prevent unordered write per channel), as in the sendAllValid method
for the very same reason.
Also all synchronized on TrafficCounter on read/writeTimeToWait() are
removed as they are unnecessary since already locked before by the
caller.
Still the creation and remove operations on lock per channel (PerChannel
object) are synchronized to prevent concurrency issue on this critical
part, but then limited using a ConcurrentHashMap.
Add a Mixte GlobalChannelTrafficShapingHandler which allows to use only one
handler for mixing Global and Channel TSH. I enables to save more memory and
tries to optimize the traffic among various channels.
Add 2 test class: AbstractChannelTest and NioChannelTest
Add test in reverse order in mixed mode
Result:
The traffic shaping is more stable, even with a huge number of writes in
short time by taking into consideration last scheduled write time.
The traffic shaping is more compatible with code as ChunkedWriteHandler,
adding a "user-defined writability".
The statistics are more valuable (asked write vs real write).
The Global TrafficShapingHandler should now have less "global"
synchronization, hoping to the minimum, but still per Channel as needed.
The GlobalChannelTrafficShapingHandler allows to have only one handler for
all channels while still offering per channel in addition to global traffic
shaping.
Motivation:
During the reading of the source codes of Netty 3.9.5.Final, I thought there might have a lurking concurrency bug in the AbstractNioBossPool#init method, of which a single volatile variable cannot correctly control the initialization sequence happened exactly only once under concurrency environment. Also I found there's already a much more elegant and concurrency safe way to do the work like this, I decided to make this PR. (Please refer to the discussion in https://github.com/netty/netty/issues/3249.)
Modifications:
Change the type of the variable that control the initialization from "volatile boolean" to "final AtomicBoolean".
Result:
The potential concurrency hazard of the initialization in AbstractNioBoss(Worker)Pool will be eliminated.
Motivations:
The chunkSize might be oversized after comparison (size being > of int capacity) if file size is bigger than an integer.
Modifications:
Changing the type to long fix the issue.
Result:
There is no more int oversized.
Related:
- d6c3b3063f
- Original author: @grahamedgecombe
Motivation:
JdkSslContext used SSL_RSA_WITH_DES_CBC_SHA in its cipher suite list.
OpenSslServerContext used DES-CBC3-SHA in the same place in its cipher suite
list, which is equivalent to SSL_RSA_WITH_3DES_EDE_CBC_SHA.
This means the lists were out of sync. Furthermore, using
SSL_RSA_WITH_DES_CBC_SHA is not desirable as it uses DES, a weak cipher. Triple
DES should be used instead.
Modifications:
Replace SSL_RSA_WITH_DES_CBC_SHA with SSL_RSA_WITH_3DES_EDE_CBC_SHA in
JdkSslContext.
Result:
The JdkSslContext and OpenSslServerContext cipher suite lists are now in sync.
Triple DES is used instead of DES, which is stronger.
Motivation:
RC4 is not a recommended cipher suite anymore, as the recent research
reveals, such as:
- http://www.isg.rhul.ac.uk/tls/
Modifications:
- Remove most RC4 cipher suites from the default cipher suites
- For backward compatibility, leave RC4-SHA, while de-prioritizing it
Result:
Potentially safer default
Related: #3107, origianlly written by @Scottmitch
Motiviation:
The HttpContentEncoder does not account for an EmptyLastHttpContent
being provides as input. This is useful in situations where the client
is unable to determine if the current content chunk is the last content
chunk (i.e. a proxy forwarding content when trnasfer encoding is
chunked)
Modifications:
- HttpContentEncoder should not attempt to compress empty HttpContent
objects.
Result:
HttpContentEncoder supports a EmptyLastHttpContent to terminate the
response.
Related: #3107
Motivation:
ZlibEn/Decoder and JdkZlibEncoder in 3.9 do not have any unit tests.
Before applying any patches, we should backport the tests in 4.x so that
we can make sure we do not break anything.
Modification:
- Backport ZlibTest and its subtypes
- Remove the test for automatic GZIP header detection because the
ZlibDecoders in 3.9 do not have that feature
- Initialize JdkZlibEncoder.out and crc only when necessary for reduced
memory footprint
- Fix the bugs in the ZlibEncoders where it fails to compress correctly
when there are not enough room in the output buffer
Result:
We are more confident when we make changes in ZlibEncoder/Decoder
Bugs have been squashed
Motivation:
The SPDY/3.1 spec does not adequate describe how to push resources
from the server. This was solidified in the HTTP/2 drafts by dividing
the push into two frames, a PushPromise containing the request,
followed by a Headers frame containing the response.
Modifications:
This commit modifies the SpdyHttpDecoder to support pushed resources
that are divided into multiple frames. The decoder will accept a
pushed SpdySynStreamFrame containing the request headers, followed by
a SpdyHeadersFrame containing the response headers.
Result:
The SpdyHttpDecoder will create an HttpRequest object followed by an
HttpResponse object when receiving pushed resources.
Related: #3131
Motivation:
To prevent users from accidentally enabling SSLv3 and making their
services vulnerable to POODLE, disable SSLv3 when SSLEngine is
instantiated via SslContext.
Modification:
- Disable SSLv3 for JdkSslContext and OpenSslServerContext
Result:
Saner default set of protocols
Related:
- 37a6f5ed5d
Motivation:
Minimize the backport cost by synchronizing NetUtil between 3.9 and 4.x
Modifications:
- Backport the bug fixes in NetUtil
- Backport the new IP address methods in NetUtil
Result:
- New useful methods in NetUtil
- Easier to backport the future bug fixes
Motivation:
Since JDK 1.8, javadoc has enabled a new feature called 'doclint', which
fails the build when javadoc has markup problems and more.
Modifications:
Do not fail the build until we fix our API documentation.
Result:
No more build failure because of malformed Javadoc
Motivation:
Sonar uses the project name in the pom.xml as the project name. (no pun
intended) 4.x and master uses 'Netty' as the project name, so we should
be consistent.
Modifications:
Rename the project from 'The Netty project' to 'Netty'
Result:
Prettier SonarQube result
Carsten Varming
Norman Maurer
Guido Medina
Hugues Bruant
Jens Kordowski
atellwu
Brad Kim
James Roper
Frederic Bregier
Stephane Landelle
Trustin Lee
Neuman Vong
Taeho Kim
Jeff Pinner
luofucong