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.http_exceptions.BadStatusLine caused by extra CRLF after POST data #1792

Closed
zarnovican opened this issue Apr 6, 2017 · 12 comments
Closed
Labels

Comments

@zarnovican
Copy link

Long story short

Some http clients may send extra CRLF after POST data. These extra two bytes are not accounted in Content-Length. It will cause..

$ python main.py 
======== Running on http://127.0.0.1:8000 ========
(Press CTRL+C to quit)
Error handling request
Traceback (most recent call last):
  File "/home/zarnovic/venv/aiotest/lib/python3.5/site-packages/aiohttp/web_protocol.py", line 276, in data_received
    messages, upgraded, tail = self._request_parser.feed_data(data)
  File "/home/zarnovic/venv/aiotest/lib/python3.5/site-packages/aiohttp/http_parser.py", line 115, in feed_data
    msg = self.parse_message(self._lines)
  File "/home/zarnovic/venv/aiotest/lib/python3.5/site-packages/aiohttp/http_parser.py", line 331, in parse_message
    raise BadStatusLine(line) from None
aiohttp.http_exceptions.BadStatusLine: ''

We have noticed this problem after upgrade to aiohttp 2.x with check_http client (part of Nagios/Icinga).

Bisect shows that this commit 51baae6 broke it. Seems like all 2.x are affected.

I'm inclined to believe that this is a client's problem. Which, I have reported nagios-plugins/nagios-plugins#266. However, this behavior was "broken" by a change in aiohttp, so I'm reporting it also here, for people to find it. Also, it may be a case that aiohttp is too strict in parsing and there may be other clients behaving like check_http.

Feel free to close it, if you believe it is not aiohttp's problem.

Expected behaviour

See linked ticket.

Actual behaviour

See linked ticket.

Steps to reproduce

See linked ticket.

Your environment

aiohttp (2.0.5)

$ /usr/lib64/nagios/plugins/check_http --version
check_http v2.0.3 (nagios-plugins 2.0.3)

There is no proxy/nat/firewall between them. It is reproducible on localhost.

@fafhrd91
Copy link
Member

fafhrd91 commented Apr 6, 2017

Could you test it with c-extension enabled.
aiohttp uses c-based parser as primary parser,
python based headers parser just a backup solution

@zarnovican
Copy link
Author

Hi, @fafhrd91 . Thanks for the suggestion, it only affects pure-python parser.

Btw, c-based parser does not build on old pip 6.0.8 (default on my Fedora).

  Running setup.py install for aiohttp
    building 'aiohttp._websocket' extension
    gcc -pthread -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/usr/include/python3.5m -c aiohttp/_websocket.c -o build/temp.linux-x86_64-3.5/aiohttp/_websocket.o
    gcc -pthread -shared -Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld build/temp.linux-x86_64-3.5/aiohttp/_websocket.o -L/usr/lib64 -lpython3.5m -o build/lib.linux-x86_64-3.5/aiohttp/_websocket.cpython-35m-x86_64-linux-gnu.so
    building 'aiohttp._http_parser' extension
    gcc -pthread -Wno-unused-result -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/usr/include/python3.5m -c aiohttp/_http_parser.c -o build/temp.linux-x86_64-3.5/aiohttp/_http_parser.o
    aiohttp/_http_parser.c:445:47: fatal error: ../vendor/http-parser/http_parser.h: No such file or directory
    compilation terminated.
    ************************************************************
    Cannot compile C accelerator module, use pure python version
    ************************************************************

With pip-9.0.1, it will install wheel package of aiohttp.

@fafhrd91
Copy link
Member

fafhrd91 commented Apr 6, 2017

try to install aiohttp from 2.0 branch

@zarnovican
Copy link
Author

Same result..

$ deactivate
$ mkvirtualenv --python=python3.5 aiotest
$ pip install git+https://github.com/aio-libs/aiohttp.git@2.0
You are using pip version 6.0.8, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
...
  Running setup.py install for aiohttp
...
    gcc: error: aiohttp/_websocket.c: No such file or directory
    gcc: fatal error: no input files
    compilation terminated.

But we digress. The topic of this issue is http parsing. I'm grateful that you gave me a workaround which we can implement. We should have been using c-based aiohttp all along. Upgrading pip before installing aiohttp is totally fine by me. Thanks!

@fafhrd91
Copy link
Member

fafhrd91 commented Apr 6, 2017

you need to install cython

@zarnovican
Copy link
Author

@fafhrd91 version 2.0.6-1 from PyPI installs fine (even on pip 6.x and without cython). Thanks!

@fafhrd91
Copy link
Member

@zarnovican so do you have same problem with c-based parser?

@zarnovican
Copy link
Author

@fafhrd91 No. The problem does not exhibit there.

AFAIK py-parser and c-parser are still not equal with regard to CRLF handling. Something like this can re-surface again. Closing the ticket anyway. Feel free to reopen..

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

while implementing parser in rust, I noticed all parsers actually remove CRLF at the begging of stream.
py http parser should remove CRLF as well

@fafhrd91
Copy link
Member

fixed in master

@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.

@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

2 participants