src

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Use the Runnable.run method to clean direct byte buffers if avaiable.

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.

Fixes #5054: IpV4Subnet.contains() always returns true for single-address subnets.

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.

Removed old JSR 166 CHM which was ported from JDK 5 to JDK 1.4

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.

SslHandler: Avoid unchecked exception on write

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.

Fix race when changing channel writability

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 2127c8eafef61a27e40c66479552febb2f042847

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().

SslHandler misses to store all queued MessageEvents on channel closed

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

Fix invalid check arguments in 'OpenSslServerContext.setTicketKeys'

Motivation:

Invalid checking arguments causes the function not to work.

Modifications:

Modified checking arguments.

Result:

Fixes #3922

Fix invalid check arguments in 'OpenSslServerContext.setTicketKeys'

Motivation:

Invalid checking arguments causes the function not to work.

Modifications:

Modified checking arguments.

Result:

Fixes #3922

This is a fair-mode alternative of OrderedMemoryAwareThreadPoolExecutor.

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.

Fix cookie encoding when maxAge is greater than 24 days

Fix cookie encoding when maxAge is greater than 24 days

Proposal to fix issue #3768 (3.10)

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)

Proposal to fix issue #3768 (3.10)

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)

Fix binary compatibility

Change type of Cookie.maxAge back to int to maintain binary

compatibility for drop in fixes.

Fix binary compatibility

Change type of Cookie.maxAge back to int to maintain binary

compatibility for drop in fixes.

Validate cookie name and value characters

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.

    • -0
    • +141
    ./main/java/org/jboss/netty/handler/codec/http/cookie/Cookie.java
  1. … 11 more files in changeset.
Validate cookie name and value characters

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.

    • -0
    • +141
    ./main/java/org/jboss/netty/handler/codec/http/cookie/Cookie.java
  1. … 11 more files in changeset.
Do not decode a cookie value with mismatching quotes

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.

Fix overzealous assertion in SslHandler.decode

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

Fix overzealous assertion in SslHandler.decode

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

Always fire interestChanged from IO thread

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.

Safely encode Strings to ASCII

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'?'.

Hide password in exception messages of SocksAuthRequest

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

Accept ';' in the filename of HTTP Content-Disposition header

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.

Clean-up

Prevent the potential concurrency hazard of the initialization in AbstractNioBoss(Worker)Pool.

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.

Fix AbstractDiskHttpData int conversion from long

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.

Use Triple DES in JdkSslContext cipher suite list.

Related:

- d6c3b3063fc367791dbddb76220110d864e5c5f8

- 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.

Remove or de-prioritize RC4 from default cipher suites

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

HTTP Content Encoder allow EmptyLastHttpContent

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.