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

Chunked request #2

Closed
fafhrd91 opened this issue May 6, 2016 · 10 comments
Closed

Chunked request #2

fafhrd91 opened this issue May 6, 2016 · 10 comments

Comments

@fafhrd91
Copy link

fafhrd91 commented May 6, 2016

i am working on httptools integration with aiohttp, some tests are failing with chunking and compression. if i modify httptools tests, it does not pass as well.

CHUNKED_REQUEST1_3 = b'''POST /test.php?a=b+c HTTP/1.2
User-Agent: Fooo
Host: bar
Transfer-Encoding: chunked

b\r\n+\xce\xcfM\xb5MI,I\x04\x00\r\n0\r\n\r\n'''
@1st1
Copy link
Member

1st1 commented May 6, 2016

Can you send me the full source code of a test that fails?

@fafhrd91
Copy link
Author

fafhrd91 commented May 6, 2016

import httptools

import unittest
from unittest import mock

CHUNKED_REQUEST1_3 = b'''POST /test.php?a=b+c HTTP/1.2
User-Agent: Fooo
Host: bar
Transfer-Encoding: chunked

b\r\n+\xce\xcfM\xb5MI,I\x04\x00\r\n0\r\n\r\n'''


class TestRequestParser(unittest.TestCase):

    def test_parser_request_chunked_3(self):
        m = mock.Mock()
        p = httptools.HttpRequestParser(m)

        p.feed_data(CHUNKED_REQUEST1_3)

        self.assertEqual(p.get_method(), b'POST')

        m.on_url.assert_called_once_with(b'/test.php?a=b+c')
        self.assertEqual(p.get_http_version(), '1.2')

        m.on_header.assert_called_with(b'Transfer-Encoding', b'chunked')
        m.on_chunk_header.assert_called_with()
        m.on_chunk_complete.assert_called_with()

        self.assertFalse(m.on_message_complete.called)

@1st1
Copy link
Member

1st1 commented May 7, 2016

Hm. I'm still not sure what's going on here.

It looks like this request is invalid: the length of first chunk is b == 11. The parser correctly calls on_body with b'+g\xce\xcfM\xb5MI,I\x04'. But there is an extra byte x00 which isn't consumed, and there is no exception either...

@1st1
Copy link
Member

1st1 commented May 7, 2016

Got it, I got an extra char in my CHUNKED_REQUEST1_3. Seems like it has troubles with \x00.

@1st1
Copy link
Member

1st1 commented May 7, 2016

Yep, it's a bug in http-parser.c. If you change \x00 to \x01 everything works.

@fafhrd91
Copy link
Author

fafhrd91 commented May 7, 2016

Yes, I noticed that.

On May 6, 2016, at 5:21 PM, Yury Selivanov notifications@github.com wrote:

Yep, it's a bug in http-parser.c. If you change \x00 to \x01 everything works.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@1st1
Copy link
Member

1st1 commented May 7, 2016

Nope. Bug in my code, which is a good thing :) Will commit a fix soon.

@1st1
Copy link
Member

1st1 commented May 7, 2016

Should be fixed in v0.0.6.

@1st1
Copy link
Member

1st1 commented May 7, 2016

The bug was in a len function called on a char*. Cython, without a warning, used strlen, which gave the http-parser a wrong length (because of the \x00 byte).

@1st1
Copy link
Member

1st1 commented May 7, 2016

Anyways, I've updated the code to use buffer protocol -- it's much more efficient.

@1st1 1st1 closed this as completed May 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants