SNS: fix Subscribe idempotency due to RawMessageDelivery casing (#12420)

bentsku · web-flow · commit 69d4d34310d9 · 2025-03-25T11:15:10.000+01:00
diff --git a/localstack-core/localstack/services/sns/provider.py b/localstack-core/localstack/services/sns/provider.py
@@ -1,4 +1,5 @@
 import base64
+import copy
 import functools
 import json
 import logging
@@ -696,26 +697,29 @@ def subscribe(
                 "Invalid parameter: Invalid parameter: Endpoint Reason: FIFO SQS Queues can not be subscribed to standard SNS topics"
             )
 
-        if attributes:
-            for attr_name, attr_value in attributes.items():
+        sub_attributes = copy.deepcopy(attributes) if attributes else None
+        if sub_attributes:
+            for attr_name, attr_value in sub_attributes.items():
                 validate_subscription_attribute(
                     attribute_name=attr_name,
                     attribute_value=attr_value,
                     topic_arn=topic_arn,
                     endpoint=endpoint,
                     is_subscribe_call=True,
                 )
+                if raw_msg_delivery := sub_attributes.get("RawMessageDelivery"):
+                    sub_attributes["RawMessageDelivery"] = raw_msg_delivery.lower()
 
         # An endpoint may only be subscribed to a topic once. Subsequent
         # subscribe calls do nothing (subscribe is idempotent), except if its attributes are different.
         for existing_topic_subscription in store.topic_subscriptions.get(topic_arn, []):
             sub = store.subscriptions.get(existing_topic_subscription, {})
             if sub.get("Endpoint") == endpoint:
-                if attributes:
+                if sub_attributes:
                     # validate the subscription attributes aren't different
                     for attr in sns_constants.VALID_SUBSCRIPTION_ATTR_NAME:
                         # if a new attribute is present and different from an existent one, raise
-                        if (new_attr := attributes.get(attr)) and sub.get(attr) != new_attr:
+                        if (new_attr := sub_attributes.get(attr)) and sub.get(attr) != new_attr:
                             raise InvalidParameterException(
                                 "Invalid parameter: Attributes Reason: Subscription already exists with different attributes"
                             )
@@ -738,25 +742,22 @@ def subscribe(
             FilterPolicyScope="MessageAttributes",  # default value, will be overridden if set
             SubscriptionPrincipal=principal,  # dummy value, could be fetched with a call to STS?
         )
-        if attributes:
-            subscription.update(attributes)
-            if "FilterPolicy" in attributes:
+        if sub_attributes:
+            subscription.update(sub_attributes)
+            if "FilterPolicy" in sub_attributes:
                 filter_policy = (
-                    json.loads(attributes["FilterPolicy"]) if attributes["FilterPolicy"] else None
+                    json.loads(sub_attributes["FilterPolicy"])
+                    if sub_attributes["FilterPolicy"]
+                    else None
                 )
                 if filter_policy:
                     validator = FilterPolicyValidator(
-                        scope=attributes.get("FilterPolicyScope", "MessageAttributes"),
+                        scope=subscription.get("FilterPolicyScope", "MessageAttributes"),
                         is_subscribe_call=True,
                     )
                     validator.validate_filter_policy(filter_policy)
 
-                store.subscription_filter_policy[subscription_arn] = (
-                    json.loads(attributes["FilterPolicy"]) if attributes["FilterPolicy"] else None
-                )
-
-            if raw_msg_delivery := attributes.get("RawMessageDelivery"):
-                subscription["RawMessageDelivery"] = raw_msg_delivery.lower()
+                store.subscription_filter_policy[subscription_arn] = filter_policy
 
         store.subscriptions[subscription_arn] = subscription
 
diff --git a/tests/aws/services/sns/test_sns.py b/tests/aws/services/sns/test_sns.py
@@ -1045,7 +1045,7 @@ def subscribe_queue_to_topic(attributes: dict = None) -> dict:
 
         subscribe_resp = subscribe_queue_to_topic(
             {
-                "RawMessageDelivery": "true",
+                "RawMessageDelivery": "True",
             }
         )
         snapshot.match("subscribe", subscribe_resp)
diff --git a/tests/aws/services/sns/test_sns.snapshot.json b/tests/aws/services/sns/test_sns.snapshot.json
@@ -3721,7 +3721,7 @@
     }
   },
   "tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_subscribe_idempotency": {
-    "recorded-date": "15-09-2023, 17:29:11",
+    "recorded-date": "20-03-2025, 17:16:39",
     "recorded-content": {
       "subscribe": {
         "SubscriptionArn": "arn:<partition>:sns:<region>:111111111111:<resource:4>:<resource:1>",
diff --git a/tests/aws/services/sns/test_sns.validation.json b/tests/aws/services/sns/test_sns.validation.json
@@ -69,7 +69,7 @@
     "last_validated_date": "2023-08-24T21:27:58+00:00"
   },
   "tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_subscribe_idempotency": {
-    "last_validated_date": "2023-09-15T15:29:11+00:00"
+    "last_validated_date": "2025-03-20T17:16:39+00:00"
   },
   "tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_subscribe_with_invalid_protocol": {
     "last_validated_date": "2023-08-24T21:27:50+00:00"

Original file line number	Diff line number	Diff line change
`@@ -1045,7 +1045,7 @@ def subscribe_queue_to_topic(attributes: dict = None) -> dict:`
`1045`	`1045`
`1046`	`1046`	`subscribe_resp = subscribe_queue_to_topic(`
`1047`	`1047`	`{`
`1048`		`- "RawMessageDelivery": "true",`
	`1048`	`+ "RawMessageDelivery": "True",`
`1049`	`1049`	`}`
`1050`	`1050`	`)`
`1051`	`1051`	`snapshot.match("subscribe", subscribe_resp)`
Original file line number	Diff line number	Diff line change
`@@ -3721,7 +3721,7 @@`
`3721`	`3721`	`}`
`3722`	`3722`	`},`
`3723`	`3723`	`"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_subscribe_idempotency": {`
`3724`		`- "recorded-date": "15-09-2023, 17:29:11",`
	`3724`	`+ "recorded-date": "20-03-2025, 17:16:39",`
`3725`	`3725`	`"recorded-content": {`
`3726`	`3726`	`"subscribe": {`
`3727`	`3727`	`"SubscriptionArn": "arn:<partition>:sns:<region>:111111111111:<resource:4>:<resource:1>",`