From 717e57f0f0cc8cbfe8ce9e4db09dd4119d661be5 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Sat, 31 Oct 2020 10:10:29 -0500 Subject: [PATCH 1/3] Add, test support for chunk extensions The chunk line is read up to the terminating "\r\n", then any extensions are ignored by only using the string up to the first ";" (if any) when determining the length of the chunk. Resources: https://tools.ietf.org/html/rfc7230#section-4.1.1 https://www.boost.org/doc/libs/1_74_0/libs/beast/doc/html/beast/using_http/chunked_encoding.html --- adafruit_requests.py | 4 +- tests/chunk_test.py | 94 +++++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 46 deletions(-) diff --git a/adafruit_requests.py b/adafruit_requests.py index fc41a8b..36a4d81 100755 --- a/adafruit_requests.py +++ b/adafruit_requests.py @@ -212,7 +212,7 @@ def _readinto(self, buf): # Consume trailing \r\n for chunks 2+ if self._remaining == 0: self._throw_away(2) - chunk_header = self._readto(b";", b"\r\n") + chunk_header = self._readto(b"\r\n").split(b";", 1)[0] http_chunk_size = int(bytes(chunk_header), 16) if http_chunk_size == 0: self._chunked = False @@ -253,7 +253,7 @@ def close(self): self._throw_away(self._remaining) elif self._chunked: while True: - chunk_header = self._readto(b";", b"\r\n") + chunk_header = self._readto(b"\r\n").split(b";", 1)[0] chunk_size = int(bytes(chunk_header), 16) if chunk_size == 0: break diff --git a/tests/chunk_test.py b/tests/chunk_test.py index ddbd048..edd50b0 100644 --- a/tests/chunk_test.py +++ b/tests/chunk_test.py @@ -9,7 +9,7 @@ headers = b"HTTP/1.0 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n" -def _chunk(response, split): +def _chunk(response, split, extra=b""): i = 0 chunked = b"" while i < len(response): @@ -19,7 +19,11 @@ def _chunk(response, split): chunk_size = remaining new_i = i + chunk_size chunked += ( - hex(chunk_size)[2:].encode("ascii") + b"\r\n" + response[i:new_i] + b"\r\n" + hex(chunk_size)[2:].encode("ascii") + + extra + + b"\r\n" + + response[i:new_i] + + b"\r\n" ) i = new_i # The final chunk is zero length. @@ -28,56 +32,58 @@ def _chunk(response, split): def test_get_text(): - pool = mocket.MocketPool() - pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) - c = _chunk(text, 33) - print(c) - sock = mocket.Mocket(headers + c) - pool.socket.return_value = sock + for extra in (b"", b";blahblah; blah"): + pool = mocket.MocketPool() + pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) + c = _chunk(text, 33, extra) + print(c) + sock = mocket.Mocket(headers + c) + pool.socket.return_value = sock - s = adafruit_requests.Session(pool) - r = s.get("http://" + host + path) + s = adafruit_requests.Session(pool) + r = s.get("http://" + host + path) - sock.connect.assert_called_once_with((ip, 80)) + sock.connect.assert_called_once_with((ip, 80)) - sock.send.assert_has_calls( - [ - mock.call(b"GET"), - mock.call(b" /"), - mock.call(b"testwifi/index.html"), - mock.call(b" HTTP/1.1\r\n"), - ] - ) - sock.send.assert_has_calls( - [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] - ) - assert r.text == str(text, "utf-8") + sock.send.assert_has_calls( + [ + mock.call(b"GET"), + mock.call(b" /"), + mock.call(b"testwifi/index.html"), + mock.call(b" HTTP/1.1\r\n"), + ] + ) + sock.send.assert_has_calls( + [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] + ) + assert r.text == str(text, "utf-8") def test_close_flush(): """Test that a chunked response can be closed even when the request contents were not accessed.""" - pool = mocket.MocketPool() - pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) - c = _chunk(text, 33) - print(c) - sock = mocket.Mocket(headers + c) - pool.socket.return_value = sock + for extra in (b"", b";blahblah; blah"): + pool = mocket.MocketPool() + pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) + c = _chunk(text, 33, extra) + print(c) + sock = mocket.Mocket(headers + c) + pool.socket.return_value = sock - s = adafruit_requests.Session(pool) - r = s.get("http://" + host + path) + s = adafruit_requests.Session(pool) + r = s.get("http://" + host + path) - sock.connect.assert_called_once_with((ip, 80)) + sock.connect.assert_called_once_with((ip, 80)) - sock.send.assert_has_calls( - [ - mock.call(b"GET"), - mock.call(b" /"), - mock.call(b"testwifi/index.html"), - mock.call(b" HTTP/1.1\r\n"), - ] - ) - sock.send.assert_has_calls( - [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] - ) + sock.send.assert_has_calls( + [ + mock.call(b"GET"), + mock.call(b" /"), + mock.call(b"testwifi/index.html"), + mock.call(b" HTTP/1.1\r\n"), + ] + ) + sock.send.assert_has_calls( + [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] + ) - r.close() + r.close() From 9d02a3075589417473febf849a234ef1608d1427 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Fri, 6 Nov 2020 12:24:44 -0600 Subject: [PATCH 2/3] Split tests for better error reporting --- tests/chunk_test.py | 112 ++++++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/tests/chunk_test.py b/tests/chunk_test.py index edd50b0..c9d227b 100644 --- a/tests/chunk_test.py +++ b/tests/chunk_test.py @@ -31,59 +31,69 @@ def _chunk(response, split, extra=b""): return chunked +def do_test_get_text(extra=b""): + pool = mocket.MocketPool() + pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) + c = _chunk(text, 33, extra) + print(c) + sock = mocket.Mocket(headers + c) + pool.socket.return_value = sock + + s = adafruit_requests.Session(pool) + r = s.get("http://" + host + path) + + sock.connect.assert_called_once_with((ip, 80)) + + sock.send.assert_has_calls( + [ + mock.call(b"GET"), + mock.call(b" /"), + mock.call(b"testwifi/index.html"), + mock.call(b" HTTP/1.1\r\n"), + ] + ) + sock.send.assert_has_calls( + [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] + ) + assert r.text == str(text, "utf-8") + def test_get_text(): - for extra in (b"", b";blahblah; blah"): - pool = mocket.MocketPool() - pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) - c = _chunk(text, 33, extra) - print(c) - sock = mocket.Mocket(headers + c) - pool.socket.return_value = sock - - s = adafruit_requests.Session(pool) - r = s.get("http://" + host + path) - - sock.connect.assert_called_once_with((ip, 80)) - - sock.send.assert_has_calls( - [ - mock.call(b"GET"), - mock.call(b" /"), - mock.call(b"testwifi/index.html"), - mock.call(b" HTTP/1.1\r\n"), - ] - ) - sock.send.assert_has_calls( - [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] - ) - assert r.text == str(text, "utf-8") + do_test_get_text() +def test_get_text_extra(): + do_test_get_text(b';blahblah; blah') -def test_close_flush(): + +def do_test_close_flush(extra=b''): """Test that a chunked response can be closed even when the request contents were not accessed.""" - for extra in (b"", b";blahblah; blah"): - pool = mocket.MocketPool() - pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) - c = _chunk(text, 33, extra) - print(c) - sock = mocket.Mocket(headers + c) - pool.socket.return_value = sock - - s = adafruit_requests.Session(pool) - r = s.get("http://" + host + path) - - sock.connect.assert_called_once_with((ip, 80)) - - sock.send.assert_has_calls( - [ - mock.call(b"GET"), - mock.call(b" /"), - mock.call(b"testwifi/index.html"), - mock.call(b" HTTP/1.1\r\n"), - ] - ) - sock.send.assert_has_calls( - [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] - ) + pool = mocket.MocketPool() + pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) + c = _chunk(text, 33, extra) + print(c) + sock = mocket.Mocket(headers + c) + pool.socket.return_value = sock + + s = adafruit_requests.Session(pool) + r = s.get("http://" + host + path) + + sock.connect.assert_called_once_with((ip, 80)) + + sock.send.assert_has_calls( + [ + mock.call(b"GET"), + mock.call(b" /"), + mock.call(b"testwifi/index.html"), + mock.call(b" HTTP/1.1\r\n"), + ] + ) + sock.send.assert_has_calls( + [mock.call(b"Host: "), mock.call(b"wifitest.adafruit.com"),] + ) + + r.close() + +def test_close_flush(): + do_test_close_flush() - r.close() +def test_close_flush_extra(): + do_test_close_flush(b';blahblah; blah') From 8079252803d7f4db8524776576ae8955283fef99 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Mon, 9 Nov 2020 21:19:39 -0600 Subject: [PATCH 3/3] run black --- tests/chunk_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/chunk_test.py b/tests/chunk_test.py index c9d227b..0982652 100644 --- a/tests/chunk_test.py +++ b/tests/chunk_test.py @@ -57,14 +57,16 @@ def do_test_get_text(extra=b""): ) assert r.text == str(text, "utf-8") + def test_get_text(): do_test_get_text() + def test_get_text_extra(): - do_test_get_text(b';blahblah; blah') + do_test_get_text(b";blahblah; blah") -def do_test_close_flush(extra=b''): +def do_test_close_flush(extra=b""): """Test that a chunked response can be closed even when the request contents were not accessed.""" pool = mocket.MocketPool() pool.getaddrinfo.return_value = ((None, None, None, None, (ip, 80)),) @@ -92,8 +94,10 @@ def do_test_close_flush(extra=b''): r.close() + def test_close_flush(): do_test_close_flush() + def test_close_flush_extra(): - do_test_close_flush(b';blahblah; blah') + do_test_close_flush(b";blahblah; blah")