Commit Graph

1277 Commits

Author SHA1 Message Date
Norman Maurer
ee9057ad99
CorsHandler.write(...) should not cause a flush. (#7839)
Motivation:

Unnecessary flushes reduce the amount of flush coalescing that can happen at higher levels and thus can increase number of packets (because of TCP_NODELAY) and lower throughput (due to syscalls, TLS frames, etc)

Modifications:

Replace writeAndFlush(...) with write(...)

Result:

Fixes https://github.com/netty/netty/issues/7837.
2018-04-03 21:11:51 +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
Norman Maurer
69582c0b6c [maven-release-plugin] prepare for next development iteration 2018-02-21 12:52:33 +00:00
Norman Maurer
786f35c6c9 [maven-release-plugin] prepare release netty-4.1.22.Final 2018-02-21 12:52:19 +00:00
Shohei Kamimori
73f23c5faa Fix typos in docs.
Motivation:

There are same typos in the docs.

Modifications:

Fix typos. Docs only changing.

Result:

More correct docs.
2018-02-14 08:44:07 +01:00
Norman Maurer
e71fa1e7b6 [maven-release-plugin] prepare for next development iteration 2018-02-05 12:02:35 +00:00
Norman Maurer
41ebb5fcca [maven-release-plugin] prepare release netty-4.1.21.Final 2018-02-05 12:02:19 +00: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
Norman Maurer
ea58dc7ac7 [maven-release-plugin] prepare for next development iteration 2018-01-21 12:53:51 +00:00
Norman Maurer
96c7132dee [maven-release-plugin] prepare release netty-4.1.20.Final 2018-01-21 12:53:34 +00: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
942b993f2b Only enable validation of headers if original headers were validating as well.
Motiviation:

In our replace(...) methods we always used validation for the newly created headers while the original headers may not use validation at all.

Modifications:

- Only use validation if the original headers used validation as well.
- Ensure we create a copy of the headers in replace(...).

Result:

Fixes [#5226]
2017-12-21 07:32:29 +01:00
Norman Maurer
264a5daa41 [maven-release-plugin] prepare for next development iteration 2017-12-15 13:10:54 +00:00
Norman Maurer
0786c4c8d9 [maven-release-plugin] prepare release netty-4.1.19.Final 2017-12-15 13:09:30 +00:00
Norman Maurer
b2bc6407ab [maven-release-plugin] prepare for next development iteration 2017-12-08 09:26:15 +00:00
Norman Maurer
96732f47d8 [maven-release-plugin] prepare release netty-4.1.18.Final 2017-12-08 09:25:56 +00:00
Tomasz Jędrzejewski
e8540c2b7a Adding stable JDK9 module names that follow reverse-DNS style
Automatic-Module-Name entry provides a stable JDK9 module name, when Netty is used in a modular JDK9 applications. More info: http://blog.joda.org/2017/05/java-se-9-jpms-automatic-modules.html

When Netty migrates to JDK9 in the future, the entry can be replaced by actual module-info descriptor.

Modification:

The POM-s are configured to put the correct module names to the manifest.

Result:

Fixes #7218.
2017-11-29 11:50:24 +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
93b144b7b4 HttpMethod#valueOf improvement
Motivation:
HttpMethod#valueOf shows up on profiler results in the top set of
results. Since it is a relatively simple operation it can be improved in
isolation.

Modifications:
- Introduce a special case map which assigns each HttpMethod to a unique
index in an array and provides constant time lookup from a hash code
algorithm. When the bucket is matched we can then directly do equality
comparison instead of potentially following a linked structure when
HashMap has hash collisions.

Result:
~10% improvement in benchmark results for HttpMethod#valueOf

Benchmark                                     Mode  Cnt   Score   Error   Units
HttpMethodMapBenchmark.newMapKnownMethods    thrpt   16  31.831 ± 0.928  ops/us
HttpMethodMapBenchmark.newMapMixMethods      thrpt   16  25.568 ± 0.400  ops/us
HttpMethodMapBenchmark.newMapUnknownMethods  thrpt   16  51.413 ± 1.824  ops/us
HttpMethodMapBenchmark.oldMapKnownMethods    thrpt   16  29.226 ± 0.330  ops/us
HttpMethodMapBenchmark.oldMapMixMethods      thrpt   16  21.073 ± 0.247  ops/us
HttpMethodMapBenchmark.oldMapUnknownMethods  thrpt   16  49.081 ± 0.577  ops/us
2017-11-20 11:07:50 -08: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
188ea59c9d [maven-release-plugin] prepare for next development iteration 2017-11-08 22:36:53 +00:00
Norman Maurer
812354cf1f [maven-release-plugin] prepare release netty-4.1.17.Final 2017-11-08 22:36:33 +00: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
Cory Benfield
1b0a545921 Do not send Content-Length: 0 on 101 responses.
Motivation:

During code read of the Netty codebase I noticed that the Netty
HttpServerUpgradeHandler unconditionally sets a Content-Length: 0
header on 101 Switching Protocols responses. This explicitly
contravenes RFC 7230 Section 3.3.2 (Content-Length), which notes
that:

    A server MUST NOT send a Content-Length header field in any
    response with a status code of 1xx (Informational) or 204
    (No Content).

While it is unlikely that any client will ever be confused by
this behaviour, there is no reason to contravene this part of the
specification.

Modifications:

Removed the line of code setting the header field and changed the
only test that expected it to be there.

Result:

When performing the server portion of HTTP upgrade, the 101
Switching Protocols response will no longer contain a
Content-Length: 0 header field.
2017-10-21 14:36:19 +02:00
Stephane Landelle
940f15f0d2 Use predefined HttpResponseStatus constant instead of hardcoded 101 code
Motivation:

#7269 removed an unnecessary instanciation for verifying WebSocket
handshake status code.
But it uses a hardcoded status code value for 101 instead of using the
intended `HttpResponseStatus#SWITCHING_PROTOCOLS` constant.

Modidication:

Compare actual `HttpResponseStatus` against predefined constant. Note
that `HttpResponseStatus#equals` is implemented in respect with the RFC
(only honor code, not text) so it’s intended to be used this way.

Result:

Cleaner code, use intended constant instead of hard coded value.
2017-10-02 18:58:12 +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
Nikolay Fedorovskikh
78786e6052 Remove unnecessary instantiation of HttpResponseStatus
Motivation:
- In the `HttpResponseStatus#equals` checks only status code. No need to create new instance of `HttpResponseStatus` for comparison with response status.
- The RFC says: `the HTTP version and reason phrase aren't important` [1].

Modifications:
Use comparison by status code without creating new `HttpResponseStatus`.

Result:
Less allocations, more clear code.

[1] https://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-00
2017-10-02 08:08:34 +02:00
Norman Maurer
625a7426cd [maven-release-plugin] prepare for next development iteration 2017-09-25 06:12:32 +02:00
Norman Maurer
f57d8f00e1 [maven-release-plugin] prepare release netty-4.1.16.Final 2017-09-25 06:12:16 +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
Scott Mitchell
9bd6d8129e HttpObjectEncoder buffer size estimation
Motivation:
HttpObjectEncoder allocates a new buffer when encoding the initial line and headers, and also allocates a buffer when encoding the trailers. The allocation always uses the default size of 256. This may lead to consistent under allocation and require a few resize/copy operations which can cause GC/memory pressure.

Modifications:
- Introduce a weighted average which tracks the historical size of encoded data and uses this as an estimate for future buffer allocations

Result:
Better approximation of buffer sizes.
2017-08-31 01:40:53 -07:00
Stephane Landelle
d9d0e633dc Fix ServerCookieEncoder javadoc, close #7115
Motivation:

ServerCookieEncoder’s javadoc contains some invalid copy-pasting from
ClientCookieEncoder.

Modifications:
* As per RFC6265, multiple cookies are sent as separate Set-Cookie
response headers.
* Fix code sample

Result:

Proper javadoc
2017-08-28 20:21:57 +02:00
Norman Maurer
b967805f32 [maven-release-plugin] prepare for next development iteration 2017-08-24 15:38:22 +02:00
Norman Maurer
da8e010a42 [maven-release-plugin] prepare release netty-4.1.15.Final 2017-08-24 15:37:59 +02:00
Norman Maurer
123e07ca80 Revert "Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message."
This reverts commit d63bb4811e as this not covered correctly all cases and so could lead to missing fireChannelReadComplete() calls. We will re-evalute d63bb4811e and resbumit a pr once we are sure all is handled correctly
2017-08-18 09:06:37 +02:00
Nikolay Fedorovskikh
4875a2aad4 Immediate caching the strings wrapped to AsciiString
Motivation:
The `AsciiString#toString` method calculate string value and cache it into field. If an `AsciiString` created from the `String` value, we can avoid rebuilding strings if we cache them immediately when creating `AsciiString`. It would be useful for constants strings, which already stored in the JVMs string table, or in cases where an unavoidable `#toString `method call is assumed.

Modifications:
- Add new static method `AsciiString#cache(String)` which save string value into cache field.
- Apply a "benign" data race in the `#hashCode` and `#toString` methods.

Result:
Less memory usage in some `AsciiString` use cases.
2017-08-15 06:22:14 +02: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
d63bb4811e Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message.
Motivation:

Its wasteful and also confusing that channelReadComplete() is called even if there was no message forwarded to the next handler.

Modifications:

- Only call ctx.fireChannelReadComplete() if at least one message was decoded
- Add unit test

Result:

Less confusing behavior. Fixes [#4312].
2017-08-04 10:54:56 +02:00
Norman Maurer
52f384b37f [maven-release-plugin] prepare for next development iteration 2017-08-02 12:55:10 +00:00
Norman Maurer
8cc1071881 [maven-release-plugin] prepare release netty-4.1.14.Final 2017-08-02 12:54:51 +00: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
Scott Mitchell
a91df58ca1 HTTP/2 enforce HTTP message flow
Motivation:
codec-http2 currently does not strictly enforce the HTTP/1.x semantics with respect to the number of headers defined in RFC 7540 Section 8.1 [1]. We currently don't validate the number of headers nor do we validate that the trailing headers should indicate EOS.

[1] https://tools.ietf.org/html/rfc7540#section-8.1

Modifications:
- DefaultHttp2ConnectionDecoder should only allow decoding of a single headers and a single trailers
- DefaultHttp2ConnectionEncoder should only allow encoding of a single headers and optionally a single trailers

Result:
Constraints of RFC 7540 restricting the number of headers/trailers is enforced.
2017-07-19 13:37:23 -07: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
3e9f617504 Deduplicate and simplify code in HttpPostMultipartRequestDecoder
Motivation:

- A `HttpPostMultipartRequestDecoder` contains two pairs of the same methods: `readFileUploadByteMultipartStandard`+`readFileUploadByteMultipart` and `loadFieldMultipartStandard`+`loadFieldMultipart`.
- These methods use `NotEnoughDataDecoderException` to detecting not last data chunk (exception handling is very expensive).
- These methods can be greatly simplified.
- Methods `loadFieldMultipart` and `loadFieldMultipartStandard` has an unnecessary catching for the `IndexOutOfBoundsException`.

Modifications:

- Remove duplicate methods.
- Replace handling `NotEnoughDataDecoderException` by the return of a boolean result.
- Simplify code.

Result:

The code is cleaner and easier to support. Less exception handling logic.
2017-07-18 13:25:12 +02:00
Norman Maurer
f1e14d0cb2 Only add / to uri if really needed.
Motivation:

We not need to include the start index in the check. See https://github.com/netty/netty/pull/6924#discussion_r125263918

Modifications:

Change <= to <

Result:

More correct code.
2017-07-18 09:24:19 +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
Norman Maurer
2a376eeb1b [maven-release-plugin] prepare for next development iteration 2017-07-06 13:24:06 +02:00
Norman Maurer
c7f8168324 [maven-release-plugin] prepare release netty-4.1.13.Final 2017-07-06 13:23:51 +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
Norman Maurer
3b5dd4e9dc Change type of X_FRAME_OPTIONS and CONTENT_SECURITY_POLICY to AsciiString as a followup to cc1a209185 2017-06-27 08:37:46 +02:00
Nikolay Fedorovskikh
ba3616da3e Apply appropriate methods for writing CharSequence into ByteBuf
Motivation:

1. `ByteBuf` contains methods to writing `CharSequence` which optimized for UTF-8 and ASCII encodings. We can also apply optimization for ISO-8859-1.
2. In many places appropriate methods are not used.

Modifications:

1. Apply optimization for ISO-8859-1 encoding in the `ByteBuf#setCharSequence` realizations.
2. Apply appropriate methods for writing `CharSequences` into buffers.

Result:

Reduce overhead from string-to-bytes conversion.
2017-06-27 07:58:39 +02:00
Norman Maurer
cc1a209185 Add content-security-policy and x-frame-options to HttpHeaderNames
Motivation:

These headers can be used to prevent clickjacking.

Modifications:

Add static fields for content-security-policy and x-frame-options

Result:

Expose general useful names
2017-06-27 07:55:25 +02:00
Nikolay Fedorovskikh
d672a5a483 Optimizations in QueryStringEncoder
Motivation:

A life cycle of QueryStringEncoder is simple: create, append params, convert to String. Current realization collect params in the list, and calculate an URI string in `toString` method. We can simplify this: don't store params to the list, and immediately append parameters to the `StringBuilder`.

Modifications:

- Remove list for params and remove a tuple class `Param`.
- Use one common `StringBuilder` and append parameters into it.
- Resolve `TODO` in the `encodeParam` method.

Result:

Less allocations (no `ArrayList`, no `Param` tuples). Second `toString` call is faster.
2017-06-23 14:03:32 -07:00
Nikolay Fedorovskikh
01eb428b39 Move methods for decode hex dump into StringUtil
Motivation:

PR #6811 introduced a public utility methods to decode hex dump and its parts, but they are not visible from netty-common.

Modifications:

1. Move the `decodeHexByte`, `decodeHexDump` and `decodeHexNibble` methods into `StringUtils`.
2. Apply these methods where applicable.
3. Remove similar methods from other locations (e.g. `HpackHex` test class).

Result:

Less code duplication.
2017-06-23 18:52:42 +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
Nikolay Fedorovskikh
b8a418d53d Remove redundant code block in HttpPostRequestEncoder and make some cleanup
Motivation:

The class `HttpPostRequestEncoder` has minor issues:
- The `encodeNextChunkMultipart()` method contains two identical blocks of code with a difference only in the cast interfaces: `Attribute` vs `HttpData`. Because the `Attribute` is extended by `HttpData`, the block with the `Attribute` can be safely deleted.
- The `getNewMultipartDelimiter()` method contains a redundant `toLowerCase()`.
- The `addBodyFileUploads()` method throws `NPE` instead of `IllegalArgumentException`.

Modifications:

- Remove duplicated code block from `encodeNextChunkMultipart()`.
- Remove redundant `toLowerCase()` from `getNewMultipartDelimiter()`.
- Replace `NPE` with `IllegalArgumentException` in `addBodyFileUploads()`.
- Use `ObjectUtil#checkNotNull` where possible.

Result:

More correct and clean code.
2017-06-14 06:49:20 +02:00
Norman Maurer
fd67a2354d [maven-release-plugin] prepare for next development iteration 2017-06-08 21:06:24 +02:00
Norman Maurer
3acd5c68ea [maven-release-plugin] prepare release netty-4.1.12.Final 2017-06-08 21:06:01 +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
Nikolay Fedorovskikh
b03b0f22d1 Removing a SeekAheadNoBackArrayException to avoid exception handling
Motivation:

A `SeekAheadNoBackArrayException` used as check for `ByteBuf#hasArray`. The catch of exceptions carries a large overhead on stack trace filling, and this should be avoided.

Modifications:

- Remove the class `SeekAheadNoBackArrayException` and replace its usage with `if` statements.
- Use methods from `ObjectUtils` for better readability.
- Make private methods static where it make sense.
- Remove unused private methods.

Result:

Less of exception handling logic, better performance.
2017-06-06 19:30:04 -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
Julien Viet
eee0ec3902 HttpObjectEncoder#isContentAlwaysEmpty cannot be overridden by subclasses
Motivation:

Allow subclasses of HttpObjectEncoder other than HttpServerCodec to override the isContentAlwaysEmpty method

Modification:

Change the method visibility from package private to protected

Result:

Fixes #6761
2017-05-31 07:36:12 +02: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
Roger Kapsi
b419bd152a Handle the possibility of HTTP/0.9 with a better error message
Motivation

RFC 1945 (see section 3.1) says that request lines may not have a version in which case the request is assumed to be HTTP/0.9. We don't necessarily want to support that but the existing Exception should indicate the possibility of the request being HTTP/0.9 and give the user a chance to track it down.

Modifications

Indicate in the Exception's message that the request is possibly HTTP/0.9.

Result

Fixes #6739
2017-05-26 07:51:49 +02:00
Cédric L
61efd81952 Add HTTP Status code 308 in HttpResponseStatus.java
Motivation:

The status 308 is defined by RFC7538.
This RFC has currently the state Proposed Standard since 2 years, but the status code is already handle by all browsers (Chrome, Firefox, Edge, Safari, …).
To let developer handles easily this status code, it is added into this list.

Modifications:

Added this status code in the list of all status codes and changed the valudOf() method

Result:

Status code 308 included
2017-05-24 16:26:32 +02:00
Anuraag Agrawal
ba5d1880bc Make HttpHeadersEncoder.encodeHeader package private to match class visibility. 2017-05-19 11:17:31 +02:00
Norman Maurer
0db2901f4d [maven-release-plugin] prepare for next development iteration 2017-05-11 16:00:55 +02:00
Norman Maurer
f7a19d330c [maven-release-plugin] prepare release netty-4.1.11.Final 2017-05-11 16:00:16 +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
Michael K. Werle
e70fbe316d Fire exceptionCaught before exception-caused close for WebSockets.
Motivation:

WebSocket decoding throws exceptions on failure that should cause the
pipline to close.  These are currently ignored in the
`WebSocketProtocolHandler` and `WebSocketServerProtocolHandler`.  In
particular, this means that messages exceding the max message size will
cause the channel to close with no reported failure.

Modifications:

Re-fire the event just before closing the socket to allow it to be
handled appropriately.

Result:

Closes [#3063].
2017-05-03 13:27:11 -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
Norman Maurer
6915ec3bb9 [maven-release-plugin] prepare for next development iteration 2017-04-29 14:10:00 +02:00
Norman Maurer
f30f242fee [maven-release-plugin] prepare release netty-4.1.10.Final 2017-04-29 14:09:32 +02: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
Aron Wieck
ffd6911586 Use constant string instead of user provided file name for DiskFileUpload temp file names.
Motivation:

DiskFileUpload creates temporary files for storing user uploads containing the user provided file name as part of the temporary file name. While most security problems are prevented by using "new File(userFileName).getName()" a small risk for bugs or security issues remains.

Modifications:

Use a constant string as file name and rely on the callers use of File.createTemp to ensure unique disk file names.

Result:

A slight security improvement at the cost of a little more obfuscated temp file names.
2017-04-27 16:02:41 -07:00
Dmitriy Dumanskiy
a45f9d7939 Improvement : allocate less object during multipart form parsing. 2017-04-25 14:06:10 +02:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Norman Maurer
fb113dce3a HttpPostRequestEncoder may return a slice which is not retained.
Motivation:

We miss to retain a slice before return it to the user and so an reference count error may accour later on.

Modifications:

Use readRetainedSlice(...) and so ensure we retain the buffer before hand it of to the user.

Result:

Fixes [#6626].
2017-04-19 11:40:38 +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
Scott Mitchell
b041f1a7a9 HttpServerKeepAliveHandler 204 response with no Content-Length should keepalive
Motivation:
https://tools.ietf.org/html/rfc7230#section-3.3.2 states that a 204 response MUST NOT include a Content-Length header. If the HTTP version permits keep alive these responses should be treated as keeping the connection alive even if there is no Content-Length header.

Modifications:
- HttpServerKeepAliveHandler#isSelfDefinedMessageLength should account for 204 respones

Result:
Fixes https://github.com/netty/netty/issues/6549.
2017-03-31 17:41:10 -07:00
cdn
71b338ce17 Non-latin character broken on HttpHeader by HttpObjectDecoder.
Motivation:

Currently netty is receiving HTTP request by ByteBuf and store it as "CharSequence" on HttpObjectDecoder. During this operation, all character on ByteBuf is moving to char[] without breaking encoding.
But in process() function, type casting from byte to char does not consider msb (sign-bit). So the value over 127 can be casted wrong value. (ex : 0xec in byte -> 0xffec in char). This is type casting bug.

Modification:

Fix type casting

Result:

Non-latin characters work.
2017-03-28 11:58:30 +02:00
David Dossot
9c1a191696 Trim optional white space in CombinedHttpHeaders values
Motivation:

The updated HTTP/1.x RFC allows for header values to be CSV and separated by OWS [1]. CombinedHttpHeaders should remove this OWS on insertion.

[1] https://tools.ietf.org/html/rfc7230#section-7

Modification:

CombinedHttpHeaders doesn't account for the OWS and returns it back to the user as part of the value.

Result:

Fixes #6452
2017-03-19 08:17:29 -07:00
Norman Maurer
2b8c8e0805 [maven-release-plugin] prepare for next development iteration 2017-03-10 07:46:17 +01:00
Norman Maurer
1db58ea980 [maven-release-plugin] prepare release netty-4.1.9.Final 2017-03-10 07:45:28 +01:00
Norman Maurer
e12f504ac1 Remove deprecated usage of Mockito methods
Motivation:

We used some deprecated Mockito methods.

Modifications:

- Replace deprecated method usage
- Some cleanup

Result:

No more usage of deprecated Mockito methods. Fixes [#6482].
2017-03-09 20:59:54 +01:00
Nikolay Fedorovskikh
2993760e92 Fix misordered 'assertEquals' arguments in tests
Motivation:

Wrong argument order in some 'assertEquals' applying.

Modifications:

Flip compared arguments.

Result:

Correct `assertEquals` usage.
2017-03-08 22:48:37 -08:00
Nikolay Fedorovskikh
f49bf4b201 Convert fields to the local variable when possible
Motivation:

Some classes have fields which can be local.

Modifications:

Convert fields to the local variable when possible.

Result:

Clean up. More chances for young generation or scalar replacement.
2017-03-08 17:09:17 -08:00
Nikolay Fedorovskikh
d702c47cab TODO for the method with typo in name 2017-03-08 09:28:33 -08:00
Norman Maurer
1392bc351f Correctly build socketaddress string, followup of 8b2badf44f 2017-03-01 20:05:20 +01:00
Norman Maurer
0514b0c61b Only add port to HOST header value if needed
Motivation:

We only need to add the port to the HOST header value if its not a standard port.

Modifications:

- Only add port if needed.
- Fix parsing of ipv6 address which is enclosed by [].

Result:

Fixes [#6426].
2017-03-01 19:08:19 +01:00
Dmitriy Dumanskiy
d1b0225724 Improvement : WebSocketServerProtocolHandshakeHandler.getWebSocketLocation now applies concat optimization and WebSocket path check moved to separated method.
Motivation:

Make code easier to read, make WebSocketServerProtocolHandshakeHandler.getWebSocketLocation method faster.

Modification:

WebSocket path check moved to separate method. Get header operation moved out from concat operation.

Result:

WebSocketServerProtocolHandshakeHandler.getWebSocketLocation is faster as OptimizeStringConcat could be applied. Code easier to read.
2017-03-01 10:47:20 +01:00
Norman Maurer
d675febf07 Optimize / Cleanup QueryStringDecoder / QueryStringEncoder
Motivation:

QueryStringDecoder and QueryStringEncoder contained some code that could either cleaned-up or optimized.

Modifications:

- Fix typos in exception messages and javadocs
- Precompile Pattern
- Make use of StringUtil.EMPTY_STRING

Result:

Faster and cleaner code.
2017-03-01 06:46:54 +01:00
Nikolay Fedorovskikh
943f4ec7ff Make methods 'static' where it missed
Motivation:

Calling a static method is faster then dynamic

Modifications:

Add 'static' keyword for methods where it missed

Result:

A bit faster method calls
2017-02-23 11:01:57 +01:00
Nikolay Fedorovskikh
0623c6c533 Fix javadoc issues
Motivation:

Invalid javadoc in project

Modifications:

Fix it

Result:

More correct javadoc
2017-02-22 07:31:07 +01:00
Norman Maurer
fbf0e5f4dd Prefer JDK ThreadLocalRandom implementation over ours.
Motivation:

We have our own ThreadLocalRandom implementation to support older JDKs . That said we should prefer the JDK provided when running on JDK >= 7

Modification:

Using ThreadLocalRandom implementation of the JDK when possible.

Result:

Make use of JDK implementations when possible.
2017-02-16 15:44:00 -08:00
Stephen E. Baker
9ee4cc0ada Correct comment for allowMaskMismatch parameter
Motivation:

The allowMaskMismatch parameter used throughout websocketx allows frames
with noncompliant masks when set to true, not false.

Modification:

Changed the javadoc comment everywhere it appears.

Result:

Fixes #6387
2017-02-16 17:03:55 +01:00
Jason Tedor
c92565d5c7 Correct expect header handling
Motivation:

Today, the HTTP codec in Netty responds to HTTP/1.1 requests containing
an "expect: 100-continue" header and a content-length that exceeds the
max content length for the server with a 417 status (Expectation
Failed). This is a violation of the HTTP specification. The purpose of
this commit is to address this situation by modifying the HTTP codec to
respond in this situation with a 413 status (Request Entity Too
Large). Additionally, the HTTP codec ignores expectations in the expect
header that are currently unsupported. This commit also addresses this
situation by responding with a 417 status.

Handling the expect header is tricky business as the specification (RFC
2616) is more complicated than it needs to be. The specification defines
the legitimate values for this header as "100-continue" and defines the
notion of expectatation extensions. Further, the specification defines a
417 status (Expectation Failed) and this is where implementations go
astray. The intent of the specification was for servers to respond with
417 status when they do not support the expectation in the expect
header.

The key sentence from the specification follows:

    The server MUST respond with a 417 (Expectation Failed) status if
    any of the expectations cannot be met or, if there are other
    problems with the request, some other 4xx status.

That is, a server should respond with a 417 status if and only if there
is an expectation that the server does not support (whether it be
100-continue, or another expectation extension), and should respond with
another 4xx status code if the expectation is supported but there is
something else wrong with the request.

Modifications:

This commit modifies the HTTP codec by changing the handling for the
expect header in the HTTP object aggregator. In particular, the codec
will now respond with 417 status if any expectation other than
100-continue is present in the expect header, the codec will respond
with 413 status if the 100-continue expectation is present in the expect
header and the content-length is larger than the max content length for
the aggregator, and otherwise the codec will respond with 100 status.

Result:

The HTTP codec can now be used to correctly reply to clients that send a
100-continue expectation with a content-length that is too large for the
server with a 413 status, and servers that use the HTTP codec will now
no longer ignore expectations that are not supported (any value other
than 100-continue).
2017-02-15 14:39:14 +01:00
Dmitriy Dumanskiy
506f0d8f8c Cleanup : String.length() == 0 replaced with String.isEmpty, removed unnecessary assert, class cast 2017-02-14 15:36:42 +01:00
Stephane Landelle
64abef5f5b Add exposeHeaders and allowedRequestHeaders that accept CharSequence, close #6328
Motivation:

Netty 4.1 introduced AsciiString and defines HttpHeaderNames constants
as such.

It would be convenient to be able to pass them to `exposeHeaders` and
`allowedRequestHeaders` directly without having to call `toString`.

Modifications:

Add `exposeHeaders` and `allowedRequestHeaders` overloads that take a
`CharSequence`.

Result:

More convenient API
2017-02-10 14:31:00 +01:00
Dmitriy Dumanskiy
c95517f759 Cleanup : removed unnecessary 'continue', explicit array creation, unwrapping 2017-02-10 12:25:01 +01:00
Stephane Landelle
9d45f514a4 Add a constant for Cookie "undefined maxAge"
Motivation:

DefaultCookie currently used an undocumented magic value for undefined
maxAge.

Clients need to be able to identify such value so they can implement a
proper CookieJar.

Ideally, we should add a `Cookie::isMaxAgeDefined` method but I guess
we can’t add a new method without breaking API :(

Modifications:

Add a new constant on `Cookie` interface so clients can use it to
compare with value return by `Cookie.maxAge` and decide if `maxAge` was
actually defined.

Result:

Clients have a better documented way to check if the maxAge attribute
was defined.
2017-02-09 10:14:29 +01:00
Norman Maurer
a7c0ff665c Only use Mockito for mocking.
Motivation:

We used various mocking frameworks. We should only use one...

Modifications:

Make usage of mocking framework consistent by only using Mockito.

Result:

Less dependencies and more consistent mocking usage.
2017-02-07 08:47:22 +01:00
Vladimir Kostyukov
0f9b739508 AggregatedFullHttpMessage.replace should also copy a decoder result
Motivation:

HttpObjectAggregator yields full HTTP messgaes (AggregatedFullHttpMessages) that don't respect decoder result when copied/replaced.

Modifications:

Copy the decoding result over to a new instance produced by AggregatedFullHttpRequest.replace or AggregatedFullHttpResponse.replace .

Result:

DecoderResult is now copied over when an original AggregatedFullHttpMessage is being replaced (i.e., AggregatedFullHttpRequest.replace or AggregatedFullHttpResponse.replace is being called).

New unit tests are passing on this branch but are failing on master.
2017-02-06 07:49:53 +01:00
Norman Maurer
735d6dd636 [maven-release-plugin] prepare for next development iteration 2017-01-30 15:14:02 +01:00
Norman Maurer
76e22e63f3 [maven-release-plugin] prepare release netty-4.1.8.Final 2017-01-30 15:12:36 +01:00
Chris Conroy
9bec25a6eb Set the Transfer-Encoding header instead of adding
Motivation:

HttpUtil.setTransferEncodingChunked could add a second Transfer-Encoding
header if one was already present. While this is technically valid, it
does not appear to be the intent of the method.

Result:

Only one Transfer-Encoding header is present after calling this method.
2017-01-25 07:53:53 +01:00
Dmitriy Dumanskiy
2d11331591 Typo fix in post encoder and replaced static hashmap with array. 2017-01-19 10:44:27 -08:00
Norman Maurer
7f01da8d0f [maven-release-plugin] prepare for next development iteration 2017-01-12 11:36:51 +01:00
Norman Maurer
7a21eb1178 [maven-release-plugin] prepare release netty-4.1.7.Final 2017-01-12 11:35:58 +01:00
Frederic BREGIER
56ddc47f23 Extends HttpPostRequestEncoder to support all methods except TRACE
Motivation:

In Netty, currently, the HttpPostRequestEncoder only supports POST, PUT, PATCH and OPTIONS, while the RFC 7231 allows with a warning that GET, HEAD, DELETE and CONNECT use a body too (but not TRACE where it is explicitely not allowed).
The RFC in chapter 4.3 says:
"A payload within a XXX request message has no defined semantics;
sending a payload body on a XXX request might cause some existing
implementations to reject the request."
where XXX can be replaced by one of GET, HEAD, DELETE or CONNECT.

Current usages, on particular in REST mode, tend to use those extra HttpMethods for such queries.

So this PR proposes to remove the current restrictions, leaving only TRACE as explicitely not supported.

Modification:

In the constructor, where the test is done, replacing all by checking only against TRACE, and adding one test to check that all methods are supported or not.

Result:

Fixes #6138.
2016-12-30 12:00:21 -08:00
Norman Maurer
0eeeb76439 Fix handling of FullHttpResponse when respond to HEAD in HttpServerCodec
Motivation:

cb139043f3 introduced special handling of response to HEAD requests. Due a bug we failed to handle FullHttpResponse correctly.

Modifications:

Correctly handle FullHttpResponse for HEAD requests.

Result:

Works as expected.
2016-12-21 20:53:02 +01:00
Scott Mitchell
3f82b53bae Add unit test for HttpObjectDecoder with message split on buffer boundaries
Motivation:
We should have a unit test which explicitly tests a HTTP message being split between multiple ByteBuf objects.

Modifications:
- Add a unit test to HttpRequestDecoderTest which splits a request between 2 ByteBuf objects

Result:
More unit test coverage for HttpObjectDecoder.
2016-12-20 12:59:00 -08:00
Malik Baktiyarov
16ddf460a6 Added checkStartsWith option for WebSocketServerProtocolHandler
Motivation:

Enables optional .startsWith() matching of req.uri() with websocketPath.

Modifications:

New checkStartsWith boolean option with default false value added to both WebSocketServerProtocolHandler and WebSocketServerProtocolHandshakeHandler. req.uri() matching is based on this option.

Result:

By default old behavior matching via .equal() is preserved. To use checkStartsWith use constructor shortcut: new WebSocketServerProtocolHandler(websocketPath, true) or fill this flag on full form of constructor among other options.
2016-12-20 10:53:33 +01:00
Norman Maurer
cb139043f3 [#5831] HttpServerCodec cannot encode a respons e to HEAD
request with a 'content-encoding: chunked' header

Motivation:

It is valid to send a response to a HEAD request that contains a transfer-encoding: chunked header, but it is not valid to include a body, and there is no way to do this using the netty4 HttpServerCodec.

The root cause is that the netty4 HttpObjectEncoder will transition to the state ST_CONTENT_CHUNK and the only way to transition back to ST_INIT is through the encodeChunkedContent method which will write the terminating length (0\r\n\r\n\r\n), a protocol error when responding to a HEAD request

Modifications:

- Keep track of the method of the request and depending on it handle the response differently when encoding it.
- Added a unit test.

Result:

Correclty handle HEAD responses that are chunked.
2016-12-15 07:54:51 +00:00
Stephane Maldini
ea0ddc0ea2 fix #6066 Support optional filename in HttpPostRequestEncoder
Motivation:

According to https://www.ietf.org/rfc/rfc2388.txt 4.4, filename after "content-disposition" is optional and arbitrary (does not need to match a real filename).

Modifications:

This change supports an extra addBodyFileUpload overload to precise the filename (default to File.getName). If empty or null this argument should be ignored during encoding.

Result:
- A backward-compatible addBodyFileUpload(String, File, String, boolean) to use file.getName() as filename.
- A new addBodyFileUpload(String, String, File, String, boolean) overload to precise filename
- Couple of tests for the empty use case
2016-12-01 06:54:51 +01:00
Stephane Landelle
ba95c401a7 Misc clean up
Motivation:
IntelliJ issues several warnings.

Modifications:

* `ClientCookieDecoder` and `ServerCookieDecoder`:
  * `nameEnd`, `valueBegin` and `valueEnd` don't need to be initialized
  * `keyValLoop` loop doesn't been to be labelled, as it's the most inner one (same thing for labelled breaks)
  * Remove `if (i != headerLen)` as condition is always true
* `ClientCookieEncoder` javadoc still mention old logic
* `DefaultCookie`, `ServerCookieEncoder` and `DefaultHttpHeaders` use ternary ops that can be turned into simple boolean ones
* `DefaultHeaders` uses a for(int) loop over an array. It can be turned into a foreach one as javac doesn't allocate an iterator to iterate over arrays
* `DefaultHttp2Headers` and `AbstractByteBuf` `equal` can be turned into a single boolean statement
Result:

Cleaner code
2016-11-22 15:17:05 -08:00
Stephane Landelle
f755e58463 Clean up following #6016
Motivation:

* DefaultHeaders from netty-codec has some duplicated logic for header date parsing
* Several classes keep on using deprecated HttpHeaderDateFormat

Modifications:

* Move HttpHeaderDateFormatter to netty-codec and rename it into HeaderDateFormatter
* Make DefaultHeaders use HeaderDateFormatter
* Replace HttpHeaderDateFormat usage with HeaderDateFormatter

Result:

Faster and more consistent code
2016-11-21 12:35:40 -08:00
radai-rosenblatt
886a7aae46 Fix timestamp parsing in HttpHeaderDateFormatter
Motivation:
code assumes a numeric value of 0 means no digits were read between separators, which fails for timestamps like 00:00:00.
also code accepts invalid timestamps like 0:0:000

Modifications:
explicitly check for number of digits between separators instead of relying on the numeric value.
also add tests.

Result:
timestamps with 00 successfully parse, timestamps with 000 no longer

Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
2016-11-21 10:17:54 +01:00
Norman Maurer
0b3122d8ff Deprecate HttpUtil.getCharsetAsString(...) and introduce HttpUtil.getCharsetAsSequence(...).
Motivation:

The method HttpUtil.getCharsetAsString(...) is missleading as its return type is CharSequence and not String.

Modifications:

Deprecate HttpUtil.getCharsetAsString(...) and introduce HttpUtil.getCharsetAsSe
quence(...).

Result:

Less confusing method name.
2016-11-21 07:47:20 +01:00
Stephane Landelle
edc4842309 Fix cookie date parsing, close #6016
Motivation:
* RFC6265 defines its own parser which is different from RFC1123 (it accepts RFC1123 format but also other ones). Basically, it's very lax on delimiters, ignores day of week and timezone. Currently, ClientCookieDecoder uses HttpHeaderDateFormat underneath, and can't parse valid cookies such as Github ones whose expires attribute looks like "Sun, 27 Nov 2016 19:37:15 -0000"
* ServerSideCookieEncoder currently uses HttpHeaderDateFormat underneath for formatting expires field, and it's slow.

Modifications:
* Introduce HttpHeaderDateFormatter that correctly implement RFC6265
* Use HttpHeaderDateFormatter in ClientCookieDecoder and ServerCookieEncoder
* Deprecate HttpHeaderDateFormat

Result:
* Proper RFC6265 dates support
* Faster ServerCookieEncoder and ClientCookieDecoder
* Faster tool for handling headers such as "Expires" and "Date"
2016-11-18 11:22:21 +00:00
Norman Maurer
0bc30a123e Eliminate usage of releaseLater(...) to reduce memory usage during tests
Motiviation:

We used ReferenceCountUtil.releaseLater(...) in our tests which simplifies a bit the releasing of ReferenceCounted objects. The problem with this is that while it simplifies stuff it increase memory usage a lot as memory may not be freed up in a timely manner.

Modifications:

- Deprecate releaseLater(...)
- Remove usage of releaseLater(...) in tests.

Result:

Less memory needed to build netty while running the tests.
2016-11-18 09:34:11 +01:00
Adrian Gonzalez
baac352f74 WebSocketClientHandshaker.rawPath(URI) should use the raw query
Motivation:

If the wsURL contains an encoded query, it will be decoded when generating the raw path.  For example if the wsURL is http://test.org/path?a=1%3A5, the returned raw path would be /path?a=1:5

Modifications:

Use wsURL.getRawQuery() rather than wsURL.getQuery()

Result:

rawPath will now return /path?a=1%3A5
2016-11-14 08:45:27 +01:00
Stephane Landelle
75728faf9b Add more HttpHeaders values
Motivation:
Some commons values are missing from HttpHeader values constants.

Modifications:
- Add constants for "application/json" Content-Type
- Add constants for "gzip,deflate" Content-Encoding

Result:
More HttpHeader values constants available, both in
`HttpHeaders.Values` and `HttpHeaderValues`.
2016-11-12 15:54:02 +01:00
Bryce Anderson
f0f0edbf78 HttpObjectAggregator adds 'Connection: close' header if necessary
Motivation:

The HttpObjectAggregator never appends a 'Connection: close' header to
the response of oversized messages even though in the majority of cases
its going to close the connection.

Modification:

This PR addresses that by ensuring the requisite header is present when
the connection is going to be closed.

Result:

Gracefully signal that we are about to close the connection.
2016-11-08 08:43:30 +01:00
Norman Maurer
8269e0f046 [#5892] Correct handle HttpMessage that is EOF terminated
Motivation:

We need to ensure we not add the Transfer-Encoding header if the HttpMessage is EOF terminated.

Modifications:

Only add the Transfer-Encoding header if an Content-Length header is present.

Result:

Correctly handle HttpMessage that is EOF terminated.
2016-11-01 11:13:44 +01:00
Moses Nakamura
bff951ca07 codec-http: HttpClientUpgradeHandler can handle streamed responses
Motivation:

We want to reject the upgrade as quickly as possible, so that we can
support streamed responses.

Modifications:

Reject the upgrade as soon as we inspect the headers if they're wrong,
instead of waiting for the entire response body.

Result:

If a remote server doesn't know how to use the http upgrade and tries to
responsd with a streaming response that never ends, the client doesn't
buffer forever, but can instead pass it along.  Fixes #5954
2016-11-01 06:32:41 +01:00