From 0673c7582b80ae1b74f05c1dec26657701ea0caf Mon Sep 17 00:00:00 2001 From: baermat Date: Fri, 26 Apr 2024 15:52:21 +0200 Subject: [PATCH 1/8] fix serialization for sqs http calls --- localstack/aws/protocol/serializer.py | 15 +++++++-- tests/aws/services/sqs/test_sqs.py | 32 +++++++++++++++++++ .../aws/services/sqs/test_sqs.validation.json | 3 ++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/localstack/aws/protocol/serializer.py b/localstack/aws/protocol/serializer.py index f7631360175ef..5350d66eb8b94 100644 --- a/localstack/aws/protocol/serializer.py +++ b/localstack/aws/protocol/serializer.py @@ -1648,11 +1648,20 @@ def _default_serialize(self, xmlnode: ETree.Element, params: str, _, name: str, def _node_to_string(self, root: Optional[ETree.ElementTree], mime_type: str) -> Optional[str]: """Replaces the previously "marked" characters with their encoded value.""" generated_string = super()._node_to_string(root, mime_type) + if generated_string is None: + return None + + generated_string = to_str(generated_string) + # Undo the second escaping of the " + if mime_type == APPLICATION_JSON: + # At this point in time we already dumped the json and all relevant characters are already escaped + generated_string = generated_string.replace(r"__marker__\"__marker__", r"\"") + else: + generated_string = generated_string.replace('__marker__"__marker__', '"') + return ( to_bytes( - to_str(generated_string) - # Undo the second escaping of the & - .replace('__marker__"__marker__', """) + generated_string # Undo the second escaping of the carriage return (\r) .replace("__marker__\r__marker__", " ") ) diff --git a/tests/aws/services/sqs/test_sqs.py b/tests/aws/services/sqs/test_sqs.py index 0f14951160182..b541bbed6ac49 100644 --- a/tests/aws/services/sqs/test_sqs.py +++ b/tests/aws/services/sqs/test_sqs.py @@ -1334,6 +1334,38 @@ def test_external_hostname_via_host_header(self, monkeypatch, sqs_create_queue, kwargs = {"flags": re.MULTILINE | re.DOTALL} assert re.match(rf".*\s*{url}/[^<]+.*", content, **kwargs) + @markers.aws.unknown + def test_marker_serialization_query_protocol( + self, sqs_create_queue, aws_client, region_name, create_iam_role_with_policy + ): + queue_name = f"queue-{short_uid()}" + queue_url = sqs_create_queue(QueueName=queue_name) + message_body = {"foo": "bar"} + aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=json.dumps(message_body)) + + # edge_url = config.internal_service_url() + # headers = mock_aws_request_headers( + # "sqs", + # aws_access_key_id=TEST_AWS_ACCESS_KEY_ID, + # region_name=region_name, + # ) + # payload = f"Action=SendMessage&QueueUrl={queue_url}&MessageBody={message_body}" + # result = requests.post(edge_url, data=payload, headers=headers) + + response = requests.get( + queue_url, + params={"Action": "ReceiveMessage", "MaxNumberOfMessages": 2}, + headers={"Accept": "application/json"}, + ) + parsed_content = json.loads(response.content.decode("utf-8")) + # TODO: this is an error in LocalStack. Usually it should be Messages[0]['Body'] + assert ( + json.loads( + parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["Message"]["Body"] + ) + == message_body + ) + @markers.aws.only_localstack def test_external_host_via_header_complete_message_lifecycle( self, monkeypatch, account_id, region_name diff --git a/tests/aws/services/sqs/test_sqs.validation.json b/tests/aws/services/sqs/test_sqs.validation.json index 81fbf2255d88a..07ee07e03f882 100644 --- a/tests/aws/services/sqs/test_sqs.validation.json +++ b/tests/aws/services/sqs/test_sqs.validation.json @@ -143,6 +143,9 @@ "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_list_queues": { "last_validated_date": "2024-04-30T13:39:55+00:00" }, + "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_query_protocol": { + "last_validated_date": "2024-04-26T11:07:04+00:00" + }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_message_deduplication_id_too_long": { "last_validated_date": "2024-04-30T13:35:34+00:00" }, From 5ee3ab00e226e4a33befabfa3a746e633c7668f9 Mon Sep 17 00:00:00 2001 From: baermat Date: Mon, 29 Apr 2024 07:44:37 +0200 Subject: [PATCH 2/8] adapt unit test --- tests/unit/aws/protocol/test_serializer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/aws/protocol/test_serializer.py b/tests/unit/aws/protocol/test_serializer.py index 84e49d40aedb6..9db8ae33e1b08 100644 --- a/tests/unit/aws/protocol/test_serializer.py +++ b/tests/unit/aws/protocol/test_serializer.py @@ -505,7 +505,6 @@ def test_query_protocol_error_serialization_plain(): # - The original response does not define an encoding. # - There is no newline after the XML declaration. # - The response does not contain a Type nor Detail tag (since they aren't contained in the spec). - # - The original response uses double quotes for the xml declaration. # Most of these differences should be handled equally by parsing clients, however, we might adopt some of these # changes in the future. expected_response_body = ( @@ -513,7 +512,7 @@ def test_query_protocol_error_serialization_plain(): '' "" "ReceiptHandleIsInvalid" - "The input receipt handle "garbage" is not a valid receipt handle." + 'The input receipt handle "garbage" is not a valid receipt handle.' "" "" "static_request_id" From 70ba0ea87ea3263638584a7bb59b1578c9adea18 Mon Sep 17 00:00:00 2001 From: baermat Date: Mon, 29 Apr 2024 08:10:25 +0200 Subject: [PATCH 3/8] fix test for AWS --- tests/aws/services/sqs/test_sqs.py | 50 ++++++++++++------- .../aws/services/sqs/test_sqs.validation.json | 2 +- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/tests/aws/services/sqs/test_sqs.py b/tests/aws/services/sqs/test_sqs.py index b541bbed6ac49..19b666e6c3ee2 100644 --- a/tests/aws/services/sqs/test_sqs.py +++ b/tests/aws/services/sqs/test_sqs.py @@ -1334,37 +1334,49 @@ def test_external_hostname_via_host_header(self, monkeypatch, sqs_create_queue, kwargs = {"flags": re.MULTILINE | re.DOTALL} assert re.match(rf".*\s*{url}/[^<]+.*", content, **kwargs) - @markers.aws.unknown + @markers.aws.validated def test_marker_serialization_query_protocol( - self, sqs_create_queue, aws_client, region_name, create_iam_role_with_policy + self, sqs_create_queue, aws_client, aws_http_client_factory ): queue_name = f"queue-{short_uid()}" queue_url = sqs_create_queue(QueueName=queue_name) message_body = {"foo": "bar"} aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=json.dumps(message_body)) - # edge_url = config.internal_service_url() - # headers = mock_aws_request_headers( - # "sqs", - # aws_access_key_id=TEST_AWS_ACCESS_KEY_ID, - # region_name=region_name, - # ) - # payload = f"Action=SendMessage&QueueUrl={queue_url}&MessageBody={message_body}" - # result = requests.post(edge_url, data=payload, headers=headers) + client = aws_http_client_factory("sqs", region="us-east-1") - response = requests.get( - queue_url, - params={"Action": "ReceiveMessage", "MaxNumberOfMessages": 2}, + if is_aws_cloud(): + endpoint_url = "https://queue.amazonaws.com" + else: + endpoint_url = config.internal_service_url() + + response = client.get( + endpoint_url, + params={"Action": "ReceiveMessage", "QueueUrl": queue_url, "Version": "2012-11-05"}, headers={"Accept": "application/json"}, ) + parsed_content = json.loads(response.content.decode("utf-8")) - # TODO: this is an error in LocalStack. Usually it should be Messages[0]['Body'] - assert ( - json.loads( - parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["Message"]["Body"] + if is_aws_cloud(): + assert ( + json.loads( + parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["messages"][0][ + "Body" + ] + ) + == message_body + ) + + # TODO: this is an error in LocalStack. Usually it should be messages[0]['Body'] + else: + assert ( + json.loads( + parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["Message"][ + "Body" + ] + ) + == message_body ) - == message_body - ) @markers.aws.only_localstack def test_external_host_via_header_complete_message_lifecycle( diff --git a/tests/aws/services/sqs/test_sqs.validation.json b/tests/aws/services/sqs/test_sqs.validation.json index 07ee07e03f882..40ea56746c813 100644 --- a/tests/aws/services/sqs/test_sqs.validation.json +++ b/tests/aws/services/sqs/test_sqs.validation.json @@ -144,7 +144,7 @@ "last_validated_date": "2024-04-30T13:39:55+00:00" }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_query_protocol": { - "last_validated_date": "2024-04-26T11:07:04+00:00" + "last_validated_date": "2024-04-29T06:07:04+00:00" }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_message_deduplication_id_too_long": { "last_validated_date": "2024-04-30T13:35:34+00:00" From 499b836680eff1e8a0562edca4265256e5c5ef8d Mon Sep 17 00:00:00 2001 From: baermat Date: Fri, 3 May 2024 09:27:15 +0200 Subject: [PATCH 4/8] WIP: carriage return still bugged --- localstack/aws/protocol/serializer.py | 25 +++++++++------------ tests/aws/services/sqs/test_sqs.py | 32 +++++++++++++++------------ 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/localstack/aws/protocol/serializer.py b/localstack/aws/protocol/serializer.py index 5350d66eb8b94..f3fc1a10d9c96 100644 --- a/localstack/aws/protocol/serializer.py +++ b/localstack/aws/protocol/serializer.py @@ -1642,7 +1642,7 @@ def _default_serialize(self, xmlnode: ETree.Element, params: str, _, name: str, node.text = ( str(params) .replace('"', '__marker__"__marker__') - .replace("\r", "__marker__\r__marker__") + .replace("\r", "__marker__-r__marker__") ) def _node_to_string(self, root: Optional[ETree.ElementTree], mime_type: str) -> Optional[str]: @@ -1650,24 +1650,19 @@ def _node_to_string(self, root: Optional[ETree.ElementTree], mime_type: str) -> generated_string = super()._node_to_string(root, mime_type) if generated_string is None: return None - generated_string = to_str(generated_string) - # Undo the second escaping of the " + # Undo the second escaping of the & + # Undo the second escaping of the carriage return (\r) if mime_type == APPLICATION_JSON: - # At this point in time we already dumped the json and all relevant characters are already escaped - generated_string = generated_string.replace(r"__marker__\"__marker__", r"\"") + generated_string = generated_string.replace(r"__marker__\"__marker__", r"\"").replace( + "__marker__-r__marker__", r"\r" + ) else: - generated_string = generated_string.replace('__marker__"__marker__', '"') - - return ( - to_bytes( - generated_string - # Undo the second escaping of the carriage return (\r) - .replace("__marker__\r__marker__", " ") + generated_string = generated_string.replace('__marker__"__marker__', '"').replace( + "__marker__-r__marker__", "\r" ) - if generated_string is not None - else None - ) + + return to_bytes(generated_string) def _add_error_tags( self, error: ServiceException, error_tag: ETree.Element, mime_type: str diff --git a/tests/aws/services/sqs/test_sqs.py b/tests/aws/services/sqs/test_sqs.py index 19b666e6c3ee2..bc4169a04c2ab 100644 --- a/tests/aws/services/sqs/test_sqs.py +++ b/tests/aws/services/sqs/test_sqs.py @@ -1335,13 +1335,15 @@ def test_external_hostname_via_host_header(self, monkeypatch, sqs_create_queue, assert re.match(rf".*\s*{url}/[^<]+.*", content, **kwargs) @markers.aws.validated - def test_marker_serialization_query_protocol( + def test_marker_serialization_json_protocol( self, sqs_create_queue, aws_client, aws_http_client_factory ): queue_name = f"queue-{short_uid()}" queue_url = sqs_create_queue(QueueName=queue_name) - message_body = {"foo": "bar"} - aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=json.dumps(message_body)) + # message_body = {"foo": "ba\rr", "foo2": "ba"r""} + # aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=json.dumps(message_body)) + message_body = '{"foo": "ba\rr", "foo2": "ba"r""}' + aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=message_body) client = aws_http_client_factory("sqs", region="us-east-1") @@ -1352,31 +1354,33 @@ def test_marker_serialization_query_protocol( response = client.get( endpoint_url, - params={"Action": "ReceiveMessage", "QueueUrl": queue_url, "Version": "2012-11-05"}, + params={ + "Action": "ReceiveMessage", + "QueueUrl": queue_url, + "Version": "2012-11-05", + "VisibilityTimeout": "0", + }, headers={"Accept": "application/json"}, ) parsed_content = json.loads(response.content.decode("utf-8")) + if is_aws_cloud(): assert ( - json.loads( - parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["messages"][0][ - "Body" - ] - ) + parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["messages"][0][ + "Body" + ] == message_body ) # TODO: this is an error in LocalStack. Usually it should be messages[0]['Body'] else: assert ( - json.loads( - parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["Message"][ - "Body" - ] - ) + parsed_content["ReceiveMessageResponse"]["ReceiveMessageResult"]["Message"]["Body"] == message_body ) + client_receive_response = aws_client.sqs.receive_message(QueueUrl=queue_url) + assert client_receive_response["Messages"][0]["Body"] == message_body @markers.aws.only_localstack def test_external_host_via_header_complete_message_lifecycle( From e3489d6c7e16c4bd832694bd3e850a4b0cfcd60a Mon Sep 17 00:00:00 2001 From: baermat Date: Fri, 3 May 2024 12:13:20 +0200 Subject: [PATCH 5/8] revert direct replacement for non-json, add dumped json message --- localstack/aws/protocol/serializer.py | 5 +++-- tests/aws/services/sqs/test_sqs.py | 8 +++++--- tests/aws/services/sqs/test_sqs.validation.json | 6 ++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/localstack/aws/protocol/serializer.py b/localstack/aws/protocol/serializer.py index f3fc1a10d9c96..c2b80a9ab38d9 100644 --- a/localstack/aws/protocol/serializer.py +++ b/localstack/aws/protocol/serializer.py @@ -1654,12 +1654,13 @@ def _node_to_string(self, root: Optional[ETree.ElementTree], mime_type: str) -> # Undo the second escaping of the & # Undo the second escaping of the carriage return (\r) if mime_type == APPLICATION_JSON: + # At this point the json was already dumped and escaped, so we replace directly. generated_string = generated_string.replace(r"__marker__\"__marker__", r"\"").replace( "__marker__-r__marker__", r"\r" ) else: - generated_string = generated_string.replace('__marker__"__marker__', '"').replace( - "__marker__-r__marker__", "\r" + generated_string = generated_string.replace('__marker__"__marker__', """).replace( + "__marker__-r__marker__", " " ) return to_bytes(generated_string) diff --git a/tests/aws/services/sqs/test_sqs.py b/tests/aws/services/sqs/test_sqs.py index bc4169a04c2ab..08e6b3e8e3dbc 100644 --- a/tests/aws/services/sqs/test_sqs.py +++ b/tests/aws/services/sqs/test_sqs.py @@ -1334,14 +1334,16 @@ def test_external_hostname_via_host_header(self, monkeypatch, sqs_create_queue, kwargs = {"flags": re.MULTILINE | re.DOTALL} assert re.match(rf".*\s*{url}/[^<]+.*", content, **kwargs) + @pytest.mark.parametrize( + argnames="json_body", + argvalues=['{"foo": "ba\rr", "foo2": "ba"r""}', json.dumps('{"foo": "ba\rr"}')], + ) @markers.aws.validated def test_marker_serialization_json_protocol( - self, sqs_create_queue, aws_client, aws_http_client_factory + self, sqs_create_queue, aws_client, aws_http_client_factory, json_body ): queue_name = f"queue-{short_uid()}" queue_url = sqs_create_queue(QueueName=queue_name) - # message_body = {"foo": "ba\rr", "foo2": "ba"r""} - # aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=json.dumps(message_body)) message_body = '{"foo": "ba\rr", "foo2": "ba"r""}' aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=message_body) diff --git a/tests/aws/services/sqs/test_sqs.validation.json b/tests/aws/services/sqs/test_sqs.validation.json index 40ea56746c813..8e9db576c007e 100644 --- a/tests/aws/services/sqs/test_sqs.validation.json +++ b/tests/aws/services/sqs/test_sqs.validation.json @@ -143,6 +143,12 @@ "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_list_queues": { "last_validated_date": "2024-04-30T13:39:55+00:00" }, + "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_json_protocol[\"{\\\\\"foo\\\\\": \\\\\"ba\\\\rr\\\\\"}\"]": { + "last_validated_date": "2024-05-07T07:46:12+00:00" + }, + "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_json_protocol[{\"foo\": \"ba\\rr\", \"foo2\": \"ba"r"\"}]": { + "last_validated_date": "2024-05-07T07:46:10+00:00" + }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_query_protocol": { "last_validated_date": "2024-04-29T06:07:04+00:00" }, From b11cae8b6111be7e8a8e4feacdf54fb07ea71b64 Mon Sep 17 00:00:00 2001 From: baermat Date: Tue, 7 May 2024 10:02:07 +0200 Subject: [PATCH 6/8] reverse unit test changes --- tests/unit/aws/protocol/test_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/aws/protocol/test_serializer.py b/tests/unit/aws/protocol/test_serializer.py index 9db8ae33e1b08..1b1ce1f793959 100644 --- a/tests/unit/aws/protocol/test_serializer.py +++ b/tests/unit/aws/protocol/test_serializer.py @@ -512,7 +512,7 @@ def test_query_protocol_error_serialization_plain(): '' "" "ReceiptHandleIsInvalid" - 'The input receipt handle "garbage" is not a valid receipt handle.' + "The input receipt handle "garbage" is not a valid receipt handle." "" "" "static_request_id" From a467d0b373ac9a1546c0b23af126ee28d0e3889b Mon Sep 17 00:00:00 2001 From: baermat Date: Tue, 7 May 2024 12:36:46 +0200 Subject: [PATCH 7/8] fix comment, re-trigger pipeline --- tests/unit/aws/protocol/test_serializer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/aws/protocol/test_serializer.py b/tests/unit/aws/protocol/test_serializer.py index 1b1ce1f793959..84e49d40aedb6 100644 --- a/tests/unit/aws/protocol/test_serializer.py +++ b/tests/unit/aws/protocol/test_serializer.py @@ -505,6 +505,7 @@ def test_query_protocol_error_serialization_plain(): # - The original response does not define an encoding. # - There is no newline after the XML declaration. # - The response does not contain a Type nor Detail tag (since they aren't contained in the spec). + # - The original response uses double quotes for the xml declaration. # Most of these differences should be handled equally by parsing clients, however, we might adopt some of these # changes in the future. expected_response_body = ( From 38a9aad73791bc9d033c40f689fdf1d938e4e57a Mon Sep 17 00:00:00 2001 From: baermat Date: Tue, 7 May 2024 15:34:29 +0200 Subject: [PATCH 8/8] fix test parameters --- tests/aws/services/sqs/test_sqs.py | 2 +- tests/aws/services/sqs/test_sqs.validation.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/aws/services/sqs/test_sqs.py b/tests/aws/services/sqs/test_sqs.py index 08e6b3e8e3dbc..8a1fbe0cc586b 100644 --- a/tests/aws/services/sqs/test_sqs.py +++ b/tests/aws/services/sqs/test_sqs.py @@ -1344,7 +1344,7 @@ def test_marker_serialization_json_protocol( ): queue_name = f"queue-{short_uid()}" queue_url = sqs_create_queue(QueueName=queue_name) - message_body = '{"foo": "ba\rr", "foo2": "ba"r""}' + message_body = json_body aws_client.sqs.send_message(QueueUrl=queue_name, MessageBody=message_body) client = aws_http_client_factory("sqs", region="us-east-1") diff --git a/tests/aws/services/sqs/test_sqs.validation.json b/tests/aws/services/sqs/test_sqs.validation.json index 8e9db576c007e..0b7e911c76c25 100644 --- a/tests/aws/services/sqs/test_sqs.validation.json +++ b/tests/aws/services/sqs/test_sqs.validation.json @@ -144,10 +144,10 @@ "last_validated_date": "2024-04-30T13:39:55+00:00" }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_json_protocol[\"{\\\\\"foo\\\\\": \\\\\"ba\\\\rr\\\\\"}\"]": { - "last_validated_date": "2024-05-07T07:46:12+00:00" + "last_validated_date": "2024-05-07T13:33:39+00:00" }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_json_protocol[{\"foo\": \"ba\\rr\", \"foo2\": \"ba"r"\"}]": { - "last_validated_date": "2024-05-07T07:46:10+00:00" + "last_validated_date": "2024-05-07T13:33:34+00:00" }, "tests/aws/services/sqs/test_sqs.py::TestSqsProvider::test_marker_serialization_query_protocol": { "last_validated_date": "2024-04-29T06:07:04+00:00"