Fishrock123 / tide-compress

Outgoing compression middleware for the Tide web framework.
https://crates.io/crates/tide-compress
Other
24 stars 7 forks source link

Cache-Control: no-transform behavior #9

Open u5surf opened 2 years ago

u5surf commented 2 years ago

https://github.com/Fishrock123/tide-compress/blob/845698b90b60d12d09f3db742962a3990310e318/src/middleware.rs#L125-L134

I consider that we don't have to care any Cache-Control: no-transform to compress any contents.

For example, Nginx gzip filter doesn't care this headers, also the ngx_brotli module does. https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_gzip_filter_module.c#L220 https://github.com/google/ngx_brotli/blob/master/filter/ngx_http_brotli_filter_module.c

Why these Nginx module don't care this point? Because, the rfc 9110 says that Cache-Control: no-transform does not apply to message transformations that do not change the content, such as the addition or removal of transfer codings. Thus I consider that the compression features indicates a such message transformations(addition Transfer-Encoding: gzip etc...), it should be available these features.

A proxy MUST NOT transform the content ([Section 6.4](https://www.rfc-editor.org/rfc/rfc9110.html#content)) of a response message that contains a no-transform cache directive ([Section 5.2.2.6](https://www.rfc-editor.org/rfc/rfc9111#section-5.2.2.6) of [[CACHING](https://www.rfc-editor.org/rfc/rfc9110.html#CACHING)]). 
Note that this does not apply to message transformations that do not change the content, such as the addition or removal of transfer codings ([Section 7](https://www.rfc-editor.org/rfc/rfc9112#section-7) of [[HTTP/1.1](https://www.rfc-editor.org/rfc/rfc9110.html#HTTP11)]).

https://www.rfc-editor.org/rfc/rfc9110.html

If you agree this approach, I would like to work on this issue. Just removing the procedure to confirm the Cache-Control: no-transform.

Fishrock123 commented 2 years ago

Reading through the docs at https://tools.ietf.org/html/rfc7234#section-5.2.2.4 / https://www.rfc-editor.org/rfc/rfc7230#section-5.7.2 / https://www.rfc-editor.org/rfc/rfc7230#section-4 - which seem a bit more clear, it seems you may be correct.

This check was originally taken from expressjs/compression, specifically, these lines: https://github.com/expressjs/compression/blob/ad5113b98cafe1382a0ece30bb4673707ac59ce7/index.js#L270-L277

The issue that added it there made a not-unreasonable case for it: https://github.com/expressjs/compression/issues/51

However, I think we can do better in Rust / Tide. For that kind of use-case we could export a special type and detect that on the request extensions, like a struct NoCompress;.