search

python-hyper / hyper

HTTP/2 for Python.
http://hyper.rtfd.org/en/latest/
MIT License
1.05k stars 192 forks source link

Implement compressors composition #338

Closed KOLANICH closed 7 years ago

KOLANICH commented 7 years ago

https://tools.ietf.org/html/rfc7231#section-3.1.2.2

If one or more encodings have been applied to a representation, the sender that applied the encodings MUST generate a Content-Encoding header field that lists the content codings in the order in which they were applied.

Lukasa commented 7 years ago

I don't see any need to support this for compressors. No-one in their right mind does multiple-compression, this has literally never come up in Requests which has enormous usage, so I think we simply don't need to support it.

KOLANICH commented 7 years ago

No-one in their right mind does multiple-compression

I think this part of spec is such because they wanted to allow the features of tweaking compression for data types (s.a. machine code or natural language text) by applying filters.

Lukasa commented 7 years ago

Sure, but that's not really super-relevant in this case. Unless we can point to anyone actually deploying multiple compressors in one go, it doesn't really matter whether we support it or not.

KOLANICH commented 7 years ago

I think that people expect spec impls to follow the specs. If someone started to experiment with compression filters he would have encountered the bug.

Lukasa commented 7 years ago

Ok, what compression filter pipeline are you expecting to see?

KOLANICH commented 7 years ago

Don't know, for now there are no filters defined in the rfc. For example we can think about the filters defined in https://tukaani.org/xz/xz-file-format.txt.

Lukasa commented 7 years ago

So my general position on this is that there is no need for us to ship code to support hypothetical use-cases. As deployed the content-encoding field is basically used only for monolithic compression algorithms. There are no defined specifications for compression algorithms that can be chained. As-and-when they are, we can revisit this decision, but I'm disinclined to add support for a use-case that is not deployed, and has never been deployed in the 20 year history of this protocol.

KOLANICH commented 7 years ago

Then we should not walk the array, but just take the first item in #336

Lukasa commented 7 years ago

Fair enough, sounds like a reasonable position. =)