Commit Graph

2957 Commits

Author SHA1 Message Date
Trustin Lee
d5264ebe85 Remove 'Content-Length' header from a chunked HTTP response
Related: #3076

Motivation:

When a user writes a chunked HTTP response with a Content-Length header,
the HttpContentEncoder should remove the Content-Length header because
the length of the encoded content is supposed to be different from the
original one.

Actually, HttpContentEncoder currently never touches the Content-Length
header when the response is chunked.

Modifications:

- Remove the Content-Length header when an HTTP response being encoded
  is chunked
- Add a test case

Result:

HttpContentEncoder sanitizes the Content-Length header properly.
2014-10-30 19:42:02 +09:00
Frederic Bregier
bbbcb9a706 V3.9 - Fix "=" character in HttpPostRequestDecoder
Motivation
Issue #3004 shows that "=" character was not supported as it should in
the HttpPostRequestDecoder in form-data boundary.

Modifications:
Add 2 methods in StringUtil

split with maxParm argument: String split with max parts only (to prevent multiple '=' to
be source of extra split while not needed)
substringAfter: String part after delimiter (since first part is not needed)
Use those methods in HttpPostRequestDecoder. Change and the
HttpPostRequestDecoderTest to check using a boundary beginning with "=".

Results:
The fix implies more stability and fix the issue.
2014-10-22 14:07:29 +09:00
Norman Maurer
c46b11f1ad [#2931] Only cancel SelectionKey if connection is not pending
Motivation:

We must only cancel the SelectionKey if the connection is not pending while try to workaround the epoll bug, otherwise we may fail to notify the future later.

Modifications:

Check if the connection is pending before cancel the SelectionKey.

Result:

Only cancel correct SelectionKeys and so also make sure the futures are notified.
2014-10-01 14:16:24 +02:00
kerr
6d3bb59598 Change the operator order of TrafficCounter to calculate the throughput to get the correct result
Motivation:
Currently the last read/write throughput is calculated by first division,this will be 0 if the last read/write bytes < interval,change the order will get the correct result

Modifications:
Change the operator order from first do division to multiplication

Result:
Get the correct result instead of 0 when bytes are smaller than interval
2014-10-01 06:46:37 +02:00
Frederic Bregier
3a3552c40a Fix overriden methos not called
Motivation:
channelConnected was override but super was never called, while it
should. It prevents next handlers to be informed of the connection.
Also add one missing information in the toString method.

Modifications:
Add the super corresponding call, and add checkInterval  to the
toString() method

Result;
Now, if the channelConnected method is correctly passed to the the next
handler.
2014-09-22 11:08:25 +02:00
Norman Maurer
cfa1074285 [#2426] Not cause busy loop when interrupt Thread of AbstractNioSelector
Motivation:
Because Thread.currentThread().interrupt() will unblock Selector.select() we need to take special care when check if we need to rebuild the Selector. If the unblock was caused by the interrupt() we will clear it and move on as this is most likely a bug in a custom ChannelHandler or a library the user makes use of.

Modification:
Clear the interrupt state of the Thread if the Selector was unblock because of an interrupt and the returned keys was 0.

Result:
No more busy loop caused by Thread.currentThread().interrupt()
2014-09-12 07:51:40 +02:00
Trustin Lee
534840a696 Fix a bug in CIDR.contains(InetAddress) implementations
Related issue: #2767

Motivation:

CIDR.contains(InetAddress) implementations should always return true
when the CIDR's prefix length is 0.

Modifications:

- Make CIDR.contains(InetAddress) return true if the current cidrMask is
  0
- Add tests

Result:

Fixed the issue #2767
2014-08-26 16:29:54 +09:00
Trustin Lee
907d38e9fe Add io.netty.noJdkZlibEncoder system property
Related issue: #2821

Motivation:

There's no way for a user to change the default ZlibEncoder
implementation.

Modifications:

Add a new system property 'io.netty.noJdkZlibEncoder'.
Use JZlib-based encoder if windowBits or memoryLevel is different from
the JDK default.

Result:

A user can tell HttpContentCompressor not to use JDK ZlibEncoder even if
the current Java version is 7+.
2014-08-26 16:05:25 +09:00
Frédéric Brégier
2a0d8818f1 Minimal fix on waiting time in TrafficShaping test
Motivation:
Possibly due to very small time (< 100ms), the trafficShaping time might be a bit high in rare condition.

Modification:
Remove extra time (only stepms is kept, not minimalms).

Result:
Time shall be ok now.
2014-08-24 11:25:12 +02:00
fredericBregier
0258772deb Fix for issue #2765 relative to unstable trafficshaping test procedure
Motivation:
The test procedure is unstable due to not enough precise timestamping
during the check.

Modifications:
Reducing the test cases and cibling "stable" test ("timestamp-able")
bring more stability to the tests.

Result:
Tests for TrafficShapingHandler seem more stable (whatever using JVM 6,
7 or 8).

Renaming to:
	src/test/java/org/jboss/netty/handler/traffic/TrafficShapingHandlerTest.java
2014-08-16 19:58:48 +02:00
Frédéric Brégier
ad75ee5550 Update TrafficShapingTest.java 2014-08-16 18:28:42 +02:00
Frédéric Brégier
7d4d17b217 Correct name
Motivation:
Wrong name!

Modification:
Change name

Result:
Compile ok
2014-08-16 18:28:17 +02:00
Frédéric Brégier
7ff9a43271 Fix for issue #2765 relative to unstable test
Fix for issue #2765 relative to unstable trafficshaping test procedure

Motivation:
The test procedure is unstable due to not enough precise timestamping during the check.

Modifications:
Reducing the test cases and cibling "stable" test ("timestamp-able") bring more stability to the tests.

Result:
Tests for TrafficShapingHandler seem more stable (whatever using JVM 6, 7 or 8).
Same version as in 4.0, 4.1 and Master.
2014-08-16 18:24:35 +02:00
Frédéric Brégier
f6ce33f9a5 Better fix for TrafficShapingHandlerTest
Motivation:
The test procedure is unstable when testing quick time (factor less or equal to 1). Changing to default 10ms in this case will force time to be correct and time to be checked only when factor is >= 2.

Modifications:
When factor is <= 1, minimalWaitBetween is 10ms

Result:
Hoping this version is finally stable.
2014-08-16 18:13:24 +02:00
fredericBregier
96f008b821 [#2749] Improve Traffic Shaping handler
Motivation:
Currently Traffic Shaping is using 1 timer only and could lead to
"partial" wrong bandwidth computation when "short" time occurs between
adding used bytes and when the TrafficCounter updates itself and finally
when the traffic is computed.
Indeed, the TrafficCounter is updated every x delay and it is at the
same time saved into "lastXxxxBytes" and set to 0. Therefore, when one
request the counter, it first updates the TrafficCounter with the added
used bytes. If this value is set just before the TrafficCounter is
updated, then the bandwidth computation will use the TrafficCounter with
a "0" value (this value being reset once the delay occurs). Therefore,
the traffic shaping computation is wrong in rare cases.

Secondly the traffic shapping should avoid if possible the "Timeout"
effect by not stopping reading or writing more than a maxTime, this
maxTime being less than the TimeOut limit.

Use same algorithm than in V4 and V5.

Modifications:
The TrafficCounter has 2 new methods that compute the time to wait
according to read or write) using in priority the currentXxxxBytes (as
before), but could used (if current is at 0) the lastXxxxxBytes, and
therefore having more chance to take into account the real traffic.

Moreover the Handler could change the default "max time to wait", which
is by default set to half of "standard" Time Out (30s:2 = 15s).

Include a test as in V4 but limited in the example to Nio.

Result:
The Traffic Shaping is better take into account (no 0 value when it
shouldn't) and it tries to not block traffic more than Time Out event.

This version is for V3.9 but could simply be port to V4.X and Master.
2014-08-13 01:49:27 +02:00
Trustin Lee
788d3dea42 Additional fix for lost first WebSocket frame after handshake
Related issue: #2179

Motivation:

Previous fix e71cbb9308 was not enough.

Modifications:

- Add more test cases for WebSocket handshake
- Fix a bug in HttpMessageDecoder where it does not always enter
  UPGRADED state
- Fix incorrect decoder replacement logic in WebSocketClientHandshaker
  implementations
  - Add WebSocketClientHandshaker.replaceDecoder() as a helper

Result:

We never lose the first WebSocket frame for all WebSocket protocol
versions.
2014-08-06 11:10:12 -07:00
Trustin Lee
c5e1ab6403 Checkstyle 2014-08-05 18:11:24 -07:00
Trustin Lee
8321035684 Remove StackTraceSimplifier
Related issue: #2735

Motivation:

When an application is under load and experiencing a lot of failure, the
instantiation of DefaultExceptionEvent spends noticeable amount of time
because of StackTraceSimplifier.

Also, StackTraceSimplifier makes people sometimes confused because it
hides the execution path partially.

Modifications:

Remove the use of StackTraceSimplifier

Result:

JVM spends much less time at Throwable.getStackTrace()
2014-08-05 18:03:15 -07:00
Trustin Lee
2b16d9f670 Fix the dead lock while failing SSL pending writes on a closed channel
Related issue: #2093

Motivation:

When a channel with SslHandler is closed with pending writes (either
unencrypted or encrypted), SslHandler.channelClosed() attempts to
acquire a lock and fail the futures of the pending writes.

If a user added a listener to any of the failed futures and the listener
attempts to write something, it will make SslHandler.handleDownstream()
invoked, which also attempts to acquire the same lock.

Because the lock is non-reentrant, even if these two lock acquisitions
are taking place in the same thread, the second attempt will block for
ever.

Modification:

Do not fail the futures while holding a lock.  Notify them after
releasing the lock.

Result:

One less dead lock
2014-08-05 17:35:11 -07:00
Trustin Lee
2c17b75a99 Fix a bug where SpdySession.getActiveStreams() returns incorrect set
Related issue: #2742

Motivation:

When there are more than one stream with the same priority, the set
returned by SpdySession.getActiveStream() will not include all of them,
because it uses TreeSet and only compares the priority of streams. If
two different streams have the same priority, one of them will be
discarded by TreeSet.

Modification:

- Rename getActiveStreams() to activeStreams()
- Replace PriorityComparator with StreamComparator

Result:

Two different streams with the same priority are compared correctly.
2014-08-05 16:47:39 -07:00
Trustin Lee
386a06dbfa HttpContentCompressor doesn't need to generate anything if Content-Length is 0
Motivation:

When Content-Encoding is deflate or gzip and Content-Length is 0,
there's no need to generate an empty stream (e.g. 20-byte gzip stream).
We can just produce nothing. At least, it works fine with most modern
browsers. Also, we can skip the instantiation of an encoder entirely by
instantiating it lazily.

Modifications:

Backport HttpContentEncoderTest from 4.0
Add similar tests for HttpContentCompressor to ensure the same tests
work with compression.

Result:

Fixes issue #2321
2014-08-05 15:22:24 -07:00
Trustin Lee
9efe48fc3a Do not issue a handshake when an SSLEngine is closing/closed
Motivation:

We sometimes get the following exception:

javax.net.ssl.SSLException: SSLEngine is closing/closed
  at sun.security.ssl.SSLEngineImpl.kickstartHandshake(SSLEngineImpl.java:692)
  at sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:734)
  at org.jboss.netty.handler.ssl.SslHandler.handshake(SslHandler.java:433)
  at org.jboss.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1233)
  at org.jboss.netty.handler.ssl.SslHandler.channelDisconnected(SslHandler.java:668)

.. which is triggered when we attempt to issue a handshake even if the
SSLEngine is closed (or being closed).

We don't really need to initiate handshake if:

- SSLEngine is closed already.
- Connection is closed already

Modifications:

Add a boolean parameter to unwrap() and unwrapNonApp() to suppress the
invocation of handshake() when SSLEngine or connection is closed
already.
2014-08-05 11:44:15 -07:00
Trustin Lee
ba84e44660 Fix a bug where ChannelFuture.setFailure(null) doesn't fail
Motivation:

We forgot to do a null check on the cause parameter of
ChannelFuture.setFailure(cause)

Modifications:

Add a null check

Result:

Fixed issue: #2728
2014-08-05 11:21:11 -07:00
Norman Maurer
6c0429391e [#2732] HttpRequestEncoder may produce invalid uri if uri parameters are included.
Motivation:

If the requests contains uri parameters but not path the HttpRequestEncoder does produce an invalid uri while try to add the missing path.

Modifications:

Correctly handle the case of uri with paramaters but no path.

Result:

HttpRequestEncoder produce correct uri in all cases.
2014-08-05 09:37:22 +02:00
Trustin Lee
9412ace4ee [#2718] Add private key decryption to JdkSslServerContext
Backported: d9cccccbb3

Motivation:

Currently it is not possible to load an encrypted private key when
creating a JDK based SSL server context.

Modifications:

- Added static method to JdkSslServerContext which handles key spec
  generation for (encrypted) private keys and make use of it.
- Added tests for creating a SSL server context based on a (encrypted)
  private key.

Result:

It is now possible to create a JDK based SSL server context with an
encrypted (password protected) private key.
2014-08-04 10:43:27 -07:00
Norman Maurer
c44be8e468 [#2713] Always use correct remote address in ConnectTimeoutException
Motivation:

As we cached ConnectTimeoutException we sometimes ended up using the wrong remote address in the exception message.

Modifications:

Always create a new ConnectTimeException and so make sure we use the connect remote address. This has a bit more overhead because of fill in the stacktrace everytime but as this only happens on connection timeouts it should be ok.

Result:

Always include the correct remote address in ConnectTimeoutException.
2014-08-04 09:37:49 +02:00
Trustin Lee
611269c0f0 Use our own URL shortener wherever possible 2014-07-31 17:05:12 -07:00
Jeff Pinner
9fc474ff33 Fire INTEREST_OPS ChannelStateEvent on change in AbstractNioWorker
Motivation:

We forgot to set the flag 'changed' to 'true' after updating
interestOps.

Modifications:

Add a missing 'changed = true;'

Result:

ChannelStateEvent(INTEREST_OPS) is always triggered correctly.
2014-07-30 16:56:52 -07:00
Trustin Lee
e71cbb9308 Fix a bug where the data that follows an HTTP upgrade response is lost
Related issue: #2179 and #2173

Motivation:

When a WebSocket server responds to a WebSocket handshake request with
its first WebSocket frame, it is sometimes swallowed by
HttpMessageDecoder.  This issue has been fixed by
4f6ccbbb78 in 4 and 5, but was not.

Modification:

Backport the UPGRADED state and the fix for #2173 to HttpMessageDecoder

Result:

The data sent by the server following an HTTP upgrade response is not
lost anymore.
2014-07-28 16:07:27 -07:00
Jeff Pinner
b35b25aedb SPDY: fix pushed response NullPointerException 2014-07-21 08:00:55 +02:00
Norman Maurer
2b401d4587 Make sure cancelled Timeouts are able to be GC'ed fast.
Motivation:
At the moment the HashedWheelTimer will only remove the cancelled Timeouts once the HashedWheelBucket is processed again. Until this the instance will not be able to be GC'ed as there are still strong referenced to it even if the user not reference it by himself/herself. This can cause to waste a lot of memory even if the Timeout was cancelled before.

Modification:
Add a new queue which holds CancelTasks that will be processed on each tick to remove cancelled Timeouts. Because all of this is done only by the WorkerThread there is no need for synchronization and only one extra object creation is needed when cancel() is executed. For addTimeout(...) no new overhead is introduced.

Result:
Less memory usage for cancelled Timeouts.
2014-06-11 10:39:49 +02:00
Trustin Lee
2fa9400a59 Fix a bug where SslHandler does not handle SSLv2Hello correctly
Motivation:

When a SSLv2Hello message is received, SSLEngine expects the application buffer size to be more than 30KB which is larger than what SslBufferPool can provide.  SSLEngine will always return with BUFFER_OVERFLOW status, blocking the SSL session from continuing the handshake.

Modifications:

When SSLEngine.getSession().getApplicationBufferSize() returns a value larger than what SslBufferPool provides, allocate a temporary heap buffer.

Result:

SSLv2Hello is handled correctly.
2014-06-10 17:56:18 +09:00
Frederic Bregier
129c17aaa4 [#2544] Correctly parse Multipart-mixed POST HTTP request in case of entity ends with odd number of 0x0D. Port of @fredericBregier 's work.
Motivation:
When an attribute is ending with an odd number of CR (0x0D), the decoder
add an extra CR in the decoded attribute and should not.

Modifications:
Each time a CR is detected, the next byte was tested to be LF or not. If
not, in a number of places, the CR byte was lost while it should not be.
When a CR is detected, if the next byte is not LF, the CR byte should be
saved as the position point to the next byte (not LF). When a CR is
detected, if there is not yet other available bytes, the position is
reset to the position of CR (since a LF could follow).

A new Junit test case is added, using DECODER and variable number of CR
in the final attribute (testMultipartCodecWithCRasEndOfAttribute).

Result:
The attribute is now correctly decoded with the right number of CR
ending bytes.
2014-06-09 10:18:38 +02:00
Frederic Bregier
dde0df9955 [#2542] HTTP post request decoder does not support quoted boundaries
Motivation:
According to RFC2616 section 19, boundary string could be quoted, but
currently the PostRequestDecoder does not support it while it should.

Modifications:
Once the boundary is found, one check is made to verify if the boundary
is "quoted", and if so, it is "unqoted".

Note: in following usage of this boundary (as delimiter), quote seems no
more allowed according to the same RFC, so the reason that only the
boundary definition is corrected.

Result:
Now the boundary could be whatever quoted or not. A Junit test case
checks it.
2014-06-09 10:12:39 +02:00
Jeff Pinner
7626484b08 Add RFC-6585 HTTP response status codes 2014-05-30 07:57:25 +02:00
Trustin Lee
9ab8898db1 Use a forked exec-maven-plugin instead of maven-antrun-plugin
Motivation:

maven-antrun-plugin does not redirect stdin, and thus it's impossible to
run interactive examples such as securechat-client and telnet-client.
org.codehaus.mojo:exec-maven-plugin redirects stdin, but it buffers
stdout and stderr, and thus an application output is not flushed timely.

Modifications:

Deploy a forked version of exec-maven-plugin which flushes output
buffers in a timely manner.

Result:

Interactive examples work.  Launches faster than maven-antrun-plugin.
2014-05-23 20:09:15 +09:00
Trustin Lee
4ce1274497 Deprecate getName() methods of the codec classes in SOCKS codec package
Motivation:

We do not provide such methods that provide the default name of the handlers.  It had to be noticed and removed during the review phase, but we failed to do so.

Modifications:

Deprecate getName() static methods in the SOCKS codec classes

Result:

Getting closer to other codecs in API design.
2014-05-23 17:24:03 +09:00
Trustin Lee
2ff4e458c1 Make hexdump proxy do hexdump
- Forgot to add LoggingHandler back into the pipeline
2014-05-23 14:51:22 +09:00
Trustin Lee
95c1013a34 Fix a problem where web socket examples do not work
- Fix wrong class names in run-example.sh
- Fix incorrect web socket url generation
2014-05-23 14:35:24 +09:00
Trustin Lee
5c7581ed9f Use maven-antrun-plugin instead of exec-maven-plugin
Motivation:

exec-maven-plugin does not flush stdout and stderr, making the console
output from the examples invisible to users

Modification:

Use maven-antrun-plugin instead

Result:

A user sees the output from the examples immediately.
2014-05-22 18:24:52 +09:00
Trustin Lee
ba28679775 Clean up the examples
Motivation:

The examples have not been updated since long time ago, showing various
issues fixed in this commit.

Modifications:

- Overall simplification to reduce LoC
  - Use system properties to get options instead of parsing args.
  - Minimize option validation
  - Just use System.out/err instead of Logger
  - Do not pass config as parameters - just access it directly
  - Move the main logic to main(String[]) instead of creating a new
    instance meaninglessly
    - Update netty-build-21 to make checkstyle not complain
  - Remove 'throws Exception' clause if possible
- Line wrap at 120 (previously at 80)
- Add an option to enable SSL for most examples
- Use ChannelFuture.sync() instead of await()
- Use System.out for the actual result. Use System.err otherwise.
- Delete examples that are not very useful:
  - websocket/html5
  - websocketx/sslserver
  - localecho/multithreaded
- Add run-example.sh which simplifies launching an example from command
  line

Result:

Shorter and simpler examples.  A user can focus more on what it actually
does than miscellaneous stuff.  A user can launch an example very
easily.
2014-05-22 17:52:23 +09:00
Trustin Lee
aa2c6b77e3 Fix NPE 2014-05-22 10:30:41 +09:00
Trustin Lee
ffa348273a Future compatibility with TLS ALPN
Motivation:

According to TLS ALPN draft-05, a client sends the list of the supported
protocols and a server responds with the selected protocol, which is
different from NPN.  Therefore, ApplicationProtocolSelector won't work
with ALPN

Modifications:

- Use Iterable<String> to list the supported protocols on the client
  side, rather than using ApplicationProtocolSelector
- Remove ApplicationProtocolSelector

Result:

Future compatibility with TLS ALPN
2014-05-22 10:12:42 +09:00
Trustin Lee
bbb5ee42b1 Fix NPE in OpenSslEngine 2014-05-21 20:01:22 +09:00
Trustin Lee
e4e7abc986 Escape a colon in protocol names 2014-05-21 17:44:07 +09:00
Trustin Lee
8672603a1d Add unified NextProtoNego extension support to SslContext
Motivation:

- OpenSslEngine and JDK SSLEngine (+ Jetty NPN) have different APIs to
  support NextProtoNego extension.
  - It is impossible to configure NPN with SslContext when the provider
    type is JDK.

Modification:

- Implement NextProtoNego extension by overriding the behavior of
  SSLSession.getProtocol() for both OpenSSLEngine and JDK SSLEngine.
  - SSLEngine.getProtocol() returns a string delimited by a colon (':')
    where the first component is the transport protosol (e.g. TLSv1.2)
    and the second component is the name of the application protocol
- Remove the direct reference of Jetty NPN classes from the examples
- Add SslContext.newApplicationProtocolSelector

Result:

- A user can now use both JDK SSLEngine and OpenSslEngine for NPN-based
  protocols such as HTTP2 and SPDY
2014-05-21 17:38:52 +09:00
Trustin Lee
2713341a7f Fix JDK 8 compatibility issue with OpenJdkSelfSignedCertGenerator
- X509CertInfo.setSubject/setIssuer() requires X500Name instead of
  CertificateSubjectName/CertificateIssuerName.
2014-05-18 03:49:07 +09:00
Trustin Lee
80e95220d8 Fix IndexOutOfBoundsException in OpenSslEngine
Motivation:

- In unwrap(), it does not check if the current index of dsts has
  reached at its end offset, resulting in IndexOutOfBoundsException.
- SSLEngine does not update the position of the source buffer correctly
  when SSL.writeToSSL() returns a negative value.

Modifications:

Fix them all

Result:

Less bugs
2014-05-18 02:41:41 +09:00
Trustin Lee
a17822cc45 Use EmptyArrays.EMPTY_X509_CERTIFICATES where possible 2014-05-17 23:44:50 +09:00
Trustin Lee
c2a3b86efc Add an exemplary snippet to SslContext 2014-05-17 23:40:27 +09:00