Motivation:
We should not depend on the implementation detail of Unpooled.buffer(int) to allocate the exact size of backing byte[] as depending on the implementation it may return a buffer with a bigger backing array.
Modifications:
Explicit allocate the byte[] and wrap it in the ByteBuf. This way we are sure that ByteBuf.array() returns an byte[] which has the exact length and content we expect.
Result:
More correct and safe usage of ByteBuf.array()
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:
When more than one connection header is present in h2c upgrade request, upgrade fails. This is to fix that.
Modification:
In HttpServerUpgradeHandler's upgrade() method, check whether any of the connection header value is upgrade, not just the first header value which might return a different value other than upgrade.
Result:
Fixes#8846.
With this PR, now when multiple connection headers are sent with the upgrade request, upgrade will not fail.
Motivation:
We can replace some "hand-rolled" integer checks with our own static utility method to simplify the code.
Modifications:
Use methods provided by `ObjectUtil`.
Result:
Cleaner code and less duplication
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:
In most cases, HttpMethod instance is built from the factory method and the same instance is taken for known Http Methods. So we can implement fast path for equals().
Modification:
Replace == checks with HttpMethod.equals;
Use this == o within HttpMethod.equals;
Replaced known new HttpMethod with HttpMethod.valueOf;
Result:
Comparisons should be a bit faster in some cases.
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:
Current implementation extract header value as String. We have an idiomatic way for checking presence of a header value.
Modification:
Use HttpHeaders#contains for checking if if contains Expect: 100-continue.
Result:
Use idiomatic way + simplify boolean logic.
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:
Most of the maven modules do not explicitly declare their
dependencies and rely on transitivity, which is not always correct.
Modifications:
For all maven modules, add all of their dependencies to pom.xml
Result:
All of the (essentially non-transitive) depepdencies of the modules are explicitly declared in pom.xml
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.
Motivation:
Get charset from Content-Type header even it contains multiple parameters.
Modification:
Extract charset value from the charset parameter if it is not last.
Result:
Fixes#8273
Motivation:
Implementation of WebSocketUtil/randomNumber is incorrect and might violate
the API returning values > maximum specified.
Modifications:
* WebSocketUtil/randomNumber is reimplemented, the idea of the solution described
in the comment in the code
* Implementation of WebSocketUtil/randomBytes changed to nextBytes method
* PlatformDependet.threadLocalRandom is used instead of Math.random to improve efficiency
* Added test cases to check random numbers generator
* To ensure corretness, we now assert that min < max when generating random number
Result:
WebSocketUtil/randomNumber always produces correct result.
Covers https://github.com/netty/netty/issues/8023
Motivation:
If a wrapped cookie value with an invalid charcater is passed to the strict
encoder, an exception is thrown on validation but the error message contains
a character at the wrong position.
Modifications:
Print `unwrappedValue.charAt(pos)` instead of `value.charAt(pos)`.
Result:
The exception indicates the correct invalid character in the unwrapped cookie.
Motivation
The HttpObjectEncoder raises an IllegalStateException due to an illegal state but doesn't mention what the state was. It could be useful for debugging purposes to figure out what happened.
Modifications
Mention the HttpObjectEncoder's state in the message of the IllegalStateException.
Result
An exception with more information what caused it.
Motivation:
Currently, when passing custom headers to a WebSocketClientHandshaker,
if values are added for headers that are reserved for use in the
websocket handshake performed with the server, these custom values can
be used by the server to compute the websocket handshake challenge. If
the server computes the response to the challenge with the custom header
values, rather than the values computed by the client handshaker, the
handshake may fail.
Modifications:
Update the client handshaker implementations to add the custom header
values first, and then set the reserved websocket header values.
Result:
Reserved websocket handshake headers, if present in the custom headers
passed to the client handshaker, will not be propagated to the server.
Instead the client handshaker will propagate the values it generates.
Fixes#7973.
Motivation:
The websockets abstract test suite does not run against the 08
implementation in the 08 version of the test suite.
Modifications:
Update the WebSocketClientHandshaker08Test to instantiate a new
WebSocketClientHandshaker08 rather than an 07 handshaker.
Result:
The WebSocketClientHandshaker08Test now tests the 08 implementation.
Motivation:
Currently, on recipt of a PongWebSocketFrame, the
WebSocketProtocolHandler will drop the frame, rather than passing it
along so it can be referenced by other handlers.
Modifications:
Add boolean field to WebSocketProtocolHandler to indicate whether Pong
frames should be dropped or propagated, defaulting to "true" to preserve
existing functionality.
Add new constructors to the client and server implementations of
WebSocketProtocolHandler that allow for overriding the behavior for the
handling of Pong frames.
Result:
PongWebSocketFrames are passed along the channel, if specified.
Motivation:
DefaultHttpResponse did not respect its status when compute the hashCode and check for equality.
Modifications:
Correctly implement hashCode and equals
Result:
Fixes https://github.com/netty/netty/issues/7964.
Motivation:
HTTP responses with status of 205 should not contain a payload. We should enforce this.
Modifications:
Correctly handle responses with status 205 and payload by set Content-Length: 0 header and stripping out the content.
Result:
Fixes https://github.com/netty/netty/issues/7888
Motivation:
It should be possible to call setContent(Unpooled.EMPTY_BUFFER) multiple times just like its possible to do the same with a non empty buffer.
Modifications:
- Correctly reset underlying storage if called multiple times.
- Add tests
Result:
Fixes https://github.com/netty/netty/issues/6418
