permessage-deflate: implement context takeover #26
Conversation
This file contains 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
pushkarnk
force-pushed
the
context-takeover-impl
branch
from
e915cbf to
b05842e
Compare
pushkarnk
force-pushed
the
context-takeover-impl
branch
from
b05842e to
59cb531
Compare
|
Seeing some autobahn failures. Investigating. |
|
For future reference: every compression test in autobahn sends this header |
|
A regression was introduced in release |
|
Submitted #28 |
pushkarnk
force-pushed
the
context-takeover-impl
branch
from
59cb531 to
7e108ba
Compare
|
#28 seems to be fixing all the issues we saw. I ran the compression tests from autobahn on Linux and all of them pass. |
added 2 commits
June 20, 2019 10:11
The compression format of the output produced by zlibs deflater is decided by the windowBits values it is configured with. We used positive windowBits values which, according to the zlib manual, emits the zlib stream starting with a zlib header. WebSockets use raw deflate streams, and not zlib streams. Consequently, we had to strip the zlib header. The deflater can be configured to emit a raw deflate format by using negative values for the windowBits. With this in place, there's no zlib header in the output stream.
The server_max_window_bits value sent by the client in the negotiation offer is used to configure the deflater on the server. This acceptance is also notified to the client by returning the server_max_window_bits header in the negotiation response. A special case arises when we receive server_max_window_bits = 8. There's an open zlib issue with the window size of 256 (windoBits=8). For zlib streams, the library silently changes the windowBits to 9 and informs the inflater via the zlib header. However, this apparent hack is not feasible for raw deflate streams, which are used in WebSockets. To take care of this fact, zlib has been patched to reject a window size of 256, via a deflateInit2() failure. Details here: madler/zlib#171 As a result, we need to handle a window size of 256 (windowBits=8) as a special case. We silently change it to 9 and inform the client via a suitable header in the negotiation response.
pushkarnk
force-pushed
the
context-takeover-impl
branch
from
5431beb to
c9d9b44
Compare
Without context takeover, the deflater and inflater are initialized and closed on every message. However, with context takeover, the deflater and inflater persist for the entire lifetime of the connection. They need to be closed on a connection close. The deflater is on a ChannelOutboundHandler and must be closed when the WebSocketConnections issues a channel.close(). The inflater is on a ChannelInboundHandler, it must be closed when we receive a `closeConnection` frame from the remote peer.
bridger
reviewed
Jun 30, 2019
bridger
approved these changes
Jun 30, 2019
|
Thanks @bridger for confirming! And thanks for raising all those issues, its helped us fix some fundamental bugs :) |
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.
This is an an implementation of context takeover.
For every WebSocket connection on which compression using permessage-deflate is negotiated, we configure a compressor and a decompressor on the channel pipeline. Context takeover is then implemented as: