From db41663ea2fa84cb77d548934c92d3d5c26c2c05 Mon Sep 17 00:00:00 2001 From: Jordon Phillips Date: Thu, 12 Sep 2024 19:11:18 +0200 Subject: [PATCH] Update event stream tests (#3254) This fixes a pair of event stream tests. Implementations of `application/vnd.amazon.eventstream` are expected to check the CRC before attempting to semantically parse the prelude. The tests meant to enforce that were present, but the wrong error was being asserted. This change fixes the ordering of checks, fixes the tests, and adds two new test cases to make assertions about the error types that were previously being applied to the old tests. --- botocore/eventstream.py | 2 +- tests/unit/test_eventstream.py | 45 +++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/botocore/eventstream.py b/botocore/eventstream.py index b7999a6e50..1f725ff412 100644 --- a/botocore/eventstream.py +++ b/botocore/eventstream.py @@ -466,9 +466,9 @@ def _parse_prelude(self): prelude_bytes = self._data[:_PRELUDE_LENGTH] raw_prelude, _ = DecodeUtils.unpack_prelude(prelude_bytes) prelude = MessagePrelude(*raw_prelude) - self._validate_prelude(prelude) # The minus 4 removes the prelude crc from the bytes to be checked _validate_checksum(prelude_bytes[: _PRELUDE_LENGTH - 4], prelude.crc) + self._validate_prelude(prelude) return prelude def _parse_headers(self): diff --git a/tests/unit/test_eventstream.py b/tests/unit/test_eventstream.py index a683e37e7f..26db9758fd 100644 --- a/tests/unit/test_eventstream.py +++ b/tests/unit/test_eventstream.py @@ -218,7 +218,7 @@ b"\x00\x00\x00=\xff\x00\x01\x02\x07\xfd\x83\x96\x0ccontent-type\x07\x00" b"\x10application/json{'foo':'bar'}\x8d\x9c\x08\xb1" ), - InvalidHeadersLength, + ChecksumMismatch, ) CORRUPTED_HEADERS = ( @@ -231,7 +231,7 @@ CORRUPTED_LENGTH = ( b"\x01\x00\x00\x1d\x00\x00\x00\x00\xfdR\x8cZ{'foo':'bar'}\xc3e96", - InvalidPayloadLength, + ChecksumMismatch, ) CORRUPTED_PAYLOAD = ( @@ -247,6 +247,31 @@ DuplicateHeader, ) +# In contrast to the CORRUPTED_HEADERS case, this message is otherwise +# well-formed - the checksums match. +INVALID_HEADERS_LENGTH = ( + ( + b"\x00\x00\x00\x3d" # total length + b"\xff\x00\x01\x02" # headers length + b"\x15\x83\xf5\xc2" # prelude crc + b"\x0ccontent-type\x07\x00\x10application/json" # headers + b"{'foo':'bar'}" # payload + b"\x2f\x37\x7f\x5d" # message crc + ), + InvalidHeadersLength, +) + +# In contrast to the CORRUPTED_PAYLOAD case, this message is otherwise +# well-formed - the checksums match. +INVALID_PAYLOAD_LENGTH = ( + b"\x01\x00\x00\x11" # total length + + b"\x00\x00\x00\x00" # headers length + + b"\xf4\x08\x61\xc5" # prelude crc + + b"0" * (16 * 1024**2 + 1) # payload + + b"\x2a\xb4\xc5\xa5", # message crc + InvalidPayloadLength, +) + # Tuples of encoded messages and their expected exception NEGATIVE_CASES = [ CORRUPTED_LENGTH, @@ -254,6 +279,8 @@ CORRUPTED_HEADERS, CORRUPTED_HEADER_LENGTH, DUPLICATE_HEADER, + INVALID_HEADERS_LENGTH, + INVALID_PAYLOAD_LENGTH, ] @@ -311,7 +338,19 @@ def test_all_positive_cases(): assert_message_equal(expected, decoded) -@pytest.mark.parametrize("encoded, exception", NEGATIVE_CASES) +@pytest.mark.parametrize( + "encoded, exception", + NEGATIVE_CASES, + ids=[ + "corrupted-length", + "corrupted-payload", + "corrupted-headers", + "corrupted-headers-length", + "duplicate-headers", + "invalid-headers-length", + "invalid-payload-length", + ], +) def test_negative_cases(encoded, exception): """Test that all negative cases raise the expected exception.""" with pytest.raises(exception):