Commit Graph

370 Commits

Author SHA1 Message Date
Norman Maurer
0261e00662
Allow to call AbstractDiskHttpData.setContent(Unpooled.EMPTY_BUFFER) multiple times. (#7890)
Motivation:

It should be possible to call setContent(Unpooled.EMPTY_BUFFER) multiple times just like its possible to do the same with a non empty buffer.

Modifications:

- Correctly reset underlying storage if called multiple times.
- Add tests

Result:

Fixes https://github.com/netty/netty/issues/6418
2018-04-30 08:39:24 +02:00
Gustavo Fernandes
f874a37ecb Fixes NPE in Corshandler for unauthorized prefligt requests (#7865)
Motivation:
NPE in `CorsHandler` if a pre-flight request is done using an Origin header which is not allowed by any `CorsConfig` passed to the handler on creation.

Modifications:
During the pre-flight, check the `CorsConfig` for `null` and handle it correctly by not returning any access-control header

Result:
No more NPE for pre-flight requests with unauthorized origins.
2018-04-13 14:36:45 +02:00
Gustavo Fernandes
76c5f6cd03 Enable per origin Cors configuration (#7800)
Motivation:

Finer granularity when configuring CorsHandler, enabling different policies for different origins.

Modifications:

The CorsHandler has an extra constructor that accepts a List<CorsConfig> that are evaluated sequentially when processing a Cors request

Result:

The changes don't break backwards compatibility. The extra ctor can be used to provide more than one CorsConfig object.
2018-04-11 10:06:13 +02:00
Nikolay Fedorovskikh
587afddb27 Fixes NPE in ClientCookieDecoder
Motivation:
NPE in `ClientCookieDecoder` if cookie starts with comma.

Modifications:
Check `cookieBuilder` for `null` in the return.

Result:
No fails NPE on invalid cookies.
2018-04-05 19:44:08 +02:00
Bryce Anderson
741602050f Don't replace all 'connection' headers when sending h2c upgrade request (#7824)
Motivation:

There may be meaningful 'connection' headers that exist on a request
that is used to attempt a HTTP/1.x upgrade request that will be
clobbered.

Modifications:

HttpClientUpgradeHandler uses the `HttpHeaders.add` instead of
`HttpHeaders.set` when adding the 'upgrade' field.

Result:

Fixes #7823, existing 'connection' headers are preserved.
2018-04-01 19:59:30 +02:00
Bryce Anderson
b309271e49 HttpServerUpgradeHandler shouldn't wait for flush to reshape pipeline
Motivation:

There is a race between both flushing the upgrade response and receiving
more data before the flush ChannelPromise can fire and reshape the
pipeline. Since We have already committed to an upgrade by writing the
upgrade response, we need to be immediately prepared for handling the
next protocol.

Modifications:

The pipeline reshaping logic in HttpServerUpgradeHandler has been moved
out of the ChannelFutureListener attached to the write of the upgrade
response and happens immediately after the writeAndFlush call, but
before the method returns.

Result:

The pipeline is no longer subject to receiving more data before the
pipeline has been reformed.
2018-03-28 19:54:30 +02:00
Stephane Landelle
d60cd0231d HttpProxyHandler generates invalid CONNECT url and Host header when address is resolved
Motivation:

HttpProxyHandler uses `NetUtil#toSocketAddressString` to compute
CONNECT url and Host header.

The url is correct when the address is unresolved, as
`NetUtil#toSocketAddressString` will then use
`getHoststring`/`getHostname`. If the address is already resolved, the
url will be based on the IP instead of the hostname.

There’s an additional minor issue with the Host header: default port
443 should be omitted.

Modifications:

* Introduce NetUtil#getHostname
* Introduce HttpUtil#formatHostnameForHttp to format an
InetSocketAddress to
HTTP format
* Change url computation to favor hostname instead of IP
* Introduce HttpProxyHandler ignoreDefaultPortsInConnectHostHeader
parameter to ignore 80 and 443 ports in Host header

Result:

HttpProxyHandler performs properly when connecting to a resolved address
2018-03-27 09:43:11 +02:00
teaey
06dcca1dbc When the response exceeds the threshold, it will be compressed
Motivation:

When the response is very small, compression will inflate the response.

Modifications:

Add filed io.netty.handler.codec.http.HttpContentCompressor#compressThreshold that control whether the HTTP response should be compressed.

Result:

Fixes #7660.
2018-02-25 16:43:45 +01:00
Matteo Bertozzi
b640797de1 Fix HttpPostMultipartRequestDecoder.splitMultipartHeader() String index out of range: -1 with empty header
Motivation:

A Malformed empty header value (e.g. Content-Type: \r\n) will trigger a String index out of range
while trying to parse the multi-part request, using the HttpPostMultipartRequestDecoder.

Modification:

Ensure that the substring() method is called passing the endValue >= valueStart.
In case of an empty header value, the empty header value associated with the header key will be returned.

Result:

Fixes #7620
2018-01-25 14:03:35 +01:00
Scott Mitchell
4921f62c8a
HttpResponseStatus object allocation reduction
Motivation:
Usages of HttpResponseStatus may result in more object allocation then necessary due to not looking for cached objects and the AsciiString parsing method not being used due to CharSequence method being used instead.

Modifications:
- HttpResponseDecoder should attempt to get the HttpResponseStatus from cache instead of allocating a new object
- HttpResponseStatus#parseLine(CharSequence) should check if the type is AsciiString and redirect to the AsciiString parsing method which may not require an additional toString call
- HttpResponseStatus#parseLine(AsciiString) can be optimized and doesn't require and may not require object allocation

Result:
Less allocations when dealing with HttpResponseStatus.
2018-01-24 22:01:52 -08:00
Ngoc Dao
2b4f667791 Fix DefaultHttpDataFactory cleanup bug
Motivation:

DefaultHttpDataFactory uses HttpRequest as map keys.

Because of the implementation of "hashCode"" and "equals" in DefaultHttpRequest,
if we use normal maps, HttpDatas of different requests may end up in the same map entry,
causing cleanup bug.

Consider this example:
- Suppose that request1 is equal to request2, causing their HttpDatas to be stored in one single map entry.
- request1 is cleaned up first, while request2 is still being decoded.
- Consequently request2's HttpDatas are suddenly gone, causing NPE, or worse loss of data.

This bug can be reproduced by starting the HttpUploadServer example,
then run this command:
ab -T 'application/x-www-form-urlencoded' -n 100 -c 5 -p post.txt http://localhost:8080/form

post.txt file content:
a=1&b=2

There will be errors like this:
java.lang.NullPointerException
        at io.netty.handler.codec.http.multipart.MemoryAttribute.getValue(MemoryAttribute.java:64)
        at io.netty.handler.codec.http.multipart.MixedAttribute.getValue(MixedAttribute.java:243)
        at io.netty.example.http.upload.HttpUploadServerHandler.writeHttpData(HttpUploadServerHandler.java:271)
        at io.netty.example.http.upload.HttpUploadServerHandler.readHttpDataChunkByChunk(HttpUploadServerHandler.java:230)
        at io.netty.example.http.upload.HttpUploadServerHandler.channelRead0(HttpUploadServerHandler.java:193)
        at io.netty.example.http.upload.HttpUploadServerHandler.channelRead0(HttpUploadServerHandler.java:66)
        at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:102)
        at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:310)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:284)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1412)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:943)
        at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:141)
        at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:645)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:580)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:497)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:459)
        at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.lang.Thread.run(Thread.java:748)

