Commit Graph

1373 Commits

Author SHA1 Message Date
Bennett Lynch
f7fa9fce72 Add option to HttpObjectDecoder to allow duplicate Content-Lengths (#10349)
Motivation:

Since https://github.com/netty/netty/pull/9865 (Netty 4.1.44) the
default behavior of the HttpObjectDecoder has been to reject any HTTP
message that is found to have multiple Content-Length headers when
decoding. This behavior is well-justified as per the risks outlined in
https://github.com/netty/netty/issues/9861, however, we can see from the
cited RFC section that there are multiple possible options offered for
responding to this scenario:

> If a message is received that has multiple Content-Length header
> fields with field-values consisting of the same decimal value, or a
> single Content-Length header field with a field value containing a
> list of identical decimal values (e.g., "Content-Length: 42, 42"),
> indicating that duplicate Content-Length header fields have been
> generated or combined by an upstream message processor, then the
> recipient MUST either reject the message as invalid or replace the
> duplicated field-values with a single valid Content-Length field
> containing that decimal value prior to determining the message body
> length or forwarding the message.

https://tools.ietf.org/html/rfc7230#section-3.3.2

Netty opted for the first option (rejecting as invalid), which seems
like the safest, but the second option (replacing duplicate values with
a single value) is also valid behavior.

Modifications:

* Introduce "allowDuplicateContentLengths" parameter to
HttpObjectDecoder (defaulting to false).
* When set to true, will allow multiple Content-Length headers only if
they are all the same value. The duplicated field-values will be
replaced with a single valid Content-Length field.
* Add new parameterized test class for testing different variations of
multiple Content-Length headers.

Result:

This is a backwards-compatible change with no functional change to the
existing behavior.

Note that the existing logic would result in NumberFormatExceptions
for header values like "Content-Length: 42, 42". The new logic correctly
reports these as IllegalArgumentException with the proper error message.

Additionally note that this behavior is only applied to HTTP/1.1, but I
suspect that we may want to expand that to include HTTP/1.0 as well...
That behavior is not modified here to minimize the scope of this change.
2020-07-06 14:50:15 +02:00
Norman Maurer
f8b3a388ff
Fix compile errors introduced by bad cherry-picks (#10391) 2020-07-06 13:22:31 +02:00
feijermu
0185ccc157 Fix a javadoc mistake. (#10364)
Motivation:

There exists a `javadoc` mistake in `HttpHeaderValues.java`.

Modification:

Just correct this `javadoc` mistake...
2020-06-23 09:24:48 +02:00
Norman Maurer
4dd6f82624 Fix memory leak in AbstractDiskHttpData when CompositeByteBuf is used (#10360)
Motivation:

AbstractDiskHttpData may cause a memory leak when a CompositeByteBuf is used. This happened because we may call copy() but actually never release the newly created ByteBuf.

Modifications:

- Remove copy() call and just use ByteBuf.getBytes(...) which will internally handle the writing to the FileChannel without any extra copies that need to be released later on.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10354
2020-06-22 13:55:02 +02:00
Bennett Lynch
f945cfbd66 Consolidate HttpObjectDecoder default values into constants (#10344)
Motivation

HttpObjectDecoder and its associated classes make frequent use of
default values for maxInitialLineLength, maxHeaderSize, maxChunkSize,
etc. Today, these defaults are defined in-line in constructors and
duplicated across many classes. This repetition is more prone to error
and inconsistencies.

Furthermore, due to the current lack of builder support, if a user wants
to change just one of these values (e.g., maxHeaderSize), they are also
required to know and repeat the other default values (e.g.,
maxInitialLineLength and maxChunkSize).

The primary motivation for this change is as we are considering adding
another constructor parameter (for multiple content length behavior),
appending this parameter may require some users to have prior knowledge
of the default initialBufferSize, and it would be cleaner to allow them
to reference the default constant.

Modifications

* Consolidate the HttpObjectDecoder default values into public constants
* Reference these constants where possible

Result

No functional change. Additional telescoping constructors will be easier
and safer to write. Users may have an easier experience changing single
parameters.
2020-06-12 08:44:39 +02:00
Lin Gao
4b6ceb2ca0 More values other than chunked defined in Transfer-Encoding header leads to decode failure (#10321)
Motivation:

`containsValue()` will check if there are multiple values defined in the specific header name, we need to use this method instead of `contains()` for the `Transfer-Encoding` header to cover the case that multiple values defined, like: `Transfer-Encoding: gzip, chunked`

Modification:

Change from `contains()` to `containsValue()` in `HttpUtil.isTransferEncodingChunked()` method.

Result:

Fixes #10320
2020-06-02 14:38:11 +02:00
Andrey Mizurov
4c7e017199 Set (and override) reserved websocket handshake response headers after custom to avoid duplication (#10319)
Motivation:
Currently we passing custom websocket handshaker response headers to a `WebSocketServerHandshaker` but they can contain a reserved headers (e.g. Connection, Upgrade, Sec-Websocket-Accept) what lead to duplication because we use response.headers().add(..) instead of response.headers().set(..).

Modification:
In each `WebSocketServerHandshaker00`, ... `WebSocketServerHandshaker13` implementation replace the method add(..) to set(..) for reserved response headers.

Result:

Less error-prone
2020-06-02 11:56:47 +02:00
prgitpr
e7c5773162 Fix a potential fd leak in AbstractDiskHttpData.getChunk (#10270)
Motivation:

`FileChannel.read()` may throw an IOException. We must deal with this in case of the occurrence of `I/O` error.

Modification:

Place the `FileChannel.read()` method call in the `try-finally` block.

Result:

Advoid fd leak.


Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2020-05-14 10:16:33 +02:00
Fabien Renaud
3e8cbb3136 Fix regression in HttpPostStandardRequestDecoder to always decode + to whitespace (#10285)
Motivations
-----------
HttpPostStandardRequestDecoder was changed in 4.1.50 to provide its own
ByteBuf UrlDecoder. Prior to this change, it was using the decodeComponent
method from QueryStringDecoder which decoded + characters to
whitespaces. This behavior needs to be preserved to maintain backward
compatibility.

Modifications
-------------
Changed HttpPostStandardRequestDecoder to detect + bytes and decode them
toe whitespaces. Added a test.

Results
-------
Addresses issues#10284
2020-05-14 09:29:13 +02:00
Norman Maurer
7f54c6d90d Don't reuse ChannelPromise in WebSocketProtocolHandler (#10248)
Motivation:

We cant reuse the ChannelPromise as it will cause an error when trying to ful-fill it multiple times.

Modifications:

- Use a new promise and chain it with the old one
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10240
2020-05-07 09:14:02 +02:00
Dmitriy Dumanskiy
a81858c520 …in order to minimize pipeline
Motivation:

Handling of `WebSocketCloseFrame` is part of websocket protocol, so it's logical to put it within the `WebSocketProtocolHandler`. Also, removal of `WebSocketCloseFrameHandler` will decrease the channel pipeline.

Modification:

- `WebSocketCloseFrameHandler` code merged into `WebSocketProtocolHandler`. `WebSocketCloseFrameHandler` not added to the pipeline anymore
- Added additional constructor to `WebSocketProtocolHandler`
- `WebSocketProtocolHandler` now implements `ChannelOutboundHandler` and implements basic methods from it

Result:

`WebSocketCloseFrameHandler` is no longer used.

Fixes https://github.com/netty/netty/issues/9944
2020-05-07 09:13:50 +02:00
Norman Maurer
a62fcd9d50 Reuse the same allocator as used by the ByteBuf that is used during… (#10226)
Motivation:

We should not use Unpooled to allocate buffers if possible to ensure we can make use of pooling etc.

Modifications:

- Only allocate a buffer if really needed
- Use the ByteBufAllocator of the offered ByteBuf
- Ensure we not use buffer.copy() but explicitly allocate a buffer and then copy into it to not hit the limit of maxCapacity()

Result:

Improve allocations
2020-04-29 15:37:51 +02:00
Norman Maurer
23e0b878af Remove some debugging cruft (#10229)
Motivation:

RtspDecoderTest did include a println(...) call which was a left over from debugging.

Modifications:

Remove println(...)

Result:

Cleanup
2020-04-29 11:35:53 +02:00
Norman Maurer
65a967c772 Fix memory leak in HttpPostMultipartRequestDecoder (#10227)
Motivation:

We need to release all ByteBufs that we allocate to prevent leaks. We missed to release the ByteBufs that are used to aggregate in two cases

Modifications:

Add release() calls

Result:

No more memory leak in HttpPostMultipartRequestDecoder
2020-04-29 08:23:55 +02:00
Fabien Renaud
8fe9c013dd HttpPostRequestDecoder: retain instead of copy when first buf is last (#10209)
Motivations
-----------
There is no need to copy the "offered" ByteBuf in HttpPostRequestDecoder
when the first HttpContent ByteBuf is also the last (LastHttpContent) as
the full content can immediately be decoded. No extra bookeeping needed.

Modifications
-------------
HttpPostMultipartRequestDecoder
 - Retain the first ByteBuf when it is both the first HttpContent offered
to the decoder and is also LastHttpContent.
 - Retain slices of the final buffers values

Results
-------
ByteBufs of FullHttpMessage decoded by HttpPostRequestDecoder are no longer
unnecessarily copied. Attributes are extracted as retained slices when
the content is multi-part. Non-multi-part content continues to return
Unpooled buffers.

Partially addresses issue #10200
2020-04-28 09:43:19 +02:00
feijermu
076f388d72 Move up the size check in AbstractDiskHttpData.setContent. (#10222)
Motivation:

`AbstractHttpData.checkSize` may throw an IOException if we set the max size limit via `AbstractHttpData.setMaxSize`. However, if this exception happens, the `AbstractDiskHttpData.file` and the `AbstractHttpData.size` are still be modified. In other words, it may break the failure atomicity here.

Modification:

Just move up the size check.

Result:

Keep the failure atomicity even if `AbstractHttpData.checkSize` fails.
2020-04-28 09:42:30 +02:00
feijermu
a1f23dbdd3 Fix a potential fd leak in AbstractDiskHttpData.delete (#10212)
Motivation:

An unexpected IOException may be thrown from `FileChannel.force`. If it happens, the `FileChannel.close` may not be invoked.

Modification:

Place the `FileChannel.close` in a finally block.

Result:

Avoid fd leak.
2020-04-27 07:04:10 +02:00
feijermu
586eca2809 Fix a potential fd leak in AbstractDiskHttpData.setContent (#10198)
Motivation:

`RandomAccessFile.setLength` may throw an IOException. We must deal with this in case of the occurrence of `I/O` error.

Modification:

Place the `RandomAccessFile.setLength` method call in the `try-finally` block.

Result:

Avoid fd leak.
2020-04-21 11:08:50 +02:00
feijermu
a4ad6d15cd Close the FileChannel in case of an IOException in AbstractDiskHttpData.addContent. (#10188)
Motivation:

`FileChannel.force` may throw an IOException. A fd leak may happen here.

Modification:

Close the fileChannel in a finally block.

Result:

Avoid fd leak.
2020-04-15 09:25:07 +02:00
feijermu
3808777c32 Close the file in case of an IOException in AbstractMemoryHttpData.renameTo. (#10163)
Motivation:

An `IOException` may be thrown from `FileChannel.write` or `FileChannel.force`, and cause the fd leak.

Modification:

Close the file in a finally block.

Result:

Avoid fd leak.
2020-04-06 14:16:30 +02:00
feijermu
a2a10b9931 Close the file when IOException occurs in AbstractMemoryHttpData. (#10157)
Motivation:

An IOException may be thrown from FileChannel.read, and cause the fd leak.

Modification:

Close the file when IOException occurs.

Result:
Avoid fd leak.
2020-04-02 15:07:07 +02:00
Romain Manni-Bucau
10dfd36030 making DefaultHttpDataFactory able to configure basedir and deleteonexit (#10146)
Motivation: 

currently (http) disk based attributes or uploads are globally configured in a single directory and can also only globally be deleted on exit or not. it does not fit well multiple cases, in particular the case you have multiple servers in the same JVM.

Modification: 
make it configurable per attribute/fileupload.

Result:

This PR duplicates Disk* constructor to add basedir and deleteonexit parameters and wires it in default http daa factory.
2020-03-30 13:12:06 +02:00
feijermu
92562f90d3 Release the ByteBuf when IOException occurs in AbstractMemoryHttpData. (#10133)
Motivation:

An IOException may be thrown from InputStream.read or checkSize method, and cause the ByteBuf leak.

Modification:

Release the ByteBuf when IOException occurs.

Result:
Avoid ByteBuf leak.
2020-03-30 11:40:18 +02:00
Stephane Landelle
cb8fdd6c73 Add some HTTP header constants (#10127)
Motivation:

Add some missing HTTP header names and values constants.

Modification:

* names:
  * dnt (Do Not Track)
  * upgrade-insecure-requests
  * x-requested-with
* values:
  * application/xhtml+xml
  * application/xml
  * text/css
  * text/html
  * text/event-stream
  * XmlHttpRequest

Result:

More constants available
2020-03-23 13:07:06 +01:00
Norman Maurer
e6e681f37d HttpObjectDecoder should limit the number of control chars (#10112)
Motivation:

At the moment HttpObjectDecoder does not limit the number of controls chars. These should be counted with the initial line and so a limit should be exposed

Modifications:

- Change LineParser to also be used when skipping control chars and so enforce a limit
- Add various tests to ensure that limit is enforced

Result:

Fixes https://github.com/netty/netty/issues/10111
2020-03-17 10:41:10 +01:00
Norman Maurer
b595ceab49 Use WebSocketVersion.toAsciiString() as header value when possible (#10105)
Motivation:

In our WebSocketClientHandshaker* implementations we "hardcode" the version number to use. This is error-prone, we should better use the WebSocketVersion so we dont need to maintain the value multiple times. Beside this we can also use an AsciiString to improve performance

Modifications:

- Use WebSocketVersion.toAsciiString

Result:

Less stuff to maintain and small performance win
2020-03-13 10:07:37 +01:00
Norman Maurer
b1a9d923ce Don't override HOST header if provided by user already in WebSocketClientHandshaker (#10104)
Motivation:

The user may need to provide a specific HOST header. We should not override it when specified during handshake.

Modifications:

Check if a custom HOST header is already provided by the user and if so dont override it

Result:

Fixes https://github.com/netty/netty/issues/10101
2020-03-12 20:26:22 +01:00
David Latorre
fa397c6127 Added support for the SameSite attribute in Cookies (#10050)
Motivation:

Netty currently does not support the SameSite attribute for response cookies (see issue #8161 for discussion).

Modifications:

The attribute has been added to the DefaultCookie class as a quick fix since adding new methods to the Cookie interface would be backwards-incompatible.
ServerCookieEncoder and ClientCookieDecoder have been updated accordingly to process this value. No validation for allowed values (Lax, None, Strict) has been implemented.

Result:

Response cookies with the SameSite attribute set can be read or written by Netty.

Co-authored-by: David Latorre <a-dlatorre@hotels.com>
2020-03-12 09:57:06 +01:00
Stephane Landelle
d639f55764 Fix WebSocketClientHandshaker not generating correct handshake request when path is empty (#10095)
Motivation:

WebSocketClientHandshaker#upgradeUrl doesn't comperly compute relative url when path is empty and produces url such as `?access_token=foo` instead of `/?access_token=foo`.

Modifications:

* fix WebSocketClientHandshaker#upgradeUrl
* add tests for urls without path, with and without query

Result:

WebSocketClientHandshaker properly connects to url without path.
2020-03-10 15:20:12 +01:00
zlm0125
c88d320230 http multipart decode with chinese chars should work (#10089)
Motivation:

I am receiving a mutlipart/form_data upload from postman. The filename contains Chinese, and so some invalid chars. We should ensure all of these are removed before trying to decode.

Modification:

Ensure all invalid characters are removed

Result:

Fixes #10087

Co-authored-by: liming.zhang <liming.zhang@luckincoffee.com>
2020-03-06 10:34:44 +01:00
Norman Maurer
ae0fbb45e4
Ensure the DefaultChannelHandlerContext is unlinked once removed (#9970)
Motivation:

At the moment the next / prev references are not set to "null" in the DefaultChannelHandlerContext once the ChannelHandler is removed. This is bad as it basically let users still use the ChannelHandlerContext of a ChannelHandler after it is removed and may produce very suprising behaviour.

Modifications:

- Fail if someone tries to use the ChannelHandlerContext once the ChannelHandler was removed (for outbound operations fail the promise, for inbound fire the error through the ChannelPipeline)
- Fix some handlers to ensure we not use the ChannelHandlerContext after the handler was removed
- Adjust DefaultChannelPipeline / DefaultChannelHandlerContext to fixes races with removal / replacement of handlers

Result:

Cleanup behaviour and make it more predictable for pipeline modifications
2020-03-01 08:13:33 +01:00
Norman Maurer
4a07f1cd10 More strict parsing of initial line / http headers (#10058)
Motivation:

Our parsing of the initial line / http headers did treat some characters as separators which should better trigger an exception during parsing.

Modifications:

- Tighten up parsing of the inital line by follow recommentation of RFC7230
- Restrict separators to OWS for http headers
- Add unit test

Result:

Stricter parsing of HTTP1
2020-02-26 10:01:41 +01:00
Norman Maurer
064ab7afa8 Remove System.out.println(...) in test (#10024)
Motivation:

We did had some System.out.println(...) call in a test which seems to be some left-over from debugging.

Modifications:

Remove System.out.println(...)

Result:

Code cleanup
2020-02-13 08:43:09 +01:00
Bennett Lynch
f4d1df9c57 Remove "Content-Length" when decoding HTTP/1.1 message with both "Tra… (#10003)
Motivation

As part of a recent commit for issue
https://github.com/netty/netty/issues/9861 the HttpObjectDecoder was
changed to throw an IllegalArgumentException (and produce a failed
decoder result) when decoding a message with both "Transfer-Encoding:
chunked" and "Content-Length".

While it seems correct for Netty to try to sanitize these types of
messages, the spec explicitly mentions that the Content-Length header
should be *removed* in this scenario.

Both Nginx 1.15.9 and Tomcat 9.0.31 also opt to remove the header:
b693d7c198/java/org/apache/coyote/http11/Http11Processor.java (L747-L755)
0ad4393e30/src/http/ngx_http_request.c (L1946-L1953)

Modifications

* Change the default behavior from throwing an IllegalArgumentException
to removing the "Content-Length" header
* Extract the behavior to a new protected method,
handleChunkedEncodingWithContentLength(), that can be overridden to
change this behavior (or capture metrics)

Result

Messages of this nature will now be successfully decoded and have their
"Content-Length" header removed, rather than creating invalid messages
(decoder result failures). Users will be allowed to override and
configure this behavior.
2020-02-10 10:43:38 +01:00
Artem Smotrakov
f760b6af84 Added tests for Transfer-Encoding header with whitespace (#9997)
Motivation:

Need tests to ensure that CVE-2020-7238 is fixed.

Modifications:

Added two test cases into HttpRequestDecoderTest which check that
no whitespace is allowed before the Transfer-Encoding header.

Result:

Improved test coverage for #9861
2020-02-05 14:34:14 +01:00
Norman Maurer
6a43807843
Use lambdas whenever possible (#9979)
Motivation:

We should update our code to use lamdas whenever possible

Modifications:

Use lambdas when possible

Result:

Cleanup code for Java8
2020-01-30 09:28:24 +01:00
Dmitriy Dumanskiy
6b6782ea01 Use compile time constants instead of status field in WebSocketServer/ClientProtocolConfig (#9976)
Motivation:

Avoid allocation of default static `WebSocketServerProtocolConfig` and `WebSocketClientProtocolConfig` configs. Prefer compile time constants instead.

Modification:

Static field with config object replaced with constructor with default fields.

Result:

No more default config allocation and static field for it. Compile time variables used instead.
2020-01-29 15:29:50 +01:00
Norman Maurer
b19958eda9 Remove usage of forbiddenHttpRequestResponder (#9941)
Motivation:

At the moment we add a handler which will respond with 403 forbidden if a websocket handshake is in progress (and after). This makes not much sense as it is unexpected to have a remote peer to send another http request when the handshake was started. In this case it is much better to let the websocket decoder bail out.

Modifications:

Remove usage of forbiddenHttpRequestResponder

Result:

Fixes https://github.com/netty/netty/issues/9913
2020-01-28 06:11:28 +01:00
Norman Maurer
9e29c39daa
Cleanup usage of Channel*Handler (#9959)
Motivation:

In next major version of netty users should use ChannelHandler everywhere. We should ensure we do the same

Modifications:

Replace usage of deprecated classes / interfaces with ChannelHandler

Result:

Use non-deprecated code
2020-01-20 17:47:17 -08:00
Andrey Mizurov
91404e1828 Fix remove 'WebSocketServerExtensionHandler' from pipeline after upgrade (#9940)
Motivation:

We should remove WebSocketServerExtensionHandler from pipeline after successful WebSocket upgrade even if the client has not selected any extensions.

Modification:

Remove handler once upgrade is complete and no extensions are used.

Result:

Fixes #9939.
2020-01-15 12:12:51 +01:00
Norman Maurer
961362f43f Utf8FrameValidator must release buffer when validation fails (#9909)
Motivation:

Utf8FrameValidator must release the input buffer if the validation fails to ensure no memory leak happens

Modifications:

- Catch exception, release frame and rethrow
- Adjust unit test

Result:

Fixes https://github.com/netty/netty/issues/9906
2019-12-27 09:16:07 +01:00
Anuraag Agrawal
ee206b6ba8 Separate out query string encoding for non-encoded strings. (#9887)
Motivation:

Currently, characters are appended to the encoded string char-by-char even when no encoding is needed. We can instead separate out codepath that appends the entire string in one go for better `StringBuilder` allocation performance.

Modification:

Only go into char-by-char loop when finding a character that requires encoding.

Result:

The results aren't so clear with noise on my hot laptop - the biggest impact is on long strings, both to reduce resizes of the buffer and also to reduce complexity of the loop. I don't think there's a significant downside though for the cases that hit the slow path.

After
```
Benchmark                                     Mode  Cnt   Score   Error   Units
QueryStringEncoderBenchmark.longAscii        thrpt    6   1.406 ± 0.069  ops/us
QueryStringEncoderBenchmark.longAsciiFirst   thrpt    6   0.046 ± 0.001  ops/us
QueryStringEncoderBenchmark.longUtf8         thrpt    6   0.046 ± 0.001  ops/us
QueryStringEncoderBenchmark.shortAscii       thrpt    6  15.781 ± 0.949  ops/us
QueryStringEncoderBenchmark.shortAsciiFirst  thrpt    6   3.171 ± 0.232  ops/us
QueryStringEncoderBenchmark.shortUtf8        thrpt    6   3.900 ± 0.667  ops/us
```

Before
```
Benchmark                                     Mode  Cnt   Score    Error   Units
QueryStringEncoderBenchmark.longAscii        thrpt    6   0.444 ±  0.072  ops/us
QueryStringEncoderBenchmark.longAsciiFirst   thrpt    6   0.043 ±  0.002  ops/us
QueryStringEncoderBenchmark.longUtf8         thrpt    6   0.047 ±  0.001  ops/us
QueryStringEncoderBenchmark.shortAscii       thrpt    6  16.503 ±  1.015  ops/us
QueryStringEncoderBenchmark.shortAsciiFirst  thrpt    6   3.316 ±  0.154  ops/us
QueryStringEncoderBenchmark.shortUtf8        thrpt    6   3.776 ±  0.956  ops/us
```
2019-12-20 08:51:26 +01:00
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