Motivation:
We did not correctly pass all supplied parameters to the called constructor and so did not apply the timeout.
Modification:
Correctly pass on the parameters.
Result:
Use timeout
Motivation:
Some of the links in javadoc point to the obsolete drafts of HTTP/2
specifications. We should point them to the latest RFC 7540 or 7541.
Modifications:
Update links from `draft-ietf-httpbis-*` to the `rfc7540` and `rfc7541`.
Result:
Correct links in javadoc.
Motivation:
`HttpObjectDecoder` pre-checks that it doesn't request characters
outside of the `AppendableCharSequence`'s length. `0` is always allowed
because the minimal length of `AppendableCharSequence` is `1`. We can
legally skip index check by using
`AppendableCharSequence.charAtUnsafe(int)` in all existing cases in
`HttpObjectDecoder`.
Modifications:
- Use `AppendableCharSequence.charAtUnsafe(int)` instead of
`AppendableCharSequence.charAt(int)` in `HttpObjectDecoder`.
Result:
No unnecessary index checks in `HttpObjectDecoder`.
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:
If all we need is the FileChannel we should better use RandomAccessFile as FileInputStream and FileOutputStream use a finalizer.
Modifications:
Replace FileInputStream and FileOutputStream with RandomAccessFile when possible.
Result:
Fixes https://github.com/netty/netty/issues/8078.
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:
We did not correctly pass the mask parameters in all cases.
Modifications:
Correctly pass on parameters
Result:
Fixes https://github.com/netty/netty/issues/9463.
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:
Our QA servers are spammed with this messages:
13:57:51.560 DEBUG- Decoding WebSocket Frame opCode=1
13:57:51.560 DEBUG- Decoding WebSocket Frame length=4
I think this is too much info for debug level. It is better to move it to trace level.
Modification:
logger.debug changed to logger.trace for WebSocketFrameEncoder/Decoder
Result:
Less messages in Debug mode.
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:
While fixing #9359 found few places that could be patched / improved separately.
Modification:
On handshake response generation - throw exception before allocating response objects if request is invalid.
Result:
No more leaks when exception is thrown.
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:
Netty homepage(netty.io) serves both "http" and "https".
It's recommended to use https than http.
Modification:
I changed from "http://netty.io" to "https://netty.io"
Result:
No effects.
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:
f17bfd0f64 removed the usage of static exception instances to reduce the risk of OOME due addSupressed calls. We should do the same for exceptions used to signal handshake timeouts.
Modifications:
Do not use static instances
Result:
No risk of OOME due addSuppressed calls
Motivation:
The first final version of GraalVM was released which deprecated some flags. We should use the new ones.
Modifications:
Removes the use of deprecated GraalVM native-image flags
Adds a flag to initialize netty at build time.
Result:
Do not use deprecated flags
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:
OOME is occurred by increasing suppressedExceptions because other libraries call Throwable#addSuppressed. As we have no control over what other libraries do we need to ensure this can not lead to OOME.
Modifications:
Only use static instances of the Exceptions if we can either dissable addSuppressed or we run on java6.
Result:
Not possible to OOME because of addSuppressed. Fixes https://github.com/netty/netty/issues/9151.
Motivation:
GraalVM native images are a new way to deliver java applications. Netty is one of the most popular libraries however there are a few limitations that make it impossible to use with native images out of the box. Adding a few metadata (in specific modules will allow the compilation to success and produce working binaries)
Modification:
Added properties files in `META-INF` and substitutions classes (under `internal.svm`) will solve the compilation issues. The substitutions classes are not visible and do not have a public constructor so they are not visible to end users.
Result:
Fixes#8959
This fix is very conservative as it applies the minimum config required to build:
* pure netty servers
* vert.x applications
* grpc applications
The build is having trouble due to checkstyle which does not seem to be able to find the copyright notice on property files.
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:
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:
In 42742e233f we already added default methods to Channel*Handler and deprecated the Adapter classes to simplify the class hierarchy. With this change we go even further and merge everything into just ChannelHandler. This simplifies things even more in terms of class-hierarchy.
Modifications:
- Merge ChannelInboundHandler | ChannelOutboundHandler into ChannelHandler
- Adjust code to just use ChannelHandler
- Deprecate old interfaces.
Result:
Cleaner and simpler code in terms of class-hierarchy.
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:
As we now us java8 as minimum java version we can deprecate ChannelInboundHandlerAdapter / ChannelOutboundHandlerAdapter and just move the default implementations into the interfaces. This makes things a bit more flexible for the end-user and also simplifies the class-hierarchy.
Modifications:
- Mark ChannelInboundHandlerAdapter and ChannelOutboundHandlerAdapter as deprecated
- Add default implementations to ChannelInboundHandler / ChannelOutboundHandler
- Refactor our code to not use ChannelInboundHandlerAdapter / ChannelOutboundHandlerAdapter anymore
Result:
Cleanup class-hierarchy and make things a bit more flexible.
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:
WebSocketVersion can be simplified by directly store the string representation in the enum.
Modification:
Pass in the string representation when creating the enum.
Result:
Cleaner code.
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:
Since Java 8, JDK has `java.util.Base64` that could replace custom netty implementation. It is faster (3x for this particular PR) and simpler.
Modifications:
Use Base64 directly.
Result:
```
Benchmark Mode Cnt Score Error Units
Base64Benchmark.base64New thrpt 6 7739181.025 ± 114230.467 ops/s
Base64Benchmark.base64Old thrpt 6 2689783.304 ± 454710.641 ops/s
```
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:
SPDY has been superseded by HTTP/2. Chrome has dropped support in 2016 and GFE no longer negociate it.
Modifications:
* drop codec
* drop examples
* drop constants from `ApplicationProtocolNames`
Result:
SPDY support dropped from Netty 5
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:
We can use ThreadLocalRandom.current().nextInt() directly .
Motivation:
Use ThreadLocalRandom.current().nextInt() directly instead of (int) ThreadLocalRandom.current().nextDouble()
Result:
Less custom code to maintain
Motivation:
We can just use Objects.requireNonNull(...) as a replacement for ObjectUtil.checkNotNull(....)
Modifications:
- Use Objects.requireNonNull(...)
Result:
Less code to maintain.
Motivation:
We can re-use Files.readAllBytes(....) and so can remove our own implementation.
Modification:
Replaced custom readAllBytes with Files.readAllBytes
Result:
Less custom code.
