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

aiohttp.Client can't parse Trailer headers and throws exception #1619

Closed
yurifedoseev opened this issue Feb 10, 2017 · 13 comments
Closed

aiohttp.Client can't parse Trailer headers and throws exception #1619

yurifedoseev opened this issue Feb 10, 2017 · 13 comments
Labels
Milestone

Comments

@yurifedoseev
Copy link

yurifedoseev commented Feb 10, 2017

Long story short

I work with some storage service via http. It sends me chunked response and several trailing headers after the body. All of this trailers are specified in a Trailer header according to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer

First problem: the client completely ignores this headers and doesn't parse them. Such behaviour violates HTTP 1.1

However, there is a bigger problem. If you use a single session the parsers would start the next parsing from the previous (!) response - beginning from these trailing headers. So the parser thinks it's the first line of new response and throws exception because it didn't find HTTP verb in it.
Therefore I can't reuse session in my app which leads to performance problems :(

Expected behaviour

  1. The client has to parse trailing headers in chunked response
  2. The next response should not be affected by any data from previous

Actual behaviour

  1. Client ignores trailer headers.
  2. The next response is affected by trailer headers from previous. The parser throws BadStatusLine exception.

Steps to reproduce

  1. Create server which respond which send chunked response with trailing headers after the body. A special Trailer header should be placed before the body and contains all trailing headers names.
  2. Create a new ClientSession. All next responses do via this session
  3. Send the first request to your server. You won't find any trailer headers in the response
  4. Send the second request. The client session will throw exception BadStatusLine

Checked on macOs 10.12.3 and Ubuntu Trusty 14.04

@fafhrd91
Copy link
Member

seems example from https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer page
is wrong. am i right?

it should be like this

HTTP/1.1 200 OK 
Content-Type: text/plain 
Transfer-Encoding: chunked
Trailer: Expires

7\r\n 
Mozilla\r\n 
9\r\n 
Developer\r\n 
7\r\n 
Network\r\n 
0\r\n 
Expires: Wed, 21 Oct 2015 07:28:00 GMT\r\n
\r\n

@yurifedoseev
Copy link
Author

Yes, you're right. I used it just to illustrate headers and trailers order

@fafhrd91
Copy link
Member

ok, should be easy to fix

@fafhrd91 fafhrd91 added this to the 2.0 milestone Feb 16, 2017
@fafhrd91
Copy link
Member

so new parser should strip trailers properly. but should we provide some api to access trailers data?

@fafhrd91
Copy link
Member

fixed in master

@fafhrd91
Copy link
Member

how to access trailer is in #1652

@yurifedoseev
Copy link
Author

In my case I don't need these headers, however, it'd a nice feature to have (access to trailer)
Thanks a lot for your fix

@yurifedoseev
Copy link
Author

Hi, can you say when do you plan to release a new version with this fix?

@fafhrd91
Copy link
Member

I am planing to release 2.0dev next Tuesday.
2.0 final release, probably week after

@yurifedoseev
Copy link
Author

Hi, I've just updated to version 2.0.5
Unfortunately, the fix didn't solve the problem with Trailers. I have the same error when use single session for requests

@fafhrd91 fafhrd91 reopened this Apr 14, 2017
@fafhrd91
Copy link
Member

Could you create test case

@asvetlov
Copy link
Member

asvetlov commented Oct 1, 2017

Actually the fix works as expected.

@lock
Copy link

lock bot commented Oct 28, 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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 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

3 participants