tower-http
tower-http copied to clipboard
Request decompression
Motivation
Currently compression and decompression of response bodies are supported with the Compression and Decompression services. Issue #276 proposes to also implement decompression for request bodies, this PR addresses that. To complete the compression and decompression modules, request compression could be added in another PR.
Solution
RequestDecompression decompresses the request body with the algorithm defined in the Content-Encoding header, it does not support multiple/stacked algorithms. When an algorithm is not supported it will return a 415 Unsupported Media Type by default, but it can also be configured to pass-through the request to the inner service.
Questions
This is my first PR and I am not sure if I am going in the right direction, I would already like some feedback while working on the issue.
Refactor decompression module
Will be done in a different PR.
Service and Future implementation
The service and future are based on the Decompression service and ResponseFuture, but I have the feeling the constraints are too complex. Can these be simplified?
And the same for the boxing of the bodies and errors, has this been done correctly?
API
I have copied the layer implementation from the request decompression layer to make the API uniform. The extra functionality is the pass-through of unsupported encodings, maybe later this can be changed into a single final decompression, that return the 415, and a stacked decompression that pops an encoding and pass-through to the next service.
Would this be a good first way to go?
General code style / conventions
Since I have the basic functionalities tested I can now refactor code. Are there any structures that can be written clearer or more concise? Or are there things that do not follow Rust or Tower conventions?
Todos
- [x] Documentation
- [x] Non-breaking
decompression::Decompression, including docs - [x] Refactor code structure / style
Thank you
Thank you for looking into this.
Is it possible for you to split things up?
Of course, where should I locate the request decompression module and what should I name it?
Also how do you think we should name the layer and service? RequestDecompression or just Decompression distiquished by its module name?
I'd call it RequestDecompression and put it in the decompression module if possible
I think we should also consider making separate middleware for (de)compressing requests and responses rather than making one middleware do both. It's easy to apply both if that's what users want.
I think we should also consider making separate middleware for (de)compressing requests and responses rather than making one middleware do both.
That is what I have done, right?
Yes sorry. It was mostly a note to myself. Haven't actually read your code yet 😅
I think most of the PR is completed and can be reviewed. Although my questions, in the first message, still stand. @davidpdrsn could you run the workflow?
Thank you
Thanks!
I'm not sure when I'll get time to review this. Going on vacation in a few days. So might not get time before I'm back in a few weeks.
I'm not sure when I'll get time to review this. Going on vacation in a few days. So might not get time before I'm back in a few weeks.
Have a good vacation. I will see if I can work on some other PR or maybe I will take some vacation as well. :)
@davidpdrsn any chance you are back from vacation? :pray: I'd also hugely appreciate getting this merged :heart:
I am 😊 I'll try and have a look soon.
Thank you
@davidpdrsn Thanks! Hope you had a lovely vacation
Do you have any critique on conventions, design or anything else?
LGTM!
I still wanna look through. Please remain patient.
I still wanna look through. Please remain patient.
Thank you. That's what I expected, but maybe @Nehliin has some critique as well
Any progress on getting this merged? At my workplace we have a need to support request decompression in our Axum/Hyper based services, and it would be much better if we get support from that in tower-http rather than doing it in our own codebase.
Apologies if this question is asking the obvious. Would it be possible to specify the maximum permitted request body size after decompression in a lazy way somehow, ie. without uncompressing the whole payload, as to avoid excessive memory consumption?
Apologies if this question is asking the obvious. Would it be possible to specify the maximum permitted request body size after decompression in a lazy way somehow, ie. without uncompressing the whole payload, as to avoid excessive memory consumption?
@mmd-osm As far as I can tell, this is doing streaming decompression, so you should be able to apply a tower_http::limit::RequestBodyLimitLayer layer to limit the number of bytes in the request body. Applying that layer after the request decompression layer should apply the limit to the decompressed bytes.
Oh and CI should be fixed if you merge the latest master.
Thank you for reviewing David. I have made the changes you requested.