Commit Graph

1115 Commits

Author SHA1 Message Date
skyguard1
9f4e155e36
Add Zstd.isAvailable() check in ZstdOptions (#11597)
Motivation:

At present, the verification methods of `ZstdOptions` and `BrotliOptions` are not consistent, and the processing methods of `ZstdOptions` and `BrotliOptions` in `HttpContentCompressor` are also inconsistent.
The http2 module does not add zstd-jni dependency, so `ClassNotFoundException` may be thrown

Modification:

Added `Zstd.isAvailable()` check in `ZstdOptions` to be consistent, and added zstd-jni dependency in http2 module

Result:

The verification methods of `ZstdOptions` and `BrotliOptions` are consistent, and `ClassNotFoundException` will not be thrown


Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-08-19 11:10:09 +02:00
Jeremy Kuhn
93484071d6
Fix support for optional encoders errors in HttpContentCompressor (#11582)
Motivation:

- Fix `HttpContentCompressor` errors due to missing optional compressor libraries such as Brotli and Zstd at runtime.
- Improve support for optional encoders by only considering the `CompressionOptions` provided to the constructor and ignoring those for which the encoder is unavailable.

Modification:

The `HttpContentCompressor` constructor now only creates encoder factories for the CompressionOptions passed to the constructor when the encoder is available which must be checked for Brotli and Zstd. In case of Brotli, it is not possible to create BrotliOptions if brotly4j is not available so there's actually nothing to check. In case of Zstd, I had to create class `io.netty.handler.codec.compression.Zstd` similar to `io.netty.handler.codec.compression.Brotli` which is used to check that zstd-jni is availabie at runtime.

The `determineEncoding()` method had to change as well in order to ignore encodings for which there's no `CompressionEncoderFactory` instance.

When the HttpContentCompressor is created using deprecated constructor (ie. with no CompressionOptions), we consider all available encoders.

Result:

Fixes #11581.
2021-08-19 08:40:25 +02:00
DD
056eba4db4
server h2c upgrade fail when request doesn't have connection header (#11569)
__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes #11568
2021-08-12 09:04:18 -07:00
Aayush Atharva
fe7b18ee83
Make variables final (#11548)
Motivation:
We should make variables `final` which are not reinstated again in code to match the code style and makes the code look better.

Modification:
Made couples of variables as `final`.

Result:
Variables marked as `final`.
2021-08-06 09:27:12 +02:00
Aayush Atharva
1ce28e80d1
Remove Unused Imports (#11546)
Motivation:
There are lots of imports which are unused. We should get rid of them to make the code look better,

Modification:
Removed unused imports.

Result:
No unused imports.
2021-08-05 13:54:48 +02:00
Norman Maurer
26cdfe2cde
Add PromiseNotifier static method which takes care of cancel propagation (#11494)
Motivation:

At the moment we not correctly propagate cancellation in some case when we use the PromiseNotifier.

Modifications:

- Add PromiseNotifier static method which takes care of cancellation
- Add unit test
- Deprecate ChannelPromiseNotifier

Result:

Correctly propagate cancellation of operation

Co-authored-by: Nitesh Kant <nitesh_kant@apple.com>
2021-07-21 13:37:32 +02:00
Mads Johannessen
3a41a97b0e
Support large or variable chunk sizes (#11469)
Motivation:

Chunks are splitted up into even smaller chunks when the underlying
buffer's readable bytes are less than the chunk size.

The underlying buffer can be smaller than a chunk size if:

- The chunk size is larger than the maximum plaintext chunk allowed by the TLS RFC,
  see: io.netty.handler.ssl.SslHandler.MAX_PLAINTEXT_LENGTH.

- The chunk sizes are variable in size,
  which may cause Netty guess a buffer size that is smaller than a chunk size.

Modification:

Create a variable in HttpObjectDecoder: ByteBuf chunkedContent

- Initialize chunkedContent in READ_CHUNK_SIZE with chunkSize as buffer size.

- In READ_CHUNKED_CONTENT write bytes into chunkedContent

  - If the remaining chunk size is not 0 and toRead ==maxChunkSize,
    create a chunk using the chunkedContent and add it to the output messages
    before re-initializing chunkedContent with the remaining chunkSize as buffer size.

  - If the remaining chunk size is not 0 and toRead != maxChunkSize,
    return without adding any output messages.

  - If the remaining chunk size is 0,
    create a chunk using the chunkedContent and add it to the output messages;
    set chunkedContent = null and fall-through.

Result:

Support chunk sizes higher than the underlying buffer's readable bytes.

Co-authored-by: Nitesh Kant <nitesh_kant@apple.com>
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-07-20 15:21:26 +02:00
skyguard1
df22356a3a
Refactor HttpContentCompressor using CompressionEncoderFactory (#11480)
Motivation:

The `HttpContentCompressor.beginEncode()` method has too many if else, so consider refactoring

Modification:

Create the corresponding `CompressionEncoderFactory` according to the compression algorithm, remove the if else

Result:

The code of `HttpContentCompressor` is cleaner than the previous implementation

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-07-13 12:25:30 +02:00
Aayush Atharva
6085cd7ab6
Update HttpContentCompressor to pass correct message to ObjectUtil (#11482)
Motivation:
In #11256, We introduced `Iterable` as a parameter but later in review, it was removed. But we forgot to change `compressionOptionsIterable` to just `compressionOptions`.

Modification:
Changed `compressionOptionsIterable` to `compressionOptions`.

Result:
Correct ObjectUtil message
2021-07-13 08:11:01 +02:00
skyguard1
6ce36f1909
Add zstd http content compression support (#11470)
Motivation:

netty needs to support zstd content-encoding http content compression

Modification:

Add ZstdOptions, and modify HttpContentCompressor and CompressorHttp2ConnectionEncoder to support zstd compression

Result:

netty supports zstd http content compression

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-07-12 08:49:04 +02:00
Aayush Atharva
fef761d03e
Introduce BrotliEncoder (#11256)
Motivation:
Currently, Netty only has BrotliDecoder which can decode Brotli encoded data. However, BrotliEncoder is missing which will encode normal data to Brotli encoded data.

Modification:
Added BrotliEncoder and CompressionOption

Result:
Fixes #6899.

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
2021-07-08 11:51:27 +02:00
skyguard1
7825fa8d7a
Add zstd http header value (#11463)
Motivation:

ZSTD has a wide range of uses on the Internet, so should consider adding `application/zstd` HTTP media-type and `zstd` content-encoding, see  https://tools.ietf.org/html/rfc8478

Modification:

Add `application/zstd` HTTP media-type and `zstd` content-encoding

Result:

netty provides `application/zstd` HTTP media-type and `zstd content-encoding` as http headers

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-07-08 11:44:54 +02:00
Kasimir Torri
c97981403d
Improve PerMessageDeflateClientExtensionHandler (#11413)
Motivation:

The `PerMessageDeflateClientExtensionHandler` has the following strange behaviors currently:

* The `requestedServerNoContext` parameter doesn't actually add the `server_no_context_takeover` parameter to the client offer; instead it depends on the requested server window size.
* The handshake will fail if the server responds with a `server_no_context_takeover` parameter and `requestedServerNoContext` is false. According to RFC 7692 (7.1.1.1) the server may do this, and this means that to cover both cases one needs to use two handshakers in the channel pipeline: one with `requestedServerNoContext = true` and one with `requestedServerNoContext = false`.
* The value of the `server_max_window_bits` parameter in the server response is never checked (should be between 8 and 15). And the value of `client_max_window_bits` is checked only in the branch handling the server window parameter.

Modification:

* Add the `server_no_context_takeover` parameter if `requestedServerNoContext` is true.
* Accept a server handshake response which includes the server no context takeover parameter even if we did not request it.
* Check the values of the client and server window size in their respective branches and fail the handshake if they are out of bounds.

Result:

There will be no need to use two handshakers in the pipeline to be lenient in what handshakes are accepted.
2021-07-02 14:47:59 +02:00
Stephane Landelle
801819b359
Fix HttpHeaderValue#XML_HTTP_REQUEST case (#11433)
Motivation:

HTTP header values are case sensitive. The expected value for `x-request-with` header is `XMLHttpRequest`, not `XmlHttpRequest`.

Modification:

Fix constant's case.

Result:

Correct `XMLHttpRequest` HTTP header value.
2021-07-01 08:13:10 +02:00
Nitesh Kant
5a658bb887
HttpUtil#normalizeAndGetContentLength() should handle empty value (#11409)
__Motivation__

`HttpUtil#normalizeAndGetContentLength()` throws `StringIndexOutOfBoundsException` for empty `content-length` values, it should instead throw `IllegalArgumentException` for all invalid values.

__Modification__

- Throw `IllegalArgumentException` if the `content-length` value is empty.
- Add tests

__Result__

Fixes https://github.com/netty/netty/issues/11408
2021-06-23 12:07:16 +02:00
ktqco
f1742c0e43
Accept smaller server_max_window_bits than requested (#11394)
Motivation:

Netty will fail a handshake for the Per-Message Deflate WebSocket
extension if the server response contains a smaller
`server_max_window_bits` value than the client offered.

However, this is allowed by RFC 7692:

> A server accepts an extension negotiation offer with this parameter
> by including the “server_max_window_bits” extension parameter in the
> extension negotiation response to send back to the client with the
> same or smaller value as the offer.

Modifications:

- Allow the server to respond with a smaller value than offered.
- Change the unit tests to test for this.

Result:

The client will not fail when the server indicates it is using a
smaller window size than offered by the client.
2021-06-18 11:45:32 +02:00
Nitesh Kant
625a7a1075
HttpUtil.getCharset() fails for charset in double-quotes (#11373)
__Motivation__

As described in https://github.com/netty/netty/issues/11370 we should support quoted charset values

__Modification__

Modify `HttpUtil.getCharset(CharSequence contentTypeValue, Charset defaultCharset)` to trim the double-quotes if present.

__Result__

`HttpUtil.getCharset()` now supports quoted charsets. Fixes https://github.com/netty/netty/issues/11370
2021-06-09 12:37:23 +02:00
Violeta Georgieva
e69107ceaf
Fix ServerCookieDecoder javadoc (#11372)
Motivation:
When decoding the cookies on the server, the "Cookie" HTTP request header value should be considered.
The "Set-Cookie" HTTP response header is used to send cookies from the server to the user agent.

Modification:
- Specify in javadoc that the "Cookie" HTTP request header value should be considered and
not the "Set-Cookie" HTTP response header value.

Result:
Correct ServerCookieDecoder javadoc
2021-06-07 16:33:31 -07:00
Stuart Douglas
9f8bfa348e
Fix issue if encoding is enabled but not used (#11358)
Motivation:

Fixes an IllegalReferenceCountException

Modification:

Retained the buffers so the encoder works correctly.

Result:

Fixes #11357
2021-06-07 08:52:44 +02:00
skyguard1
6621e4a60f
Add default block in HttpObjectDecoder (#11342)
Motivation:

There should always be a default in switch blocks.

Modification:

Add default

Result:

Code cleanup.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-06-02 08:41:17 +02:00
skyguard1
8d815d55f9
Add default block in HttpClientCodec (#11352)
Motivation:

There should always be a default in switch blocks.

Modification:

Add default

Result:

Code cleanup.

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-06-02 08:40:36 +02:00
skyguard1
a64a7eb39d
Remove useless code (#11339)
Signed-off-by: xingrufei <xingrufei@sogou-inc.com>

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-05-31 15:35:42 +02:00
Frédéric Brégier
103f1d269e
HttpPostMultipartRequestDecoder IndexOutOfBoundsException error (#11335)
Motivation:

When searching for the delimiter, the decoder part within HttpPostBodyUtil
was not checking the left space to check if it could be included or not,
while it should.

Modifications:

Add a check on toRead being greater or equal than delimiterLength before
going within the loop. If the check is wrong, the delimiter is obviously not found.

Add a Junit test to preserve regression.

Result:

No more IndexOutOfBoundsException

Fixes #11334
2021-05-31 08:43:39 +02:00
skyguard1
cfe631ad05
Add default block in DefaultHttpHeaders (#11329)
Motivation:

Every switch block should also have a default case.

Modification:

Add default block in DefaultHttpHeaders to ensure we not fall-through by mistake

Result:

Cleanup

Signed-off-by: xingrufei <xingrufei@sogou-inc.com>
2021-05-28 08:47:53 +02:00
Trustin Lee
3972bacd60
Provide a way to pass through a certain HTTP upgrade request (#11267)
Motivation:

A user might want to handle a certain HTTP upgrade request differently
than what `HttpServerUpgradeHandler` does by default. For example, a
user could let `HttpServerUpgradeHandler` handle HTTP/2 upgrades but
not WebSocket upgrades.

Modifications:

- Added `HttpServerUpgradeHandler.isUpgrade(HttpRequest)` so a user can
  tell `HttpServerUpgradeHandler` to pass the request as it is to the
  next handler.

Result:

- A user can handle a certain upgrade request specially.
2021-05-18 11:42:38 +02:00
Stephane Landelle
400858cd5e
Introduce BrotliDecoder (#10960)
Motivation:

Netty lacks client side support for decompressing Brotli compressed response bodies.

Modification:

* Introduce optional dependency to brotli4j by @hyperxpro. It will be up to the user to provide the brotli4j libraries for the target platform in the classpath. brotli4j is currently available for Linux, OSX and Windows, all for x86 only.
* Introduce BrotliDecoder in codec module
* Plug it onto `HttpContentDecompressor` for HTTP/1 and `DelegatingDecompressorFrameListener` for HTTP/2
* Add test in `HttpContentDecoderTest`
* Add `BrotliDecoderTest` that doesn't extend `AbstractDecoderTest` that looks flaky

Result:

Netty now support decompressing Brotli compressed response bodies.
2021-05-10 15:25:24 +02:00
Frédéric Brégier
e7330490e3
Fix Memory release not correctly in Multipart Decoder (#11188)
Motivation:
2 years ago a change remove the default clearing of all HttpData, whatever
they are disk based or memory based.

A lot of users were probably releasing HttpData directly, so there was no issue.
But now, it seems, and as the Javadoc said, that `decoder.destroy()` shall clean up
also Memory based HttpData, and not only Disk based HttpData as currently.

Change:
- Add in `destroy()` method the necessary code to release if necessary
the underlying Memory based HttpDatas.

- Change one Junit Test (using Mixed, Memory and Disk based factories)
in order to check the correctness of this behavior and to really act
as a handler (releasing buffers or requests).

- Modify one Junit core to check validity when a delimiter is present in the Chunk
but not CRLF/LF (false delimiter), to ensure correctness.

Result:
No more issue on memory leak

Note that still the List and the Map are not cleaned, since they were not
before. No change is done on this, since it could produce backward issue compatibility.

Fix issues #11175 and #11184
2021-04-29 12:13:45 +02:00
Norman Maurer
def8a3f17d
Destroy HttpPostMultipartRequestDecoder if contructor throws (#11207)
Motivation:

We need to call destroy() if the constructor of HttpPostMultipartRequestDecoder throws as otherwise we may leak memory.

Modifications:

- Call destroy() if we throw
- Add unit test

Result:

No more leaks when constructor throws

Co-authored-by: Frederic Bregier <frederic.bregier@waarp.fr>
2021-04-28 12:27:12 +02:00
Norman Maurer
3657805711
Correctly throw ErrorDataDecoderException for errors while decoding (#11198)
Motivation:

We didn't correctly handle the case when no content-type header was found or if the charset was illegal and just did throw a NPE or ICE. We should in both cases throw an ErrorDataDecoderException to reflect what is documented in the javadocs.

Modifications:

- Throw correct exception
- Merge private method into the constructor as it is only used there
- Add unit tests

Result:

Throw expected exceptions on decoding errors
2021-04-27 16:37:37 +02:00
roy
c1b922b224
adjust validation logic when websocket server check starts with '/' (#11191)
Motivation:

When create a WebSocketServerProtocolConfig to check URI path starts from '/',
only '/' or '//subPath' can be passed by the checker,but '/subPath' should be
passed as well

Modifications:

in `WebSocketServerProtocolHandshakeHandler.isWebSocketPath()` treat '/' a special case

Result:
'/subPath' can be passed
2021-04-26 10:32:10 +02:00
Boris Unckel
db2f60c870
Utilize i.n.u.internal.ObjectUtil to assert Preconditions (codec-http) (#11170) (#11187)
Motivation:

NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.

Modifications:

* import static relevant checks
* Replace manual checks with ObjectUtil methods

Result:

All checks needed are done with ObjectUtil, some exception texts are improved.

Fixes #11170
2021-04-23 08:08:28 +02:00
Norman Maurer
697952e3e6
Add WebSocketClientHandshaker / WebSocketServerHandshaker close methods that take ChannelHandlerContext as parameter (#11171)
Motivation:

At the moment we only expose close(...) methods that take a Channel as paramater. This can be problematic as the write will start at the end of the pipeline which may contain ChannelOutboundHandler implementations that not expect WebSocketFrame objects. We should better also support to pass in a ChannelHandlerContext as starting point for the write which ensures that the WebSocketFrame objects will be handled correctly from this position of the pipeline.

Modifications:

- Add new close(...) methods that take a ChannelHandlerContext
- Add javadoc sentence to point users to the new methods.

Result:

Be able to "start" the close at the right position in the pipeline.
2021-04-21 15:16:10 +02:00
Dmitriy Dumanskiy
3b89ac7cf0
Use ThreadLocalRandom instead of Math.random() (#11165)
Motivation:

`ThreadLocalRandom` doesn't cause contention. Also `nextInt()` generates only 4 random bytes while `Math.random()` generates 8 bytes.

Modification:

Replaced `(int) Math.random()` with `PlatformDependent.threadLocalRandom().nextInt()`

Result:

No possible contention when random numbers for WebSockets.
2021-04-19 13:56:16 +02:00
skyguard1
976486f021
[Clean] Remove useless code (#11154)
Motivation:
There is an unused field

Modifications:
Remove useless code

Result:
Code cleanup

Co-authored-by: xingrufei <xingrufei@sogou-inc.com>
2021-04-19 10:09:40 +02:00
Frédéric Brégier
93f021141d
Fix behavior of HttpPostMultipartRequestDecoder for Memory based Factory (#11145)
Motivation:
When Memory based Factory is used, if the first chunk starts with Line Break, the HttpData
is not filled with the current available buffer if the delimiter is not found yet, while it may
add some.

Fix JavaDoc to note potential wrong usage of content() or getByteBuf() if HttpDatais has
a huge content with the risk of Out Of Memory Exception.

Fix JavaDoc to explain how to release properly the Factory, whatever it is in Memory,
Disk or Mixed mode.

Fix issue #11143

Modifications:
First, when the delimiter is not found, instead of searching Line Break from readerIndex(), we should search
from readerIndex() + readableBytes() - delimiter size, since this is the only part where usefull
Line Break could be searched for, except if readableBytes is less than delimiter size (then we search from
readerIndex).

Second, when a Memory HttpData is created, it should be assigned an empty buffer to be
consistent with the other implementations (Disk or Mixed mode).
We cannot change the default behavior of the content() or getByteBuf() of the Memory based HttpData
since the ByteBuf is supposed to be null when released, but not empty.
When a new ByteBuf is added, one more check verifies if the current ByteBuf is empty, and if so, it
is released and replaced by the new one, without creating a new CompositeByteBuf.

Result:
In the tests testBIgFileUploadDelimiterInMiddleChunkDecoderMemoryFactory and related for other modes,
the buffers are starting with a CRLF.
When we offer only the prefix part of the multipart (no data at all), the current Partial HttpData has
an empty buffer.
The first time we offer the data starting with CRLF to the decoder, it now
has a correct current Partial HttpData with a buffer not empty.

The Benchmark was re-run against this new version.

Old Benchmark                                                                       Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  4,037 ± 0,358  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  4,226 ± 0,471  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,875 ± 0,029  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  4,346 ± 0,275  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,044 ± 0,020  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  2,278 ± 0,159  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,174 ± 0,004  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,370 ± 0,065  ops/ms

New Benchmark                                                                       Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  5,604 ± 0,415  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  6,058 ± 0,111  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,914 ± 0,031  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  6,053 ± 0,051  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,636 ± 0,141  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  3,033 ± 0,181  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,178 ± 0,006  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,859 ± 0,189  ops/ms

So +20 to +40% improvement due to not searching for CRLF/LF into the full buffer when no delimiter is found,
but only from the end and delimiter size + 2 (CRLF).
2021-04-16 11:01:15 +02:00
Dmitriy Dumanskiy
aaef54951f
remove unnecessary check in WebSocketFrameDecoder (#11113)
Motivation:

There are some redundant checks and so these can be removed

Modifications:

- First check frameOpcode != OPCODE_PING is removed because the code executed int the branch where frameOpcode  <= 7, while OPCODE_PING is 9.
- Second check frameOpcode != OPCODE_PING is removed because its checked before.

Result:

Code cleanup
2021-03-29 09:01:38 +02:00
Stuart Douglas
4f316b7cbd Fix exception if Response has content (#11093)
Motivation:

If compression is enabled and the HttpResponse also
implements HttpContent (but not LastHttpContent) then
the buffer will be freed to eagerly.

Modification:

I retain the buffer the same way that is done for the LastHttpContent case.

Note that there is another suspicious looking call a few lines above (if beginEncode returns null). I am not sure if this should also be retained.

Result:

Fixes #11092
2021-03-21 14:51:56 +01:00
Bennett Lynch
3f23f59b87
Introduce HttpMessageDecoderResult to expose decoded header size (#11068)
Motivation

The HttpObjectDecoder accepts input parameters for maxInitialLineLength
and maxHeaderSize. These are important variables since both message
components must be buffered in memory. As such, many decoders (like
Netty and others) introduce constraints. Due to their importance, many
users may wish to add instrumentation on the values of successful
decoder results, or otherwise be able to access these values to enforce
their own supplemental constraints.

While users can perhaps estimate the sizes today, they will not be
exact, due to the decoder being responsible for consuming optional
whitespace and the like.

Modifications

* Add HttpMessageDecoderResult class. This class extends DecoderResult
and is intended for HttpMessage objects successfully decoded by the
HttpObjectDecoder. It exposes attributes for the decoded
initialLineLength and headerSize.
* Modify HttpObjectDecoder to produce HttpMessageDecoderResults upon
successfully decoding the last HTTP header.
* Add corresponding tests to HttpRequestDecoderTest &
HttpResponseDecoderTest.

Co-authored-by: Bennett Lynch <Bennett-Lynch@users.noreply.github.com>
2021-03-12 13:49:51 +01:00
Norman Maurer
89c241e3b1
Merge pull request from GHSA-wm47-8v5p-wjpj
Motivation:

As stated by https://tools.ietf.org/html/rfc7540#section-8.1.2.6 we should report a stream error if the content-length does not match the sum of all data frames.

Modifications:

- Verify that the sum of data frames match if a content-length header was send.
- Handle multiple content-length headers and also handle negative values
- Add io.netty.http2.validateContentLength system property which allows to disable the more strict validation
- Add unit tests

Result:

Correctly handle the case when the content-length header was included but not match what is send and also when content-length header is invalid
2021-03-09 08:20:09 +01:00
Bennett Lynch
bf9b90c340
[HttpObjectDecoder] Include hex-value of illegal whitespace char (#11067)
Motivation:

HttpObjectDecoder may throw an IllegalArgumentException if it encounters
a character that Character.isWhitespace() returns true for, but is not
one of the two valid OWS (optional whitespace) values. Such values may
not have friendly or readable toString() values. We should include the
hex value so that the illegal character can always be determined.

Modifications:

Add hex value as well

Result:

Easier to debug 

Co-authored-by: Bennett Lynch <Bennett-Lynch@users.noreply.github.com>
2021-03-09 08:02:26 +01:00
Frédéric Brégier
1529ef1794
Minimize get byte multipart and fix buffer reuse (#11001)
Motivation:
- Underlying buffer usages might be erroneous when releasing them internaly
in HttpPostMultipartRequestDecoder.

2 bugs occurs:
1) Final File upload seems not to be of the right size.
2) Memory, even in Disk mode, is increasing continuously, while it shouldn't.

- Method `getByte(position)` is too often called within the current implementation
of the HttpPostMultipartRequestDecoder.
This implies too much activities which is visible when PARANOID mode is active.
This is also true in standard mode.

Apply the same fix on buffer from HttpPostMultipartRequestDecoder to HttpPostStandardRequestDecoder
made previously.

Finally in order to ensure we do not rewrite already decoded HttpData when decoding
next ones within multipart, we must ensure the buffers are copied and not a retained slice.

Modification:
- Add some tests to check consistency for HttpPostMultipartRequestDecoder.
Add a package protected method for testing purpose only.

- Use the `bytesBefore(...)` method instead of `getByte(pos)` in order to limit the external
access to the underlying buffer by retrieving iteratively the beginning of a correct start
position.
It is used to find both LF/CRLF and delimiter.
2 methods in HttpPostBodyUtil were created for that.

The undecodedChunk is copied when adding a chunk to a DataMultipart is loaded.
The same buffer is also rewritten in order to release the copied memory part.

Result:

Just for note, for both Memory or Disk or Mixed mode factories, the release has to be done as:

      for (InterfaceHttpData httpData: decoder.getBodyHttpDatas()) {
          httpData.release();
          factory.removeHttpDataFromClean(request, httpData);
      }
      factory.cleanAllHttpData();
      decoder.destroy();

The memory used is minimal in Disk or Mixed mode. In Memory mode, a big file is still
in memory but not more in the undecodedChunk but its own buffer (copied).

In terms of benchmarking, the results are:

Original code Benchmark                                                             Mode  Cnt  Score    Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  0,152 ±  0,100  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  0,543 ±  0,218  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,001 ±  0,001  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  0,615 ±  0,070  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  0,114 ±  0,063  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  0,664 ±  0,034  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,001 ±  0,001  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  0,620 ±  0,140  ops/ms

New code Benchmark                                                                  Mode  Cnt  Score   Error   Units
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigAdvancedLevel   thrpt    6  4,037 ± 0,358  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigDisabledLevel   thrpt    6  4,226 ± 0,471  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigParanoidLevel   thrpt    6  0,875 ± 0,029  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderBigSimpleLevel     thrpt    6  4,346 ± 0,275  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighAdvancedLevel  thrpt    6  2,044 ± 0,020  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighDisabledLevel  thrpt    6  2,278 ± 0,159  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighParanoidLevel  thrpt    6  0,174 ± 0,004  ops/ms
HttpPostMultipartRequestDecoderBenchmark.multipartRequestDecoderHighSimpleLevel    thrpt    6  2,370 ± 0,065  ops/ms

In short, using big file transfers, this is about 7 times faster with new code, while
using high number of HttpData, this is about 4 times faster with new code when using Simple Level.
When using Paranoid Level, using big file transfers, this is about 800 times faster with new code, while
using high number of HttpData, this is about 170 times faster with new code.
2021-02-26 14:24:39 +01:00
strugcoder
bb9370f2a2
Simplity some code (#11000)
Motivation:

There was some code that could be simplified.

Modification:

Simplify code.

Result:

Code cleanup
2021-02-11 08:42:01 +01:00
Norman Maurer
c735357bf2 Use Files.createTempFile(...) to ensure the file is created with proper permissions
Motivation:

File.createTempFile(String, String)` will create a temporary file in the system temporary directory if the 'java.io.tmpdir'. The permissions on that file utilize the umask. In a majority of cases, this means that the file that java creates has the permissions: `-rw-r--r--`, thus, any other local user on that system can read the contents of that file.
This can be a security concern if any sensitive data is stored in this file.

This was reported by Jonathan Leitschuh <jonathan.leitschuh@gmail.com> as a security problem.

Modifications:

Use Files.createTempFile(...) which will use safe-defaults when running on java 7 and later. If running on java 6 there isnt much we can do, which is fair enough as java 6 shouldnt be considered "safe" anyway.

Result:

Create temporary files with sane permissions by default.
2021-02-08 11:44:05 +01:00
Norman Maurer
6ce15414ff
Revert HttpPostMultipartRequestDecoder and HttpPostStandardRequestDecoder to e5951d46fc (#10989)
Motivation:

The changes introduced in 1c230405fd did cause various issues while the fix itself is not considered critical. For now it is considered the best to just rollback and investigate more.

Modifications:

- Revert changes done in 1c230405fd (and later) for
the post decoders.
- Ensure we give memory back to the system as soon as possible in a safe manner

Result:

Fixes https://github.com/netty/netty/issues/10973
2021-02-03 20:40:29 +01:00
Stephane Landelle
ccd01934f5
Merge WebSocket extensions, close #10792 (#10956)
Motivation:

We currently append extensions to the user defined "sec-websocket-extensions" headers. This can cause duplicated entries.

Modifications:

* Replace existing `WebSocketExtensionUtil#appendExtension` private helper with a new `computeMergeExtensionsHeaderValue`. User defined parameters have higher precedence.
* Add tests (existing method wasn't tested)
* Reuse code for both client and server side (code was duplicated).

Result:

No more duplicated entries when user defined extensions overlap with the ones Netty generated.
2021-01-22 08:15:56 +01:00
Norman Maurer
60780c647e Revert "Merge WebSocket extensions, close #10792 (#10951)"
This reverts commit 4fbbcf8702.
2021-01-21 14:22:59 +01:00
Stephane Landelle
4fbbcf8702
Merge WebSocket extensions, close #10792 (#10951)
Motivation:

We currently append extensions to the user defined "sec-websocket-extensions" headers. This can cause duplicated entries.

Modifications:

* Replace existing `WebSocketExtensionUtil#appendExtension` private helper with a new `computeMergeExtensionsHeaderValue`. User defined parameters have higher precedence.
* Add tests (existing method wasn't tested)
* Reuse code for both client and server side (code was duplicated).

Result:

No more duplicated entries when user defined extensions overlap with the ones Netty generated.
2021-01-21 13:58:52 +01:00
Aayush Atharva
2b1785458b
Change switch to if (#10880)
Motivation:
switch is used when we have a good amount of cases because switch is faster than if-else. However, we're using only 1 case in switch which can affect performance.

Modification:
Changed switch to if.

Result:
Good code.
2020-12-22 19:25:33 +01:00
James Kleeh
c0674cff29
Fix infinite loop (#10855)
Motivation:

To fix the infinite loop parsing a multipart body.

Modifications:

Modified the loop to use the correct variable.

Result:

Multipart bodies will be parsed correctly again.
2020-12-11 14:15:23 +01:00
Violeta Georgieva
05093de0d6
Enforce status code validation in CloseWebSocketFrame (#10846)
Motivation:

According to specification 1006 status code must not be set as a status code in a
Close control frame by the endpoint. However 1006 status code can be
used in applications to indicate that the connection was closed abnormally.

Modifications:

- Enforce status code validation in CloseWebSocketFrame
- Add WebSocketCloseStatus construction with disabled validation
- Add test

Result:

Fixes #10838
2020-12-07 13:20:08 +01:00