Commit Graph

1331 Commits

Author SHA1 Message Date
Norman Maurer
bc32efe396 Add testcase for internal used Comparator in ClientCookieEncoder (#9897)
Motivation:

https://github.com/netty/netty/pull/9883 added a bug-fix for the Comparator in ClientCookieEncoder but did not add a testcase.

Modifications:

- Add testcase
- Simplify code

Result:

Include a test to ensure we not regress.
2019-12-20 08:50:27 +01:00
Gerd Riesselmann
ae313513dd Avoid possible comparison contract violation (#9883)
Motivation:

The current implementation causes IllegalArgumetExceptions to be thrown on Java 11.

The current implementation would violate comparison contract for two cookies C1 and C2 with same path length, since C1 < C2 and C2 < C1. Returning 0 (equality) does not since C1 == C2 and C2 == C1. See #9881

Modification:

Return equality instead of less than on same path length.

Result:

Fixes #9881.
2019-12-19 12:28:25 +01:00
Anuraag Agrawal
0f42eb1ceb Use array to buffer decoded query instead of ByteBuffer. (#9886)
Motivation:

In Java, it is almost always at least slower to use `ByteBuffer` than `byte[]` without pooling or I/O. `QueryStringDecoder` can use `byte[]` with arguably simpler code.

Modification:

Replace `ByteBuffer` / `CharsetDecoder` with `byte[]` and `new String`

Result:

After
```
Benchmark                                   Mode  Cnt  Score   Error   Units
QueryStringDecoderBenchmark.noDecoding     thrpt    6  5.612 ± 2.639  ops/us
QueryStringDecoderBenchmark.onlyDecoding   thrpt    6  1.393 ± 0.067  ops/us
QueryStringDecoderBenchmark.mixedDecoding  thrpt    6  1.223 ± 0.048  ops/us
```

Before
```
Benchmark                                   Mode  Cnt  Score   Error   Units
QueryStringDecoderBenchmark.noDecoding     thrpt    6  6.123 ± 0.250  ops/us
QueryStringDecoderBenchmark.onlyDecoding   thrpt    6  0.922 ± 0.159  ops/us
QueryStringDecoderBenchmark.mixedDecoding  thrpt    6  1.032 ± 0.178  ops/us
```

I notice #6781 switched from an array to `ByteBuffer` but I can't find any motivation for that in the PR. Unit tests pass fine with an array and we get a reasonable speed bump.
2019-12-18 21:15:44 +01:00
Norman Maurer
607cf801d2 Revert "Bugfix #9673: Origin header is always sent from WebSocket client (#9692)"
This reverts commit f48d9fa8d0 as it needs more thoughts
2019-12-18 09:24:17 +01:00
Norman Maurer
0e4c073bcf
Remove the intermediate List from ByteToMessageDecoder (and sub-class… (#8626)
Motivation:

ByteToMessageDecoder requires using an intermediate List to put results into. This intermediate list adds overhead (memory/CPU) which grows as the number of objects increases. This overhead can be avoided by directly propagating events through the ChannelPipeline via ctx.fireChannelRead(...). This also makes the semantics more clear and allows us to keep track if we need to call ctx.read() in all cases.

Modifications:

- Remove List from the method signature of ByteToMessageDecoder.decode(...) and decodeLast(...)
- Adjust all sub-classes
- Adjust unit tests
- Fix javadocs.

Result:

Adjust ByteToMessageDecoder as noted in https://github.com/netty/netty/issues/8525.
2019-12-16 21:00:32 +01:00
Norman Maurer
ac76a24ff6 Verify we do not receive multiple content-length headers or a content-length and transfer-encoding: chunked header when using HTTP/1.1 (#9865)
Motivation:

RFC7230 states that we should not accept multiple content-length headers and also should not accept a content-length header in combination with transfer-encoding: chunked

Modifications:

- Check for multiple content-length headers and if found mark message as invalid
- Check if we found a content-length header and also a transfer-encoding: chunked and if so mark the message as invalid
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9861
2019-12-13 08:53:51 +01:00
Dmitriy Dumanskiy
0611683106 #9867 fix confusing method parameter name (#9874)
Motivation:

Parameter name is confusing and not match the actual type.

Modification:

Rename parameter.

Result:

Code cleanup
2019-12-12 14:42:30 +01:00
Norman Maurer
4a8476af67 Revert "Protect ChannelHandler from reentrancee issues (#9358)"
This reverts commit 48634f1466.
2019-12-12 14:42:30 +01:00
Norman Maurer
745814b382 Detect missing colon when parsing http headers with no value (#9871)
Motivation:

Technical speaking its valid to have http headers with no values so we should support it. That said we need to detect if these are "generated" because of an "invalid" fold.

Modifications:

- Detect if a colon is missing when parsing headers.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9866
2019-12-11 15:49:41 +01:00
Andrey Mizurov
b7d685648e Fix #9770, last frame may contain extra data that doesn't affect decompression (#9832)
Motivation:
Client can split data into different numbers of fragments and sometimes the last frame may contain trash data that doesn't affect decompression process.

Modification:
Added check if last frame is `ContinuationWebSocketFrame` and decompression data is empty
then don't throw an exception.

Result:
Fixes #9770
2019-12-11 15:45:44 +01:00
Carl Mastrangelo
daa354d659 Add Server Cookie Parser overload for multiple cookies. (#9856)
Motivation:

Multiple cookie values can be present in a single header.

Modification:

Add `decodeAll` overload which returns all cookies

Result:

Fixes #7210

Note:

This change is not as perscriptive as the ideas brought up in the linked issue.  Changing the Set implementation or the equals/compareTo definition is likely a breaking change, so they are practical.
2019-12-10 11:44:58 +01:00
Norman Maurer
ff8846f1b5
Replace ObjectUtil.checkNonNull(...) with Objects.requireNonNull(...) (#9864)
Motivation:

We should use Objects.requireNonNull(...) as we require java8

Modifications:

Replace ObjectUtil.checkNonNull(...) with Objects.requireNonNull(...)

Result:

Code cleanup
2019-12-10 11:27:32 +01:00
Norman Maurer
01e8100496 Prevent any leaks when HttpPostStandardRequestDecoder constructor throws (#9837)
Motivation:

HttpPostStandardRequestDecoder may throw multiple different exceptions in the constructor which could lead to memory leaks. We need to guard against this by explicit catch all of them and rethrow after we released any allocated memory.

Modifications:

- Catch, destroy and rethrow in any case
- Ensure we correctly wrap IllegalArgumentExceptions
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/9829
2019-12-04 13:58:22 +01:00
ursa
f48d9fa8d0 Bugfix #9673: Origin header is always sent from WebSocket client (#9692)
Those who need 'Origin' or 'Sec-WebSocket-Origin' headers should provide them explicitly, like it is stated in WebSocket specs.

E.g. through custom headers:

    HttpHeaders customHeaders = new DefaultHttpHeaders()
        .add(HttpHeaderNames.ORIGIN, "http://localhost:8080");
    new WebSocketClientProtocolHandler(
        new URI("ws://localhost:1234/test"), WebSocketVersion.V13, subprotocol,
        allowExtensions, customHeaders, maxFramePayloadLength, handshakeTimeoutMillis)

* Remove enforced origin headers.
* Update tests

Fixes #9673: Origin header is always sent from WebSocket client
2019-11-27 08:40:57 +01:00
Norman Maurer
f2596fd993 Simplify Deflate* implementations by using EmbeddedChannel.finishAndReleaseAll() (#9808)
Motivation:

We can simplify the code by just using finishAndReleaseAll()

Modifications:

Remove some code and simplify

Result:

Cleaner code
2019-11-27 06:54:52 +01:00
Norman Maurer
e8d72fda5f
Fix tests failures introduced by bad cherry-pick of 7ff8cde66f (#9795)
Motivation:

7ff8cde66f introduced some tests which needs some small adjustments for master.

Modifications:

Add explicit casts

Result:

master builds again without test failures
2019-11-22 08:54:25 +01:00
ursa
7ff8cde66f Send close frame on channel close, when this frame was not send manually (#9745)
Motivation:
By default CloseWebSocketFrames are handled automatically.
However I need manually manage their sending both on client- and on server-sides.

Modification:
Send close frame on channel close automatically, when it was not send before explicitly.

Result:
No more messages like "Connection closed by remote peer" for normal close flows.
2019-11-18 20:32:45 +01:00
Nick Hill
7df012884f Rename SimpleChannelInboundHandler.channelRead0() to messageReceived() (#8819)
Motivation

Per javadoc in 4.1.x SimpleChannelInboundHandler:

"Please keep in mind that channelRead0(ChannelHandlerContext, I) will be
renamed to messageReceived(ChannelHandlerContext, I) in 5.0."

Modifications

Rename aforementioned method and all references/overrides.

Result

Method is renamed.
2019-11-01 07:23:07 +01:00
ursa
db84735975 Simplify WebSocket handlers constructor arguments hell #9698 (#9699)
### Motivation:

Introduction of `WebSocketDecoderConfig` made our server-side code more elegant and simpler for support.

However there is still some problem with maintenance and new features development for WebSocket codecs (`WebSocketServerProtocolHandler`, `WebSocketServerProtocolHandler`).

Particularly, it makes me ~~crying with blood~~ extremely sad to add new parameter and yet another one constructor into these handlers, when I want to contribute new feature.

### Modification:

I've extracted all parameters for client and server WebSocket handlers into config/builder structures, like it was made for decoders in PR #9116.

### Result:

* Fixes #9698: Simplify WebSocket handlers constructor arguments hell
* Unblock further development in this module (configurable close frame handling on server-side; automatic close-frame sending, when missed; memory leaks on protocol violations; etc...)

Bonuses:

* All defaults are gathered in one place and could be easily found/reused.
* New API greatly simplifies usage, but does NOT allow inheritance or modification.
* New API would simplify long-term maintenance of WebSockets module.

### Example

    WebSocketClientProtocolConfig config = WebSocketClientProtocolConfig.newBuilder()
        .webSocketUri("wss://localhost:8443/fx-spot")
        .subprotocol("trading")
        .handshakeTimeoutMillis(15000L)
        .build();
    ctx.pipeline().addLast(new WebSocketClientProtocolHandler(config));
2019-10-29 20:48:44 +01:00
Norman Maurer
6667b590c4 HttpClientCodec need to keep request / response pairs in sync all the… (#9721)
Motivation:

At the moment we miss to poll the method queue when we see an Informational response code. This can lead to out-of-sync of request / response pairs when later try to compare these.

Modifications:

Always poll the queue correctly

Result:

Always compare the correct request / response pairs
2019-10-29 19:48:58 +01:00
Anuraag Agrawal
404008decf Respect all informational status codes. (#9712)
Motivation:

HTTP 102 (WebDAV) is not correctly treated as an informational response

Modification:

Delegate all `1XX` status codes to superclass, not just `100` and `101`.

Result:

Supports WebDAV response.
Removes a huge maintenance [headache](https://github.com/line/armeria/pull/2210) in Armeria which has forked the class for these features
2019-10-28 14:22:04 +01:00
Esteban Ginez
77accfcc53 Adds DeflateDecoder to native-image.properties of codec-http (#9708)
Motivation:
DeflateDecoder was found to be needed when building spring examples app
with graalvm's native image: https://github.com/spring-projects-experimental/spring-graal-native

Modification:
Adds extra native-image.properties to code-http package

Result:
Both:
https://github.com/spring-projects-experimental/spring-graal-native/tree/master/spring-graal-native-samples/spring-petclinic-jpa
and 
https://github.com/spring-projects-experimental/spring-graal-native/tree/master/spring-graal-native-samples/webflux-netty
Build and run
2019-10-25 20:15:54 +02:00
Norman Maurer
af132384cc Support semicolons in query parameters as explain in the W3C recommentation (#9701)
Motivation:

Support semicolons in query parameters as explain in the W3C recommentation:
https://www.w3.org/TR/2014/REC-html5-20141028/forms.html#url-encoded-form-data

Modification:

- Add a new constructor arg that can be used to "switch" modes for decoding ;
- Add unit test

Result:

Fixes #8855
2019-10-24 13:14:13 +02:00
Andrey Mizurov
b17371c198 Fix indexOutOfBoundsException when multipart/form-data is incorrect value (#9688)
Motivation:

HttpPostRequestDecoder.splitHeaderContentType() throws a StringIndexOutOfBoundsException when it parses a Content-Type header that starts with a semicolon ;. We should skip the execution for incorrect multipart form data.


Modification:

Avoid invocation of HttpPostRequestDecoder#splitHeaderContentType(...) for incorrect multipart form data content-type.

Result:

Fixes #8554
2019-10-23 09:03:40 +02:00
Jonathan Leitschuh
cde6a6d7d1 [DOC] Add CWE-113 warning to DefaultHttpHeaders constructor (#9646)
### Motivation:

I've now found two libraries that use Netty to be vulnerable to [CWE-113: Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting')](https://cwe.mitre.org/data/definitions/113.html) due to using `new DefaultHttpHeaders(false)`.

Some part of me hopes that this warning will help dissuade library authors from disabling this important security check.

### Modification:

Add documentation to `DefaultHttpHeaders(boolean)` to warn about the implications of `false`.

### Result:

This improves the documentation on `DefaultHttpHeaders`.
2019-10-10 20:47:50 +02:00
康智冬
1c69448e2e Fix typos in javadocs (#9527)
Motivation:

We should have correct docs without typos

Modification:

Fix typos and spelling

Result:

More correct docs
2019-10-09 15:25:41 +02:00
liyixin
f81d02bdc7 Fix incorrect comment (#9598)
Motivation:

The comment is incorrect and so missleading

Modification:

Correct the comment

Result:

Correct comment in code
2019-09-24 10:01:20 +02:00
liyixin
5c19c3e6ce Optimize the QueryStringEncoder performance (#9568)
Motivation:

Optimize the QueryStringEncoder for lower memory overhead and higher encode speed.

Modification:

Encode the space to + directly, and reuse the uriStringBuilder rather then create a new one.

Result:

Improved performance
2019-09-20 21:11:07 +02:00
Norman Maurer
017a9658c9 Correctly handle whitespaces in HTTP header names as defined by RFC7230#section-3.2.4 (#9585)
Motivation:

When parsing HTTP headers special care needs to be taken when a whitespace is detected in the header name.

Modifications:

- Ignore whitespace when decoding response (just like before)
- Throw exception when whitespace is detected during parsing
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/9571
2019-09-20 21:03:16 +02:00
Andrey Mizurov
f26840478a Fix HttpContentEncoder does not handle multiple Accept-Encoding (#9557)
Motivation:
At the current moment HttpContentEncoder handle only first value of multiple accept-encoding headers.

Modification:

Join multiple accept-encoding headers to one separated by comma.

Result:

Fixes #9553
2019-09-11 08:50:43 +02:00
Norman Maurer
48634f1466
Protect ChannelHandler from reentrancee issues (#9358)
Motivation:

At the moment it is quite easy to hit reentrance issues when you have multiple handlers in the pipeline and each of the handlers does not correctly protect against these. To make it easier for the user we should try to protect from these. The issue is usually if and inbound event will trigger and outbound event and this outbound event then against triggeres an inbound event. This may result in having methods in a ChannelHandler re-enter some method and so state can be corrupted or messages be re-ordered.

Modifications:

- Keep track of inbound / outbound operations in DefaultChannelHandlerContext and if reentrancy is detected break it by scheduling the action on the EventLoop. This will then be picked up once the method returns and so the reentrancy is broken up.
- Adjust tests which made strange assumptions about execution order

Result:

No more reentrancy of handlers possible.
2019-09-03 10:28:08 +02:00
Norman Maurer
64eb392d70 HttpPostStandardRequestDecoder leaks memory when constructor throws ErrorDataDecoderException. (#9517)
Motivation:

Currently when HttpPostStandardRequestDecoder throws a ErrorDataDecoderException during construction we leak memory. We need to ensure all is released correctly.

Modifications:

- Call destroy() if parseBody() throws and rethrow the ErrorDataDecoderException
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9513.
2019-08-28 10:33:44 +02:00
Andrey Mizurov
88e247ee45 Fix sending an empty String like "" causes an error #9429 (#9512)
Motivation:

Handle https://tools.ietf.org/html/rfc7692#section-7.2.3.6

Result:

The empty buffer is correctly handled in deflate  encoder/decoder.

 Fixes #9429 .
2019-08-28 08:22:18 +02:00
hengyunabc
13575be12f Correctly pass all parameters in WebSocketServerProtocolHandler constructor (#9506)
Motivation:

We did not correctly pass all supplied parameters to the called constructor and so did not apply the timeout.

Modification:

Correctly pass on the parameters.

Result:

Use timeout
2019-08-27 09:01:13 +02:00
Idel Pivnitskiy
05bbecff49 Update links to the latest HTTP/2 specifications (#9493)
Motivation:

Some of the links in javadoc point to the obsolete drafts of HTTP/2
specifications. We should point them to the latest RFC 7540 or 7541.

Modifications:

Update links from `draft-ietf-httpbis-*` to the `rfc7540` and `rfc7541`.

Result:

Correct links in javadoc.
2019-08-22 13:59:44 +02:00
Idel Pivnitskiy
dd07caec6b Use AppendableCharSequence.charAtUnsafe(int) in HttpObjectDecoder (#9492)
Motivation:

`HttpObjectDecoder` pre-checks that it doesn't request characters
outside of the `AppendableCharSequence`'s length. `0` is always allowed
because the minimal length of `AppendableCharSequence` is `1`. We can
legally skip index check by using
`AppendableCharSequence.charAtUnsafe(int)` in all existing cases in
`HttpObjectDecoder`.

Modifications:

- Use `AppendableCharSequence.charAtUnsafe(int)` instead of
`AppendableCharSequence.charAt(int)` in `HttpObjectDecoder`.

Result:

No unnecessary index checks in `HttpObjectDecoder`.
2019-08-22 13:58:41 +02:00
Norman Maurer
2abaf50570 Fix compile error introduced by 67b851209f 2019-08-21 10:40:36 +02:00
Sergey S. Sergeev
6af0ecc795 Fix unexpected IllegalReferenceCountException on decode multipart request. (#8575)
Motivation:

Http post request may be encoded as 'multipart/form-data' without any files and consist mixed attributes only.

Modifications:

- Do not double release attributes
- Add unit test

Result:

Code does not throw an IllegalReferenceCountException.
2019-08-19 15:10:13 +02:00
Norman Maurer
67b851209f Ensure we replace WebSocketServerProtocolHandshakeHandler before doing the handshake (#9472)
Motivation:

We need to ensure we replace WebSocketServerProtocolHandshakeHandler before doing the actual handshake as the handshake itself may complete directly and so forward pending bytes through the pipeline.

Modifications:

Replace the handler before doing the actual handshake.

Result:

Fixes https://github.com/netty/netty/issues/9471.
2019-08-17 10:00:14 +02:00
Norman Maurer
6f616bb3cf Avoid creating FileInputStream and FileOutputStream for obtaining Fil… (#8110)
Motivation:

If all we need is the FileChannel we should better use RandomAccessFile as FileInputStream and FileOutputStream use a finalizer.

Modifications:

Replace FileInputStream and FileOutputStream with RandomAccessFile when possible.

Result:

Fixes https://github.com/netty/netty/issues/8078.
2019-08-17 09:52:16 +02:00
Norman Maurer
fc33d6ae99 Fix compile error in test introduced by 96f92929ab 2019-08-16 10:11:03 +02:00
Norman Maurer
96f92929ab Fix possible NPE when using HttpClientCodec (#9465)
Motivation:

It was possible to produce a NPE when we for examples received more responses as requests as we did not check if the queue did not contain a method before trying to compare method names.

Modifications:

- Add extra null check
- Add unit tet

Result:

Fixes https://github.com/netty/netty/issues/9459
2019-08-16 08:17:33 +02:00
Norman Maurer
cdb92c5b0f Correctly respect mask parameters in all WebSocketClientHandshakerFactory#newHandshaker(...) methods (#9464)
Motivation:

We did not correctly pass the mask parameters in all cases.

Modifications:

Correctly pass on parameters

Result:

Fixes https://github.com/netty/netty/issues/9463.
2019-08-15 08:33:54 +02:00
Andrey Mizurov
4340f6c116 Set the ORIGIN header from a custom headers if present (#9435)
Motivation:

Allow to set the ORIGIN header value from custom headers in WebSocketClientHandshaker

Modification:

Only override header if not present already

Result:

More flexible handshaker usage
2019-08-11 08:22:31 +02:00
noSim
e8c2986cf1 Fix HttpUtil.getCharset to not throw illegal charset exception (#9439)
Motivation:

If the HttpUtil.getCharset method is called with an illegal charset like
"charset=!illegal!" it throws an IllegalCharsetNameException. But the javadoc
states, that defaultCharset is returned if incorrect header value. Since the
client sending the request sets the header value this should not crash.

Modification:

HttpUtil.getCharset catches the IllegalCharsetNameException and returns the
defualt value.

Result:

HttpUtil.getCharset does not throw IllegalCharsetNameException any more.
2019-08-10 19:16:12 +02:00
Dmitriy Dumanskiy
10ee697557 Allow to turn off Utf8FrameValidator creation for websocket with Bina… (#9417)
…ryWebSocketFrames

Motivation:

`Utf8FrameValidator` is always created and added to the pipeline in `WebSocketServerProtocolHandler.handlerAdded` method. However, for websocket connection with only `BinaryWebSocketFrame`'s UTF8 validator is unnecessary overhead. Adding of `Utf8FrameValidator` could be easily avoided by extending of `WebSocketDecoderConfig` with additional property.

Specification requires UTF-8 validation only for `TextWebSocketFrame`.

Modification:

Added `boolean WebSocketDecoderConfig.withUTF8Validator` that allows to avoid adding of `Utf8FrameValidator` during pipeline initialization.

Result:

Less overhead when using only `BinaryWebSocketFrame`within web socket.
2019-08-03 10:36:54 +00:00
Dmitriy Dumanskiy
aea6c67cfb Decrease the level of logging in WebSocketFrameEncoder/Decoder (#9415)
Motivation:

Our QA servers are spammed with this messages:

13:57:51.560 DEBUG- Decoding WebSocket Frame opCode=1
13:57:51.560 DEBUG- Decoding WebSocket Frame length=4
I think this is too much info for debug level. It is better to move it to trace level.

Modification:

logger.debug changed to logger.trace for WebSocketFrameEncoder/Decoder

Result:

Less messages in Debug mode.
2019-08-03 10:17:52 +00:00
Norman Maurer
e4995be33c Use allocator when constructing ByteBufHolder sub-types or use Unpool… (#9377)
Motivation:

In many places Netty uses Unpooled.buffer(0) while should use EMPTY_BUFFER. We can't change this due to back compatibility in the constructors but can use Unpooled.EMPTY_BUFFER in some cases to ensure we not allocate at all. In others we can directly use the allocator either from the Channel / ChannelHandlerContext or the request / response.

Modification:

- Use Unpooled.EMPTY_BUFFER where possible
- Use allocator where possible

Result:

Fixes #9345 for websockets and http package
2019-07-18 10:36:03 +02:00
Norman Maurer
2ef4b16138 Add websocket encoder / decoder in correct order to the pipeline when HttpServerCodec is used (#9386)
Motivation:

We need to ensure we place the encoder before the decoder when doing the websockets upgrade as the decoder may produce a close frame when protocol violations are detected.

Modifications:

- Correctly place encoder before decoder
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/9300
2019-07-18 10:19:33 +02:00
Emily Littleworth
0f8685ae9b Return null in HttpPostRequestEncoder (#9352)
Motivation:

If the encoded value of a form element happens to exactly hit
the chunk limit (8096 bytes), the post request encoder will
throw a NullPointerException.

Modifications:

Catch the null case and return.

Result:

No NPE.
2019-07-16 13:30:07 +02:00