Motivation:
HttpPostStandardRequestDecoder may throw multiple different exceptions in the constructor which could lead to memory leaks. We need to guard against this by explicit catch all of them and rethrow after we released any allocated memory.
Modifications:
- Catch, destroy and rethrow in any case
- Ensure we correctly wrap IllegalArgumentExceptions
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/9829
### Motivation:
Those who need 'Origin' or 'Sec-WebSocket-Origin' headers should provide them explicitly, like it is stated in WebSocket specs.
E.g. through custom headers:
HttpHeaders customHeaders = new DefaultHttpHeaders()
.add(HttpHeaderNames.ORIGIN, "http://localhost:8080");
new WebSocketClientProtocolHandler(
new URI("ws://localhost:1234/test"), WebSocketVersion.V13, subprotocol,
allowExtensions, customHeaders, maxFramePayloadLength, handshakeTimeoutMillis)
### Modification:
* Remove enforced origin headers.
* Update tests
### Result:
Fixes#9673: Origin header is always sent from WebSocket client
Motivation:
By default CloseWebSocketFrames are handled automatically.
However I need manually manage their sending both on client- and on server-sides.
Modification:
Send close frame on channel close automatically, when it was not send before explicitly.
Result:
No more messages like "Connection closed by remote peer" for normal close flows.
### Motivation:
Introduction of `WebSocketDecoderConfig` made our server-side code more elegant and simpler for support.
However there is still some problem with maintenance and new features development for WebSocket codecs (`WebSocketServerProtocolHandler`, `WebSocketServerProtocolHandler`).
Particularly, it makes me ~~crying with blood~~ extremely sad to add new parameter and yet another one constructor into these handlers, when I want to contribute new feature.
### Modification:
I've extracted all parameters for client and server WebSocket handlers into config/builder structures, like it was made for decoders in PR #9116.
### Result:
* Fixes#9698: Simplify WebSocket handlers constructor arguments hell
* Unblock further development in this module (configurable close frame handling on server-side; automatic close-frame sending, when missed; memory leaks on protocol violations; etc...)
Bonuses:
* All defaults are gathered in one place and could be easily found/reused.
* New API greatly simplifies usage, but does NOT allow inheritance or modification.
* New API would simplify long-term maintenance of WebSockets module.
### Example
WebSocketClientProtocolConfig config = WebSocketClientProtocolConfig.newBuilder()
.webSocketUri("wss://localhost:8443/fx-spot")
.subprotocol("trading")
.handshakeTimeoutMillis(15000L)
.build();
ctx.pipeline().addLast(new WebSocketClientProtocolHandler(config));
Motivation:
At the moment we miss to poll the method queue when we see an Informational response code. This can lead to out-of-sync of request / response pairs when later try to compare these.
Modifications:
Always poll the queue correctly
Result:
Always compare the correct request / response pairs
Motivation:
HTTP 102 (WebDAV) is not correctly treated as an informational response
Modification:
Delegate all `1XX` status codes to superclass, not just `100` and `101`.
Result:
Supports WebDAV response.
Removes a huge maintenance [headache](https://github.com/line/armeria/pull/2210) in Armeria which has forked the class for these features
Motivation:
HttpPostRequestDecoder.splitHeaderContentType() throws a StringIndexOutOfBoundsException when it parses a Content-Type header that starts with a semicolon ;. We should skip the execution for incorrect multipart form data.
Modification:
Avoid invocation of HttpPostRequestDecoder#splitHeaderContentType(...) for incorrect multipart form data content-type.
Result:
Fixes#8554
Motivation:
When parsing HTTP headers special care needs to be taken when a whitespace is detected in the header name.
Modifications:
- Ignore whitespace when decoding response (just like before)
- Throw exception when whitespace is detected during parsing
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/9571
Motivation:
At the current moment HttpContentEncoder handle only first value of multiple accept-encoding headers.
Modification:
Join multiple accept-encoding headers to one separated by comma.
Result:
Fixes#9553
Motivation:
Currently when HttpPostStandardRequestDecoder throws a ErrorDataDecoderException during construction we leak memory. We need to ensure all is released correctly.
Modifications:
- Call destroy() if parseBody() throws and rethrow the ErrorDataDecoderException
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/9513.
Motivation:
Http post request may be encoded as 'multipart/form-data' without any files and consist mixed attributes only.
Modifications:
- Do not double release attributes
- Add unit test
Result:
Code does not throw an IllegalReferenceCountException.
Motivation:
We need to ensure we replace WebSocketServerProtocolHandshakeHandler before doing the actual handshake as the handshake itself may complete directly and so forward pending bytes through the pipeline.
Modifications:
Replace the handler before doing the actual handshake.
Result:
Fixes https://github.com/netty/netty/issues/9471.
Motivation:
It was possible to produce a NPE when we for examples received more responses as requests as we did not check if the queue did not contain a method before trying to compare method names.
Modifications:
- Add extra null check
- Add unit tet
Result:
Fixes https://github.com/netty/netty/issues/9459
Motivation:
Allow to set the ORIGIN header value from custom headers in WebSocketClientHandshaker
Modification:
Only override header if not present already
Result:
More flexible handshaker usage
Motivation:
If the HttpUtil.getCharset method is called with an illegal charset like
"charset=!illegal!" it throws an IllegalCharsetNameException. But the javadoc
states, that defaultCharset is returned if incorrect header value. Since the
client sending the request sets the header value this should not crash.
Modification:
HttpUtil.getCharset catches the IllegalCharsetNameException and returns the
defualt value.
Result:
HttpUtil.getCharset does not throw IllegalCharsetNameException any more.
…ryWebSocketFrames
Motivation:
`Utf8FrameValidator` is always created and added to the pipeline in `WebSocketServerProtocolHandler.handlerAdded` method. However, for websocket connection with only `BinaryWebSocketFrame`'s UTF8 validator is unnecessary overhead. Adding of `Utf8FrameValidator` could be easily avoided by extending of `WebSocketDecoderConfig` with additional property.
Specification requires UTF-8 validation only for `TextWebSocketFrame`.
Modification:
Added `boolean WebSocketDecoderConfig.withUTF8Validator` that allows to avoid adding of `Utf8FrameValidator` during pipeline initialization.
Result:
Less overhead when using only `BinaryWebSocketFrame`within web socket.
Motivation:
In many places Netty uses Unpooled.buffer(0) while should use EMPTY_BUFFER. We can't change this due to back compatibility in the constructors but can use Unpooled.EMPTY_BUFFER in some cases to ensure we not allocate at all. In others we can directly use the allocator either from the Channel / ChannelHandlerContext or the request / response.
Modification:
- Use Unpooled.EMPTY_BUFFER where possible
- Use allocator where possible
Result:
Fixes#9345 for websockets and http package
Motivation:
We need to ensure we place the encoder before the decoder when doing the websockets upgrade as the decoder may produce a close frame when protocol violations are detected.
Modifications:
- Correctly place encoder before decoder
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/9300
Motivation:
If the encoded value of a form element happens to exactly hit
the chunk limit (8096 bytes), the post request encoder will
throw a NullPointerException.
Modifications:
Catch the null case and return.
Result:
No NPE.
Motivation:
Based on https://tools.ietf.org/html/rfc6455#section-1.3 - for non-browser
clients, Origin header field may be sent if it makes sense in the context of those clients.
Modification:
Replace Sec-WebSocket-Origin to Origin
Result:
Fixes#9134 .
Motivation:
There are is some unnecessary code (like toString() calls) which can be cleaned up.
Modifications:
- Remove not needed toString() calls
- Simplify subString(...) calls
- Remove some explicit casts when not needed.
Result:
Cleaner code
Motivation:
asList should only be used if there are multiple elements.
Modification:
Call to asList with only one argument could be replaced with singletonList
Result:
Cleaner code and a bit of memory savings
Motivation:
The HttpPostRequestEncoder overwrites the original filename of file uploads sharing the same name encoded in mixed mode when it rewrites the multipart body header of the previous file. The original filename should be preserved instead.
Modifications:
Change the HttpPostRequestEncoder to reuse the correct filename when the encoder switches to mixed mode. The original test is incorrect and has been modified too, in addition it tests with an extra file upload since the current test was not testing the continuation of a mixed mode.
Result:
The HttpPostRequestEncoder will preserve the original filename of the first fileupload when switching to mixed mode
Motivation:
I need to control WebSockets inbound flow manually, when autoRead=false
Modification:
Add missed ctx.read() call into WebSocketProtocolHandler, where read request has been swallowed.
Result:
Fixes#9257
Motivation:
Incorrect WebSockets closure affects our production system.
Enforced 'close socket on any protocol violation' prevents our custom termination sequence from execution.
Huge number of parameters is a nightmare both in usage and in support (decoders configuration).
Modification:
Fix violations handling - send proper response codes.
Fix for messages leak.
Introduce decoder's option to disable default behavior (send close frame) on protocol violations.
Encapsulate WebSocket response codes - WebSocketCloseStatus.
Encapsulate decoder's configuration into a separate class - WebSocketDecoderConfig.
Result:
Fixes#8295.
Motivation:
When connecting through an HTTP proxy over clear HTTP, user agents must send requests with an absolute url. This hold true for WebSocket Upgrade request.
WebSocketClientHandshaker and subclasses currently always send requests with a relative url, which causes proxies to crash as request is malformed.
Modification:
Introduce a new parameter `absoluteUpgradeUrl` and expose it in constructors and WebSocketClientHandshakerFactory.
Result:
It's now possible to configure WebSocketClientHandshaker so it works properly with HTTP proxies over clear HTTP.
delete Other "Content-" MIME Header Fields exception
Motivation:
RFC7578 4.8. Other "Content-" Header Fields
The multipart/form-data media type does not support any MIME header
fields in parts other than Content-Type, Content-Disposition, and (in
limited circumstances) Content-Transfer-Encoding. Other header
fields MUST NOT be included and MUST be ignored.
Modification:
Ignore other Content types.
Result:
Other "Content-" Header Fields should be ignored no exception
Motivation:
Support handshake timeout option in websocket handlers. It makes sense to limit the time we need to move from `HANDSHAKE_ISSUED` to `HANDSHAKE_COMPLETE` states when upgrading to WebSockets
Modification:
- Add `handshakeTimeoutMillis` option in `WebSocketClientProtocolHandshakeHandler` and `WebSocketServerProtocolHandshakeHandler`.
- Schedule a timeout task, the task will trigger user event `HANDSHAKE_TIMEOUT` if the handshake timed out.
Result:
Fixes issue https://github.com/netty/netty/issues/8841
Motivation
Pipeline handlers are free to "take control" of input buffers if they have singular refcount - in particular to mutate their raw data if non-readonly via discarding of read bytes, etc.
However there are various places (primarily unit tests) where a wrapped byte-array buffer is passed in and the wrapped array is assumed not to change (used after the wrapped buffer is passed to EmbeddedChannel.writeInbound()). This invalid assumption could result in unexpected errors, such as those exposed by #8931.
Modifications
Anywhere that the data passed to writeInbound() might be used again, ensure that either:
- A copy is used rather than wrapping a shared byte array, or
- The buffer is otherwise protected from modification by making it read-only
For the tests, copying is preferred since it still allows the "mutating" optimizations to be exercised.
Results
Avoid possible errors when pipeline assumes it has full control of input buffer.
Motivation:
We did manually call HttpObjectDecoder.reset() in HttpObjectAggregator.handleOversizedMessage(...) which is incorrect and will prevent correct parsing of the next message.
Modifications:
- Remove call to HttpObjectDecoder.reset()
- Add unit test
Result:
Verify that we can correctly parse the next request after we rejected a request.
Motivation:
RFC 6455 defines that, generally, a WebSocket client should not close a TCP
connection as far as a server is the one who's responsible for doing that.
In practice tho', it's not always possible to control the server. Server's
misbehavior may lead to connections being leaked (if the server does not
comply with the RFC).
RFC 6455 #7.1.1 says
> In abnormal cases (such as not having received a TCP Close from the server
after a reasonable amount of time) a client MAY initiate the TCP Close.
Modifications:
* WebSocket client handshaker additional param `forceCloseAfterMillis`
* Use 10 seconds as default
Result:
WebSocket client handshaker to comply with RFC. Fixes#8883.
Motivation:
32563bfcc1 introduced a regression in which we did now not longer discard the messages after we handled an oversized message.
Modifications:
- Do not set aggregating to false after handleOversizedMessage is called
- Adjust unit tests to verify the behaviour is correct again.
Result:
Fixes https://github.com/netty/netty/issues/9007.
Motivation:
Add user possibility to skip the evaluation of certain web socket extension,
for example we can skip compression extension for messages that already compressed or very small and etc.
Modification:
This pull request is related with #5669
Result:
User can set to WebSocketClientExtensionHandshaker or WebSocketServerExtensionHandshaker a filter to skip the evaluation of certain extension.
Motivation:
According to the specification, the "Connection" header's syntax is:
"
The Connection header field's value has the following grammar:
Connection = 1#connection-option
connection-option = token
Connection options are case-insensitive.
"
https://tools.ietf.org/html/rfc7230#section-6.1
This means that Connection's value can have at least one element or
a comma separated list with elements
When calculating whether the connection can remain open,
HttpUtil.isKeepAlive(HttpMessage) should take this into account.
Modifications:
- Check for "close" and "keep-alive" in a comma separated list
- Add unit test
Result:
HttpUtil.isKeepAlive(HttpMessage) works correctly when "Connection: Upgrade, close"
Motivation:
When HttpContentDecoder (and so HttpContentDecompressor) does not produce any message we need to make sure it calls ctx.read() if auto read is false to not stale.
Modifications:
- Keep track if we need to call ctx.read() or not
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/8915.
Motivation:
Gracefully respond on bad client request.
We have a set of errors produced by Android 7.1.1/7.1.2 clients where both headers `HttpHeaderNames.SEC_WEBSOCKET_VERSION` and `HttpHeaderNames.ORIGIN` are not present. Absence of the first headers leads to WebSocketServerHandshaker00 be applied as a handshaker. However, null 2nd header causes
```
java.lang.NullPointerException: value
io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:33)
io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:327)
io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:123)
io.netty.handler.codec.http.websocketx.WebSocketServerHandshaker00.newHandshakeResponse(WebSocketServerHandshaker00.java:162)
```
Which causes connection close with unclear reason.
Modification:
Added null-check, and in case of null an appropriate WebSocketHandshakeException is thrown.
Result:
In case of null `HttpHeaderNames.ORIGIN` header a WebSocketHandshakeException is caught by WebSocketServerProtocolHandler which sends a graceful `BAD_REQUEST`.
Motivation
Implementations of MessageAggregator (HttpObjectAggregator in particular) may wish to
selectively aggrerage requests and responses on a case-by-case basis such as for example
only POST requests or only responses of a certain content-type.
Modifications
Adding a flag to MessageAggregator that toggles between true/false depending on if aggregation
is desired for the current message or not.
Result
Fixes#8772
* HttpObjectDecoder ignores HTTP trailer header when empty line is received in seperate ByteBuf
Motivation:
When the empty line that termines the trailers was sent in a seperate ByteBuf we did ignore the previous parsed trailers and just returned none.
Modifications:
- Correct respect previous parsed trailers.
- Add unit test.
Result:
Fixes https://github.com/netty/netty/issues/8736
Motivation:
We need to update to a new checkstyle plugin to allow the usage of lambdas.
Modifications:
- Update to new plugin version.
- Fix checkstyle problems.
Result:
Be able to use checkstyle plugin which supports new Java syntax.
Motivation:
I want to fix bug in vert.x project (eclipse-vertx/vert.x#2562) caused by ComposedLastHttpContent result being null. I don't know if it is intentional that this last decoded chuck in the issue returns null, but if not - I am providing fix for that.
Modification:
* Added new constructor in ComposedLastHttpContent allowing to pass DecoderResult
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentEncoder
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentDecoder
Result:
Fixeseclipse-vertx/vert.x#2562
Motivation:
The CorruptedFrameException from the finish() method of the Utf8Validator gets propagated to other handlers while the connection is still open.
Modification:
Override exceptionCaught method of the Utf8FrameValidator and close the connection if it is a CorruptedFrameException.
Result:
The CorruptedFrameException gets propagated to other handlers only after properly closing the connection.
Motivation:
RFC 6455 doesn't define close status codes 1012, 1013 and 1014.
Yet, since then, IANA has defined them and web browsers support them.
From https://www.iana.org/assignments/websocket/websocket.xhtml:
* 1012: Service Restart
* 1013: Try Again Later
* 1014: The server was acting as a gateway or proxy and received an invalid response from the upstream server. This is similar to 502 HTTP Status Code.
Modification:
Make status codes 1012, 1013 and 1014 legit.
Result:
WebSocket status codes as defined by IANA are supported.
Motivation:
RFC 6455 doesn't define status codes 1012, 1013 and 1014.
Yet, since then, IANA has defined them, web browsers support them, applications in the wild do use them but it's currently not possible to buid a Netty based client for those services.
From https://www.iana.org/assignments/websocket/websocket.xhtml:
* 1012: Service Restart
* 1013: Try Again Later
* 1014: The server was acting as a gateway or proxy and received an invalid response from the upstream server. This is similar to 502 HTTP Status Code.
Modification:
Make status codes 1012, 1013 and 1014 legit.
Result:
WebSocket status codes as defined by IANA are supported.
Motivation:
According to the HTTP spec set-cookie headers should not be combined
because they are not using the list syntax.
Modifications:
Do not combine set-cookie headers.
Result:
Set-Cookie headers won't be combined anymore
Motivation:
As mentioned in RFC 7692 :
The "server_no_context_takeover" Extension Parameter should be used on server side for compression and on client side for decompression.
The "client_no_context_takeover" Extension Parameter should be used on client side for compression and on server side for decompression.
Right now, in PerMessageDeflateClientExtensionHandshaker, the decoder uses clientNoContext instead of serverNoContext and the encoder uses serverNoContext instead of clientNoContext.
The same inversion is present in PerMessageDeflateServerExtensionHandshaker: the decoder uses
serverNoContext instead of clientNoContext, while the encoder uses serverNoContext instead of clientNoContext. Besides the context inversion, the sliding window sizes seem to be inversed as well.
Modification:
Inverse clientNoContext with serverNoContext and clientWindowSize with serverWindowSize for both the Decoder and Encoder in PerMessageDeflateServerExtensionHandshaker and PerMessageDeflateClientExtensionHandshaker.
Result:
This fixes the decompression fail in the case that one of the contexts is set and the other one is not.
