这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@yawkat
Copy link
Contributor

@yawkat yawkat commented Sep 16, 2025

Motivation:

The current decompression decoders have a few issues:

  • They're bound to ChannelHandler, so for HTTP, they need to use an EmbeddedChannel with all the associated overhead.
  • They do not support backpressure, so the zip bomb fix could be better.
  • Each one has about 5 constructor overloads

Modification:

  • Introduce a new Decompressor API not based on ChannelHandler. It is similar in design to the brotli API, with separate operations for adding input and removing output.
  • Implement decompressors for all supported formats. The code is copied from the decoders, which remain untouched. No effort is made to reduce code duplication. This is why the PR is enormous.
  • Use builders as a unified way of constructing all the new decompressors, instead of constructor overloads.
  • Implement a BackpressureDecompressionHandler that uses the new Decompressor API.

Result:

Lower overhead, better backpressure handling.

Work yet to be done:

  • Deprecate old decoders, suggesting replacement with BackpressureDecompressionHandler
  • Create a replacement for HttpContentDecoder based on the new decompressors

@yawkat
Copy link
Contributor Author

yawkat commented Oct 2, 2025

@normanmaurer I've added new logic that allows a number of buffers to be forwarded before a readComplete, and a test. I hope that works for you. The logic is getting complicated though. PTAL.

I've also changed the handler API to create the decompressor lazily, in handlerAdded. This looks a bit odd (you pass the decompressor builder, not the decompressor, to the handler) but I think it works as an API and it's safer.

@yawkat
Copy link
Contributor Author

yawkat commented Oct 7, 2025

I've added a HttpContentDecoder and -decompressor replacement, based on the BackpressureDecompressionHandler. I've also deprecated all the old decoders.

I still need to migrate DelegatingDecompressorFrameListener. It is a bit more difficult because it uses a different flow control mechanism. Probably it needs an independent Decompressor-based implementation, not based on BackpressureDecompressionHandler.

Finally, I changed the decompressor API a little bit so that the alloc is required by the .build call, not when creating the builder. This complements the change in 9279fc0. Now, the BackpressureDecompressionHandler will use the alloc from the context, and you don't need to manually pass in the alloc at all before that.

@yawkat
Copy link
Contributor Author

yawkat commented Oct 10, 2025

Okay, I rebuilt the decompression codecs again. HTTP/1.1 pipelining has a lot of edge cases to handle. I decided to build a central AbstractBackpressureDecompressionHandler which is designed for HTTP/1.1 pipelining, and then also extend it for BackpressureDecompressionHandler, since BackpressureDecompressionHandler is basically just the base case without pipelining. Easier than maintaining two separate backpressure implementations.

I've also built a Http2FrameListener based on the new decompressors. It appears to be a lot simpler than the ChannelHandler approach, unless I missed some edge cases.

I'm going on vacation for a few days so I might be slow to answer now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants