Merged
Conversation
… if delegate is instance of Http2SettingsReceivedConsumer (#9343) Motivation: b3dba31 introduced the concept of Http2SettingsReceivedConsumer but did not correctly inplement DecoratingHttp2ConnectionEncoder.consumeRemoteSettings(...). Modifications: - Add missing `else` around the throws - Add unit tests Result: Correctly implement DecoratingHttp2ConnectionEncoder.consumeRemoteSettings(...)
Motivation: Http2ConnectionHandler (and sub-classes) allow to configure a graceful shutdown timeout but only apply it if there is at least one active stream. We should always apply the timeout. This is also true when we try to send a GO_AWAY and close the connection because of an connection error. Modifications: - Always apply the timeout if one is configured - Add unit test Result: Always respect gracefulShutdownTimeoutMillis
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 Debugging SSL/TLS connections through wireshark is a pain -- if the cipher used involves Diffie-Hellman then it is essentially impossible unless you can have the client dump out the master key [1] This is a work-in-progress change (tests & comments to come!) that introduces a new handler you can set on the SslContext to receive the master key & session id. I'm hoping to get feedback if a change in this vein would be welcomed. An implementation that conforms to Wireshark's NSS key log[2] file is also included. Depending on feedback on the PR going forward I am planning to "clean it up" by adding documentation, example server & tests. Implementation will need to be finished as well for retrieving the master key from the OpenSSL context. [1] https://jimshaver.net/2015/02/11/decrypting-tls-browser-traffic-with-wireshark-the-easy-way/ [2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format Modification - Added SslMasterKeyHandler - An implementation of the handler that conforms to Wireshark's key log format is included. Result: Be able to debug SSL / TLS connections more easily. Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
) Motivation: Recently I'm going to build MQTT broker and client based on Netty. I had MQTT encoder and decoder founded, while no basic examples. So I'm going to share my simple heartBeat MQTT broker and client as an example. Modification: New MQTT heartBeat example under io.netty.example/mqtt/heartBeat/. Result: Client would send CONNECT and PINGREQ(heartBeat message). - CONNECT: once channel active - PINGREQ: once IdleStateEvent triggered, which is 20 seconds in this example Client would discard all messages it received. MQTT broker could handle CONNECT, PINGREQ and DISCONNECT messages. - CONNECT: send CONNACK back - PINGREQ: send PINGRESP back - DISCONNECT: close the channel Broker would close the channel if 2 heartBeat lost, which set to 45 seconds in this example.
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 I noticed this while looking at something else. AbstractEpollStreamChannel::spliceQueue is an MPSC queue but only accessed from the event loop. So it could be just changed to e.g. an ArrayDeque. This PR instead reverts to using is as an MPSC queue to avoid dispatching a task to the EL, as appears was the original intention. Modification Change AbstractEpollStreamChannel::spliceQueue to be volatile and lazily initialized via double-checked locking. Add tasks directly to the queue from the public methods rather than possibly waking the EL just to enqueue. An alternative is just to change PlatformDependent.newMpscQueue() to new ArrayDeque() and be done with it :) Result Less disruptive channel/fd-splicing.
Motivation: There are situations where the user may want to be more flexible when to send the PING acks for various reasons or be able to attach a listener to the future that is used for the ping ack. To be able to do so we should allow to manage the acks manually. Modifications: - Add constructor to DefaultHttp2ConnectionDecoder that allows to disable the automatically sending of ping acks (default is to send automatically to not break users) - Add methods to AbstractHttp2ConnectionHandlerBuilder (and sub-classes) to either enable ot disable auto acks for pings - Make DefaultHttp2PingFrame constructor public that allows to write acks. - Add unit test Result: More flexible way of handling acks.
…arked as public by mistake (#9372) Motivation: Mark Http2StreamChannelBootstrap.open0(...) as deprecated as the user should not use it. It was marked as public by mistake. Modifications: Add deprecation warning. Result: User will be aware the method should not be used directly.
Motivation The AbstractEpollStreamChannel::spliceTo(FileDescriptor, ...) methods take an offset parameter but this was effectively ignored due to what looks like a typo in the corresponding JNI function impl. Instead it would always use the file's own native offset. Modification - Fix typo in netty_epoll_native_splice0() and offset accounting in AbstractEpollStreamChannel::SpliceFdTask. - Modify unit test to include an invocation of the public spliceTo method using non-zero offset. Result spliceTo FD methods work as expected when an offset is provided.
…9360) Motivation: The Http2FrameCodec should be responsible to create the upgrade stream. Modifications: Move code to create stream to Http2FrameCodec Result: More correct responsibility
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: `instanceOf` doesn't perform null check like `getClass()` does. So `instanceOf` may be faster. However, it not true for all cases, as C2 could eliminate these null checks for `getClass()`. Modification: Replaced `string.getClass() == AsciiString.class` with `string instanceof AsciiString`. Proof: ``` @BenchmarkMode(Mode.Throughput) @fork(value = 1) @State(Scope.Thread) @WarmUp(iterations = 5, time = 1, batchSize = 1000) @measurement(iterations = 10, time = 1, batchSize = 1000) public class GetClassInstanceOf { Object key; @setup public void setup() { key = "123"; } @benchmark public boolean getClassEquals() { return key.getClass() == String.class; } @benchmark public boolean instanceOf() { return key instanceof String; } } ``` ``` Benchmark Mode Cnt Score Error Units GetClassInstanceOf.getClassEquals thrpt 10 401863.130 ± 3092.568 ops/s GetClassInstanceOf.instanceOf thrpt 10 421386.176 ± 4317.328 ops/s ```
Motivation: A new EA release for OpenJDK13 was released Modifications: Update EA version Result: Use latest OpenJDK 13 EA on ci
Motivation: We should use latest jdk 1.8 release Modifications: Update to adopt@1.8.212-04 Result: Use latest jdk 1.8 on ci
Motivation: We recently made a change to use ET for the eventfd and not trigger a read each time. This testcase proves everything works as expected. Modifications: Add testcase that verifies thqat the wakeups happen correctly Result: More tests
Motivation: If a read triggers a AbstractHttp2StreamChannel to close we can get an NPE in the read loop. Modifications: Make sure that the inboundBuffer isn't null before attempting to continue the loop. Result: No NPE. Fixes #9337
… HttpServerCodec is used (#9386) 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 #9300
…9382) Motivation: At the moment we lookup the ChannelHandlerContext used in Http2StreamChannelBootstrap each time the open(...) method is invoked. This is not needed and we can just cache it for later usage. Modifications: Cache ChannelHandlerContext in volatile field. Result: Speed up open(...) method implementation when called multiple times
#9377) 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: There were new openjdk releases Modifications: Update releases to latest Result: Use latest openjdk versions on CI
Motivation: We can easily reuse the Http2FrameStreamEvent instances and so reduce GC pressure as there may be multiple events per streams over the life-time. Modifications: Reuse instances Result: Less allocations
…oRead is false (#9389) Motivation: When using the HTTP/2 multiplex implementation we need to ensure we correctly drain the buffered inbound data even if the RecvByteBufallocator.Handle tells us to stop reading in between. Modifications: Correctly loop through the buffered inbound data until the user does stop to request from it. Result: Fixes #9387. Co-authored-by: Bryce Anderson <banderson@twitter.com>
… reclaimSpace(...) call (#9394) Motivation: We did miss to call reclaimSpace(...) in one case which can lead to the situation of having the Recycler to not correctly reclaim space and so just create new objects when not needed. Modifications: Correctly call reclaimSpace(...) Result: Recycler correctly reclaims space in all situations.
… and the correct handler is used (#9396) Motivation: 3062993 introduced some code change to move the responsibility of creating the stream for the upgrade to Http2FrameCodec. Unfortunaly this lead to the situation of having newStream().setStreamAndProperty(...) be called twice. Because of this we only ever saw the channelActive(...) on Http2StreamChannel but no other events as the mapping was replaced on the second newStream().setStreamAndProperty(...) call. Beside this we also did not use the correct handler for the upgrade stream in some cases Modifications: - Just remove the Http2FrameCodec.onHttpClientUpgrade() method and so let the base class handle all of it. The stream is created correctly as part of the ConnectionListener implementation of Http2FrameCodec already. - Consolidate logic of creating stream channels - Adjust unit test to capture the bug Result: Fixes #9395
Motivation: Docs should have no typos Modifications: Fix a few typos Result: More correct docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Explain here the context, and why you're making that change.
What is the problem you're trying to solve.
Modification:
Describe the modifications you've done.
Result:
Fixes #.
If there is no issue then describe the changes introduced by this PR.