Modifications:

Keep identity of requests by using IdentityHashMap

Result:

DefaultHttpDataFactory is fixed.

The ConcurrentHashMap is replaced with a synchronized map, but I think the performance won't be affected much in real web apps.
2017-12-28 07:40:17 +01:00
Norman Maurer
9ef83234f3 Add tests for HttpObjectDecoder related to limits
Motivation:

HttpObjectDecoder will throw a TooLongFrameException when either the max size for the initial line or the header size was exceeed. We have no tests for this.

Modifications:

Add test cases.

Result:

More tests.
2017-11-28 13:38:01 +01:00
Norman Maurer
7aca99f986 DefaultHttpHeader.names().toArray(...) may throw ArrayStoreException
Motivation:

DefaultHttpHeader.names() exposes HTTP header names as a Set<String>. Converting the resulting set to an array using toArray(String[]) throws an exception: java.lang.ArrayStoreException: io.netty.util.AsciiString.

Modifications:

- Remove our custom implementation of toArray(...) (and others) by just extending AbstractCollection.
- Add unit test

Result:

Fixes [#7428].
2017-11-22 19:04:55 +01:00
Stephane Landelle
e420f857fa Optimize DefaultHeaders#toString and implement HttpHeaders#toString
Motivation:

For debugging/logging purpose, it would be convenient to have
HttpHeaders#toString implemented.

DefaultHeaders does implement toString be the implementation is suboptimal and allocates a Set for the names and Lists for values.

Modification:

* Introduce HeadersUtil#toString that provides a convenient optimized helper to implement toString for various headers implementations
* Have DefaultHeaders#toString and HttpHeaders#toString delegate their toString implementation to HeadersUtil

Result:

Convenient HttpHeaders#toString. Optimized DefaultHeaders#toString.
2017-11-22 08:42:59 +01:00
Norman Maurer
7f4ade7e7d Correctly propagate channelInactive even if cleanup throws
Motivation:

Its possible that cleanup() will throw if invalid data is passed into the wrapped EmbeddedChannel. We need to ensure we still call channelInactive(...) in this case.

Modifications:

- Correctly forward Exceptions caused by cleanup()
- Ensure all content is released when cleanup() throws
- Add unit tests

Result:

Correctly handle the case when cleanup() throws.
2017-11-21 11:55:39 +01:00
Norman Maurer
e5e4c18c1b Add testcase for writing empty last content with headers.
Motivation:

https://github.com/netty/netty/issues/7418 reported an issue with writing a LastHttpContent with trailers set.

Modifications:

Add unit test to ensure this issue is fixed in latest netty release.

Result:

Ensure code is correct.
2017-11-21 07:45:58 +01:00
Scott Mitchell
0a47c590fe HttpHeaders valuesIterator and contains improvements
Motivation:
In order to determine if a header contains a value we currently rely
upon getAll(..) and regular expressions. This operation is commonly used
during the encode and decode stage to determine the transfer encoding
(e.g. HttpUtil#isTransferEncodingChunked). This operation requires an
intermediate collection and possibly regular expressions for the
CombinedHttpHeaders use case which can be expensive.

Modifications:
- Add a valuesIterator to HttpHeaders and specializations of this method
for DefaultHttpHeaders, ReadOnlyHttpHeaders, and CombinedHttpHeaders.

Result:
Less intermediate collections and allocation overhead when determining
if HttpHeaders contains a name/value pair.
2017-11-20 08:34:06 -08:00
Dan McNulty
48b4502d1d Move EmptyHttpHeaders.INSTANCE initialization to inner class.
Motivation:

If the HttpUtil class is initialized before HttpHeaders or
EmptyHttpHeaders, EmptyHttpHeaders.INSTANCE will be null. This
can lead to NPEs in code that relies on this field being
non-null. One example is the
LastHttpContent.EMPTY_LAST_CONTENT.trailingHeaders method.

Modifications:

- Move HttpUtil.EMPTY_HEADERS to a private static final inner class
  of EmptyHttpHeaders called InstanceInitializer.
- Add tests, that when run in isolation, validate the fix for the issue.

Result:

Any initialization order of HttpUtil, EmptyHttpHeaders or
HttpHeaders will result in EmptyHttpHeaders.INSTANCE being initialized
correctly.
2017-11-17 16:31:09 +01:00
Violeta Georgieva
72a216625f Correctly handle 205 Reset Content response with transfer-encoding
Motivation:

According to RFC 7231 the server may choose to:
```
indicate a zero-length payload for the response by including a
Transfer-Encoding header field with a value of chunked and a message
body consisting of a single chunk of zero-length
```
https://tools.ietf.org/html/rfc7231#page-53

In such cases the exception below appears during decoding phase:
```
java.lang.IllegalArgumentException: invalid version format: 0
	at io.netty.handler.codec.http.HttpVersion.<init>(HttpVersion.java:121)
	at io.netty.handler.codec.http.HttpVersion.valueOf(HttpVersion.java:76)
	at io.netty.handler.codec.http.HttpResponseDecoder.createMessage(HttpResponseDecoder.java:118)
	at io.netty.handler.codec.http.HttpObjectDecoder.decode(HttpObjectDecoder.java:219)
```

Modifications:

HttpObjectDecoder.isContentAlwaysEmpty specifies content NOT empty
when 205 Reset Content response

Result:

There is no `IllegalArgumentException: invalid version format: 0`
when handling 205 Reset Content response with transfer-encoding
2017-11-14 08:02:09 +01:00
Norman Maurer
3554646a60 Correctly convert empty HttpContent to ByteBuf
Motivation:

93130b172a introduced a regression where we not "converted" an empty HttpContent to ByteBuf and just passed it on in the pipeline. This can lead to the situation that other handlers in the pipeline will see HttpContent instances which is not expected.

Modifications:

- Correctly convert HttpContent to ByteBuf when empty
- Add unit test.

Result:

Handlers in the pipeline will see the expected message type.
2017-11-08 13:46:32 -08:00
Scott Mitchell
8618a3351c ReadOnlyHttpHeaders
Motivation:
For use cases that create headers, but do not need to modify them a read only variant of HttpHeaders would be useful and may be able to provide better iteration performance for encoding.

Modifications:
- Introduce ReadOnlyHttpHeaders that is backed by a flat array

Result:
ReadOnlyHttpHeaders exists for non-modifiable HttpHeaders use cases.
2017-11-06 21:58:16 -08:00
Norman Maurer
e0bbff74f7 Correctly handle WebSockets 00 when using HttpClientCodec.
Motivation:

7995afee8f introduced a change that broke special handling of WebSockets 00.

Modifications:

Correctly delegate to super method which has special handling for WebSockets 00.

Result:

Fixes [#7362].
2017-11-03 15:55:22 +01:00
Scott Mitchell
93130b172a HttpObjectEncoder and MessageAggregator EMPTY_BUFFER usage
Motivation:
HttpObjectEncoder and MessageAggregator treat buffers that are not readable special. If a buffer is not readable, then an EMPTY_BUFFER is written and the actual buffer is ignored. If the buffer has already been released then this will not be correct as the promise will be completed, but in reality the original content shouldn't have resulted in any write because it was invalid.

Modifications:
- HttpObjectEncoder should retain/write the original buffer instead of using EMPTY_BUFFER
- MessageAggregator should retain/write the original ByteBufHolder instead of using EMPTY_BUFFER

Result:
Invalid write operations which happen to not be readable correctly reflect failed status in the promise, and do not result in any writes to the channel.
2017-11-03 07:03:19 +01:00
Piotr Kołaczkowski
7995afee8f Don't disable HttpObjectDecoder on upgrade from HTTP/1.x to HTTP/1.x over TLS
This change allows to upgrade a plain HTTP 1.x connection to TLS
according to RFC 2817. Switching the transport layer to TLS should be
possible without removing HttpClientCodec from the pipeline,
because HTTP/1.x layer of the protocol remains untouched by the switch
and the HttpClientCodec state must be retained for proper
handling the remainder of the response message,
per RFC 2817 requirement in point 3.3:

  Once the TLS handshake completes successfully, the server MUST
  continue with the response to the original request.

After this commit, the upgrade can be established by simply
inserting an SslHandler at the front of the pipeline after receiving
101 SWITCHING PROTOCOLS response, exactly as described in SslHander
documentation.

Modifications:
- Don't set HttpObjectDecoder into UPGRADED state if
  101 SWITCHING_PROTOCOLS response contains HTTP/1.0 or HTTP/1.1 in
  the protocol stack described by the Upgrade header.
- Skip pairing comparison for 101 SWITCHING_PROTOCOLS, similar
  to 100 CONTINUE, since 101 is not the final response to the original
  request and the final response is expected after TLS handshake.

Fixes #7293.
2017-10-29 13:21:11 +01:00
Dmitry Minkovsky
8aeba78ecc HttpPostMultipartRequestDecoder should decode header field parameters
Motivation:

I am receiving a multipart/form_data upload from a Mailgun webhook. This webhook used to send parts like this:

--74e78d11b0214bdcbc2f86491eeb4902
Content-Disposition: form-data; name="attachment-2"; filename="attached_�айл.txt"
Content-Type: text/plain
Content-Length: 32

This is the content of the file

--74e78d11b0214bdcbc2f86491eeb4902--
but now it posts parts like this:

--74e78d11b0214bdcbc2f86491eeb4902
Content-Disposition: form-data; name="attachment-2"; filename*=utf-8''attached_%D1%84%D0%B0%D0%B9%D0%BB.txt

This is the content of the file

--74e78d11b0214bdcbc2f86491eeb4902--
This new format uses field parameter encoding described in RFC 5987. More about this encoding can be found here.

Netty does not parse this format. The result is the filename is not decoded and the part is not parsed into a FileUpload.

Modification:

Added failing test in HttpPostRequestDecoderTest.java and updated HttpPostMultipartRequestDecoder.java
Refactored to please Netkins
Result:

Fixes:

HttpPostMultipartRequestDecoder identifies the RFC 5987 format and parses it.
Previous functionality is retained.
2017-10-24 19:30:59 +02:00
Ned Twigg
dcbbae7f90 Added QueryStringDecoder.rawPath() and rawQuery()
Motivation:

Before this commit, it is impossible to access the path component of the
URI before it has been decoded.  This makes it impossible to distinguish
between the following URIs:

/user/title?key=value
/user%2Ftitle?key=value

The user could already access the raw uri value, but they had to calculate
pathEndIdx themselves, even though it might already be cached inside
QueryStringDecoder.

Result:

The user can easily and efficiently access the undecoded path and query.
2017-10-24 09:32:06 +02:00
Nikolay Fedorovskikh
dc98eae5a5 Correct filling an origin header for WS client
Motivation:
An `origin`/`sec-websocket-origin` header value in websocket client is filling incorrect in some cases:
- Hostname is not converting to lower-case as prescribed by RFC 6354 (see [1]).
- Selecting a `http` scheme when source URI has `wss`/`https` scheme and non-standard port.

Modifications:
- Convert uri-host to lower-case.
- Use a `https` scheme if source URI scheme is `wss`/`https`, or if source scheme is null and port == 443.

Result:
Correct filling an `origin` header for WS client.

[1] https://tools.ietf.org/html/rfc6454#section-4
2017-10-23 11:38:34 +02:00
Idel Pivnitskiy
50a067a8f7 Make methods 'static' where it possible
Motivation:

Even if it's a super micro-optimization (most JVM could optimize such
 cases in runtime), in theory (and according to some perf tests) it
 may help a bit. It also makes a code more clear and allows you to
 access such methods in the test scope directly, without instance of
 the class.

Modifications:

Add 'static' modifier for all methods, where it possible. Mostly in
test scope.

Result:

Cleaner code with proper 'static' modifiers.
2017-10-21 14:59:26 +02:00
Matt Belisle
ad548a6a0a Fixing CorsHandler response Content-Length
Motivation:

https://github.com/netty/netty/issues/7253

Modifications:

Adding `Content-Length: 0` to `CorsHandler.forbidden()` and `CorsHandler.handlePreflight()`

Result:

Contexts that are terminated by the CorsHandler will always include a Content-Length header
2017-10-02 08:20:15 +02:00
Norman Maurer
70c5c48eab Correctly not write any body when 1xx, 204 or 304 is used as response status code.
Motivation:

We need to ensure we not write any body when a response with status code of 1xx, 204 or 304 is used as stated in rfc:
https://tools.ietf.org/html/rfc7230#section-3.3.3

Modifications:

- Correctly handle status codes
- Add unit tests

Result:

Correctly handle responses with 1xx, 204, 304 status codes.
2017-09-20 07:41:13 -07:00
Daniel Schobel
52e19d5c63 Strip http 'expect' headers when expectation response is produced
Motivation:

HttpObjectAggregator differs from HttpServerExpectContinueHandler's handling
of expect headers by not stripping the 'expect' header when a response
is generated.

Modifications:

HttpObjectAggregator now removes the 'expect' header in cases where it generates
a response.

Result:

Consistent and correct behavior between HttpObjectAggregator and HttpServerExpectContinueHandler.
2017-08-12 17:18:27 +02:00
Norman Maurer
918fde82f8 Add testcases to prove HttpResponseEncoder correctly handles empty content
Motivation:

Issue #6695 states that there is an issue when writing empty content via HttpResponseEncoder.

Modifications:

Add two test-cases.

Result:

Verified that all works as expected.
2017-08-07 07:25:33 +02:00
Norman Maurer
d141ba11bf Fix flacky multipart test introduced by 08748344d8.
Motivation:

08748344d8 introduced two new tests which did not take into account that the multipart delimiter can be between 2 and 16 bytes long.

Modifications:

Take the multipart delimiter length into account.

Result:

Fixes [#7001]
2017-07-21 14:28:32 +02:00
Violeta Georgieva
08748344d8 Fix NPEs in HttpPostRequestEncoder#nextChunk
Motivation:

HttpPostRequestEncoder maintains an internal buffer that holds the
current encoded data. There are use cases when this internal buffer
becomes null, the next chunk processing implementation should take
this into consideration.

Modifications:

- When preparing the last chunk if currentBuffer is null, mark
isLastChunkSent as true and send LastHttpContent.EMPTY_LAST_CONTENT
- When calculating the remaining size take into consideration that the
currentBuffer might be null
- Tests are based on those provided in the issue by @nebhale and @bfiorini

Result:

Fixes #5478
2017-07-19 14:35:51 +02:00
Nikolay Fedorovskikh
df568c739e Use ByteBuf#writeShort/writeMedium instead of writeBytes
Motivation:

1. Some encoders used a `ByteBuf#writeBytes` to write short constant byte array (2-3 bytes). This can be replaced with more faster `ByteBuf#writeShort` or `ByteBuf#writeMedium` which do not access the memory.
2. Two chained calls of the `ByteBuf#setByte` with constants can be replaced with one `ByteBuf#setShort` to reduce index checks.
3. The signature of method `HttpHeadersEncoder#encoderHeader` has an unnecessary `throws`.

Modifications:

1. Use `ByteBuf#writeShort` or `ByteBuf#writeMedium` instead of `ByteBuf#writeBytes` for the constants.
2. Use `ByteBuf#setShort` instead of chained call of the `ByteBuf#setByte` with constants.
3. Remove an unnecessary `throws` from `HttpHeadersEncoder#encoderHeader`.

Result:

A bit faster writes constants into buffers.
2017-07-10 14:37:41 +02:00
Dmitriy Dumanskiy
dd69a813d4 Performance improvement for HttpRequestEncoder. Insert char into the string optimized.
Motivation:

Right now HttpRequestEncoder does insertion of slash for url like http://localhost?pararm=1 before the question mark. It is done not effectively.

Modification:

Code:

new StringBuilder(len + 1)
                .append(uri, 0, index)
                .append(SLASH)
                .append(uri, index, len)
                .toString();
Replaced with:

new StringBuilder(uri)
                .insert(index, SLASH)
                .toString();
Result:

Faster HttpRequestEncoder. Additional small test. Attached benchmark in PR.

Benchmark                                      Mode  Cnt        Score        Error  Units
HttpRequestEncoderInsertBenchmark.newEncoder  thrpt   40  3704843.303 ±  98950.919  ops/s
HttpRequestEncoderInsertBenchmark.oldEncoder  thrpt   40  3284236.960 ± 134433.217  ops/s
2017-06-27 10:53:43 +02:00
Jason Tedor
9ad74e72e6 Remove content-length header leniency
Motivation:

If the content-length does not parse as a number, leniency causes this
to instead be parsed as the default value. This leads to bodies being
silently ignored on requests which can be incredibly dangerous. Instead,
if the content-length header is invalid, an exception should be thrown
for upstream handling.

Modifications:

This commit removes the leniency in parsing the content-length header by
allowing a number format exception, if thrown, to escape from the method
rather than falling back to the default value.

Result:

In invalid content-length header will not be silently ignored.
2017-06-22 09:20:11 -07:00
Dmitriy Dumanskiy
81f9434ec7 Added test for multi header, HttpObjectDecoder performance improvement for multi header, removed empty else block.
Motivation:

For multi-line headers HttpObjectDecoder uses StringBuilder.append(a).append(b) pattern that could be easily replaced with regular a + b. Also oparations with a and b moved out from concat operation to make it friendly for StringOptimizeConcat optimization and thus - faster.

Modification:

StringBuilder.append(a).append(b) reaplced with a + b. Operations with a and b moved out from concat oparation.

Result:
Code simpler to read and faster.
2017-06-20 07:11:02 +02:00
Renjie Sun
629b83e0a5 Move QueryStringDecoder.decodeHexByte into ByteBufUtil
Motivations:
1. There are duplicated implementations of decoding hex strings. #6797
2. ByteBufUtil.HexUtil.decodeHexDump does not handle substring start
index properly and does not decode hex byte rigorously.

Modifications:
1. Function decodeHexByte is moved from QueryStringDecoder into ByteBufUtil.
2. ByteBufUtil.HexUtil.decodeHexDump is changed to use decodeHexByte.
3. Tests are Updated accordingly.

Result:
Fixed #6797 and made hex decoding functions more robust.
2017-06-07 09:27:36 -07:00
Bryce Anderson
9fa3e556f3 Adjust Content-Length header when encoding Full Responses
Motivation:
If a full HttpResponse with a Content-Length header is encoded by the HttpContentEncoder subtypes the Content-Length header is removed and the message is set to Transfer-Encoder: chunked. This is an unnecessary loss of information about the message content.

Modifications:
- If a full HttpResponse has a Content-Length header, the header is adjusted after encoding.

Result:
Complete messages continue to have the Content-Length header after encoding.
2017-06-06 22:07:29 +02:00
Nikolay Fedorovskikh
270e9d66c5 Fixes in QueryStringDecoder
Motivation:

QueryStringDecoder has several problems:
- doesn't decode correctly path part with `+` (plus) sign in it,
- doesn't cut a `fragment` (after `#`) from query string (see RFC 3986),
- doesn't work correctly with encoding,
- treat `%%` as a percent character escaping (it's don't described in RFC).

Modifications:

- leave `+` chars in a `path` part of uri string,
- ignore `fragment` part (after `#`),
- correctly work with encoding.
- don't treat `%%` as escaping for the `%`.

Result:

Fixed issues from #6745.
2017-05-31 13:54:56 -07:00
Nolan O'Brien
d56a7560ea Netty force encodes already encoded responses
Motivation:

Fix the regression recently introduced that causes already encoded responses to be encoded again as gzip

Modification:

instead of just looking for IDENTITY, anything set for Content-Encoding should be respected and left as-is

added unit tests to capture this use case

Result:

Fixes #6784
2017-05-27 08:26:14 +02:00
Nitesh Kant
a093b89bfe Allow HTTP decoding post CONNECT in HttpClientCode
__Motivation__

`HttpClientCodec` skips HTTP decoding on the connection after a successful HTTP CONNECT response is received.
 This behavior follows the spec for a client but pragmatically, if one creates a client to use a proxy transparently, the codec becomes useless after HTTP CONNECT.
 Ideally, one should be able to configure whether HTTP CONNECT should result in pass-through or not. This will enable client writers to continue using HTTP decoding even after HTTP CONNECT.

 __Modification__

 Added overloaded constructors to accept `parseHttpPostConnect`. If this parameter is `true` then the codec continues decoding even after a successful HTTP CONNECT.

 Also fixed a bug in the codec that was incrementing request count post HTTP CONNECT but not decrementing it on response. Now, the request count is only incremented if the codec is not `done`.

 __Result__

 Easier usage by HTTP client writers who wants to connect to a proxy but still decode HTTP for their users for subsequent requests.
2017-05-11 16:48:16 +02:00
Nikolay Fedorovskikh
94e9448ae3 Simplify JUnit assertions
Motivation:

Some JUnit assert calls can be replaced by simpler.

Modifications:

Replacement with a more suitable methods.

Result:

More informative JUnit reports.
2017-05-09 20:19:10 +02:00
Dmitriy Dumanskiy
174f4ea005 HttpServerKeepAliveHandler doesn't correctly handle VoidChannelPromise
Motivation:

HttpServerKeepAliveHandler throws unexpected error when I do ctx.writeAndFlush(msg, ctx.voidPromise()); where msg is with header "Connection:close".

Modification:

HttpServerKeepAliveHandler does promise.unvoid() before adding close listener.

Result:

No error for VoidChannelPromise with HttpServerKeepAliveHandler. Fixes [#6698].
2017-05-04 14:08:18 -07:00
Dmitry Spikhalskiy
464ae9fb7a Expose CharSequence version of HttpUtil#getMimeType and HttpUtil#getCharset
Motivation:

It would be more flexible to make getCharset and getMimeType code usable not only for HttpMessage entity but just for any CharSequence. This will improve usability in general purpose code and will help to avoid multiple fetching of ContentType header from a message. It could be done in an external code once and CharSequence method versions could be applied.

Modification:
Expose HttpUtil#getMimeType, HttpUtil#getCharsetAsString, HttpUtil#getCharset versions which works with CharSequence. New methods are reused in the old ones which work with HttpMessage entity.

Result:

More flexible methods set with a good code reusing.
2017-05-03 14:14:39 -07:00
Norman Maurer
a3e496a521 Not try to compresses HttpMessage if IDENTITY header value is set.
Motivation:

If Content-Encoding: IDENTITY is used we should not try to compress the http message but just let it pass-through.

Modifications:

Remove "!"

Result:

Fixes [#6689]
2017-05-03 10:55:13 -07:00
Daniel Schobel
b1cb059540 Motivation:
It is generally useful to have origin http servers respond to
"expect: continue-100" as soon as possible but applications without a
HttpObjectAggregator in their pipelines must use boiler plate to do so.

Modifications:

Introduce the HttpServerExpectContinueHandler handler to make it easier.

Result:

Less boiler plate for http application authors.
2017-04-27 16:20:29 -07:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Brendt Lucas
dcd322dda2 Fix regression in QueryStringEncoder
Motivation:

Commit #d675febf07d14d4dff82471829f974369705655a introduced a regression in QueryStringEncoder, resulting in whitespace being converted into a literal `+` sign instead of `%20`.

Modification:

Modify `encodeComponent` to pattern match and replace on the result of the call to `URLEncoder#encode`

Result:

Fixes regression
2017-04-17 19:32:50 +02:00