Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StreamResponse + HTTP/1.0 = corrupted file #476

Closed
vbraun opened this issue Aug 27, 2015 · 8 comments
Closed

StreamResponse + HTTP/1.0 = corrupted file #476

vbraun opened this issue Aug 27, 2015 · 8 comments
Labels
Milestone

Comments

@vbraun
Copy link

vbraun commented Aug 27, 2015

I'm using StreamResponse to serve files. My code is

from aiohttp import web
response = web.StreamResponse()
response.enable_chunked_encoding()
response.content_length = statinfo.st_size
response.content_type = 'application/octet-stream'
response.start(request)
with open(filename, 'rb') as f:
    while True:
        chunk = f.read(CHUNK_SIZE)
        if not chunk:
            return response
        response.write(chunk)
        yield from response.drain()

If I'm using wget to download the file then it works beautifully. But when I'm using Python urllib to download the file, my download is corrupted. The difference seems to be that wget can HTTP/1.1, but urllib only does HTTP/1.0. The network trace is

Host: ...
User-Agent: Python-urllib/1.17

HTTP/1.0 200 OK
CONTENT-LENGTH: 815206
CONTENT-TYPE: application/octet-stream
TRANSFER-ENCODING: chunked
CONNECTION: close
DATE: Thu, 27 Aug 2015 17:31:25 GMT
SERVER: Python/3.4 aiohttp/0.17.2

10000
... binary data ...

and Python saves everything starting at (and including) the 10000 into the file.

I think this is because TRANSFER-ENCODING: chunked is HTTP/1.1 only, and a HTTP/1.0 client is entirely correct in ignoring it. Aiohttp should not send chunk sizes to HTTP/1.0 clients as they almost certainly end up in the data stream.

@asvetlov
Copy link
Member

The only thing that the library can do is forbidding chunked encoding if HTTP version is not 1.1
In your example just remove response.enable_chunked_encoding() line to make it work with HTTP/1.1

@vbraun
Copy link
Author

vbraun commented Aug 31, 2015

I agree. I'm just saying that the library should do that automatically, and not let the user fall into that trap.

@asvetlov
Copy link
Member

@vbraun I don't follow.
Do you agree with raising error on trying to call .enable_chunked_encoding() when http version is HTTP/1.1 or do you propose something else?

@asvetlov asvetlov added this to the 0.18 milestone Sep 1, 2015
@vbraun
Copy link
Author

vbraun commented Sep 1, 2015

No, I thought it should just fall back to not using chunked encoding. I.e. ignore the call to enable_chunked_encoding() on HTTP/1.0. Anything else is just going to result in obscure errors in users of the library, making it unnecessarily difficult to use.

@asvetlov
Copy link
Member

asvetlov commented Sep 1, 2015

No, without chunked encoding you should collect all sent data into buffer before generating headers.
That's because HTTP/1.0 requires Content-Length header set.

Buffering whole response body is not appropriate.
Well, there is ability to use HTTP/0.9 way -- just don't send Content-Length but drop connection after all data sent but I don't encourage this way.

StreamResponse is relative low-level API, so user should care about protocol version and chunked encoding. If you want buffer BODY -- just use web.Response instead.

@vbraun
Copy link
Author

vbraun commented Sep 1, 2015

OK, I do know the content length, and I do want to use StreamResponse since the body can be very large. I already set the response.content_length attribute in the snippet. If I just leave out the response.enable_chunked_encoding() then is it streaming the response?

@asvetlov
Copy link
Member

asvetlov commented Sep 1, 2015

If you know content length and set up response.content_length attr you don't need chunked encoding.
Just send your data slice-by-slice without response.enable_chunked_encoding() call -- it works pretty well for both HTTP/1.0 and HTTP/1.1

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants