enable_compression causes exception on HTTP/1.0

[hubo1016]

Long story short

call set_compression() on a response object and return it to aiohttp server causes a RuntimeError ("Using chunked encoding is forbidden for HTTP/1.0")

Expected behaviour

There should be at least two ways to use compression on HTTP/1.0:

1. Fully compress the content, and return the length of the compressed data in the Content-Length header;
2. Do not return Content-Length header, return the compressed stream and set Connection: close. Close the connection after transmit.

Actual behaviour

An runtime error is raised, and the connection is closed immediately.

In fact, it seems that by default, even a HTTP/1.1 request is responded with a HTTP/1.0 response, so it always raises this exception.

Steps to reproduce

Call enable_compression on any response object before returning.

Comment

I think it is absolutely not correct to enable chunked encoding on compression. Content-Encoding is not a part of transfer, it is a part of the content. It should never affect the transfer encoding type.

[hubo1016]

Sorry, the "HTTP/1.1 is responsed with HTTP/1.0" is caused by a forwarding nginx.

[hubo1016]

A related problem also exists on the client side:
seems that the client always assume chunked encoding on "content-encoding: deflate" ?

[fafhrd91]

we do not know final size of payload, so it is just simpler to switch to chunked encoding.
nothing prevents you from setting content-length and send compressed payload.

[fafhrd91]

I am not interested in changing current behavior. we may use compression without chunking for simple web.Request but for streaming response it seems easier to allow developer to choose what he wants.

@hubo1016 how would you fix this?

[hubo1016]

@fafhrd91 I think compress with "Content-Encoding" should not affect the transfer encoding, they are on different layer. What is the expected behavior when sending a stream data without a specified length on a HTTP/1.0 connection? (According to RFC, the server should send a "Connection: close" header and shutdown the connection write side after all payload are sent, read and drop extra messages from the client which may be pipelining requests, then close the connection.)

I will dig deeper when I have time. Maybe it is just simplier to create a speicifed "content" which is compressed, either as a fixed-length bytes or a stream. Then we use the exactly same tranfer technology to transfer the compressed payload acrossing the HTTP channel.

[fafhrd91]

@hubo1016 would you like to work on this issue?

[fafhrd91]

seems, nobody wants to work on this. I suggest to close.

@asvetlov ?

[asvetlov]

The issue in not in my priority list but please postpone the closing.
We could implement palliative solution by raising an exception with comprehensive error message on enabling compression for HTTP 1.0 as an option (both server and client side).
Specifying explicit compression header with sending pre-compressed body is still available workaround.
But yes, I have no strong motivation for supporting HTTP 1.0 quirks.

[hubo1016]

@fafhrd91 Finally I have time for this issue, but firstly I want to read the existing codes and evaluate the effort. I will have a conclusion in a week.

[hubo1016]

@fafhrd91 @asvetlov I have read the related code. The fix is quite straight-forward: instead of set _chunked directly, just remove the Content-Length header instead. Also the content-length property in Response class must be re-implemented to return the length of the compressed data.

Still some more tests should be added for this new behavior, including tests with version set to HTTP/1.0.

There is also some related issues and I need your opinions:

1. According to RFC (https://tools.ietf.org/html/rfc7230#section-4.2.2), a "deflate" encoding should be a zlib format deflate stream, but aiohttp currently generates "raw" stream (by setting wbits=-15), which is mentioned as "non-conformant implementations" in the RFC. Should we replace it with wbits=15 to generate a zlib-wrapped stream?
2. Content decoding is currently deeply planted in the HttpParser, which might be a problem because Content-Encoding is actually an attribute of the content (same as Content-Type, different from Transfer-Encoding). The client should be able to keep the compressed data without decoding it when necessary. Also, if a partial part of the payload (Status 206) is requested with Range header (as mentioned in https://tools.ietf.org/html/rfc7233), the content cannot be decoded even if the content-encoding is "gzip" or "deflate". We might choose to keep the data untouched and decode them in Request or ClientResponse (or not if explictly set to "no auto-decoding"), but this decision could be hard to make.

I will submit the pull request after reparing unittests. The current status can be viewed in hubo1016#2

[mickeyl]

@hubo1016 the "HTTP/1.1 is responsed with HTTP/1.0" is caused by a forwarding nginx. Why is that so and can it be fixed in the nginx config?

[hubo1016]

@mickeyl our configuration did not set the http version, so it defaults to HTTP/1.0, it can be fixed by setting the http version. But I still think it should be fixed.

[mickeyl]

Thanks, I have since found the neccessary proxy_http_version 1.1; statement. In my opinion, nginx is clearly validating the spec here w/ chunked encoding, but I agree, we don't live in a perfect world and should try to be compatible, if it doesn't destabilize the codebase for the normal cases.