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

Make LineTooLong exception more detailed about actual data size. #2857

Closed
popravich opened this issue Mar 20, 2018 · 5 comments
Closed

Make LineTooLong exception more detailed about actual data size. #2857

popravich opened this issue Mar 20, 2018 · 5 comments

Comments

@popravich
Copy link
Member

Long story short

Currently seeing such exceptions in logs doesn't help much in understanding how big the actual data was:

Traceback (most recent call last):
  ...
  File "aiohttp/_http_parser.pyx", line 295, in aiohttp._http_parser.HttpParser.feed_data
  File "aiohttp/_http_parser.pyx", line 422, in aiohttp._http_parser.cb_on_header_value
aiohttp.http_exceptions.LineTooLong: 400, message='Got more than 8190 bytes when reading Header value is too long.'

Although HttpParser has the max_field_size argument allowing to control this limit it is better
to know how big received data is to increase this limit to some reasonable value.

So, I'd like to modify LineTooLong exception extending it with one more argument actual_size.

@aio-libs/aiohttp-committers, thoughts?

@kxepal
Copy link
Member

kxepal commented Mar 20, 2018

I think this wouldn't harm.

@asvetlov
Copy link
Member

Agree

@popravich
Copy link
Member Author

I've just found another interesting issue with Py/C parsers:
modifying the tests case in the following way causes this issue

diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py
index 57232ad6..c17f3774 100644
--- a/tests/test_http_parser.py
+++ b/tests/test_http_parser.py
@@ -367,7 +367,7 @@ def test_max_header_field_size(parser):
 
 
 def test_max_header_value_size(parser):
-    name = b'test' * 10 * 1024
+    name = b'A' * 8190
     text = (b'GET /test HTTP/1.1\r\n'
             b'data:' + name + b'\r\n\r\n')
 

I would expect this test to fail for both parsers but it passes for HttpRequestParserPy

tests/test_http_parser.py::test_max_header_value_size[HttpRequestParserPy-pyloop] PASSED     [ 12%] 
tests/test_http_parser.py::test_max_header_value_size[HttpRequestParserPy-uvloop] PASSED     [ 25%] 
tests/test_http_parser.py::test_max_header_value_size[HttpRequestParserC-pyloop] FAILED      [ 37%] 
tests/test_http_parser.py::test_max_header_value_size[HttpRequestParserC-uvloop] FAILED      [ 50%] 
tests/test_http_parser.py::test_max_header_value_size_continuation[HttpRequestParserPy-pyloop] PASSED [ 62%]
tests/test_http_parser.py::test_max_header_value_size_continuation[HttpRequestParserPy-uvloop] PASSED [ 75%]
tests/test_http_parser.py::test_max_header_value_size_continuation[HttpRequestParserC-pyloop] PASSED [ 87%]
tests/test_http_parser.py::test_max_header_value_size_continuation[HttpRequestParserC-uvloop] PASSED [100%]

============================================= FAILURES =============================================
______________________ test_max_header_value_size[HttpRequestParserC-pyloop] _______________________

parser = <aiohttp._http_parser.HttpRequestParserC object at 0x7fab09a23588>                         

    def test_max_header_value_size(parser):       
        name = b'A' * 8190                        
        text = (b'GET /test HTTP/1.1\r\n'         
                b'data:' + name + b'\r\n\r\n')    
                         
        with pytest.raises(http_exceptions.LineTooLong):                                            
>           parser.feed_data(text)                
E           Failed: DID NOT RAISE <class 'aiohttp.http_exceptions.LineTooLong'>                     

tests/test_http_parser.py:375: Failed             
______________________ test_max_header_value_size[HttpRequestParserC-uvloop] _______________________

parser = <aiohttp._http_parser.HttpRequestParserC object at 0x7fab09a23438>                         

    def test_max_header_value_size(parser):       
        name = b'A' * 8190                        
        text = (b'GET /test HTTP/1.1\r\n'         
                b'data:' + name + b'\r\n\r\n')    
                         
        with pytest.raises(http_exceptions.LineTooLong):                                            
>           parser.feed_data(text)                
E           Failed: DID NOT RAISE <class 'aiohttp.http_exceptions.LineTooLong'>                     

tests/test_http_parser.py:375: Failed             
====================================== 3456 tests deselected =======================================
======================= 2 failed, 6 passed, 3456 deselected in 1.17 seconds ========================

@roganov
Copy link
Contributor

roganov commented Apr 17, 2018

Should this be closed? #2863 is merged.

@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.
Projects
None yet
Development

No branches or pull requests

4 participants