seanmonstar / warp

A super-easy, composable, web server framework for warp speeds.
https://seanmonstar.com/post/176530511587/warp
MIT License
9.35k stars 708 forks source link

Client-respecting compression #665

Open bertptrs opened 3 years ago

bertptrs commented 3 years ago

Release 2.3 added output compression, but this compression is applied unconditionally. It would be better to respect the Accept-encoding header (or at least an approximation of it) by only compressing data to clients that actually support it.

I currently have a workaround that looks as follows (with types massively simplified for readability):

pub fn optional_compression(filter: BoxedFilter<(impl Reply,)>) -> impl Filter {
    gzip_supported()
        .and(filter.clone())
        .with(warp::compression::gzip())
        .or(filter)
}

where gzip_supported returns a trivial filter that extracts nothing and causes a rejection whenever the client does not support gzip compression. This workaround mostly works for my use case, but it has a glaring issue: it can cause the entire filter to be evaluated twice for requests that result in a Rejection.

Of course, you can improve upon this method slightly (by for example adding a gzip_not_suported filter guarding the or) but that is error prone and tedious.

Is it possible for the compression filters to check whether they should apply themselves? Or alternatively, how would one fix this themselves?

One more approach I tried and failed to use: have the gzip_supported filter take in the original Reply and pass it to the Rejection if it fails. This doesn't work because you can't get the Reply out of the Rejection in a recover or or_else handler since you only get a shared reference.

jakajancar commented 3 years ago

Kind of related to #669 which I just posted.

With gzip_not_suported the additional problem is that while the wrapped filter is executed only once, the support determination logic is executed twice.

Given how many people use warp for real projects and in production, I kind of suspect I'm missing something really obvious. An "if" statement is not exactly an edge-case :)

jakajancar commented 3 years ago

Actually, are you sure the entire filter will be executed twice? I would expect that after gzip_supported() rejects, filter will not be called.

Of course, there will be two clones of filter, but they are cloned at startup, not called per-request.

bertptrs commented 3 years ago

The filter is executed twice if it resulted in a Rejection the first time, yes.

novacrazy commented 3 years ago

Was there ever a good solution for this?

Seems like a rather glaring issue. Ideally I shouldn't have to put warp behind nginx or similar just to get sane compression.

BratSinot commented 2 years ago

Any update on this?