Add tests for message attribute validation in SNS (#1582)

* Add tests for message attribute validation in SNS

Fixes up bug in return value of
moto.sns.responses.SNSResponse._parse_message_attributes due to
accidental recycling of a variable.

* Fix test_sns.test_publish_to_http in py36 env

Http response is encoded as a byte string which json.loads can't handle.

moto/sns/responses.py

@@ -55,11 +55,12 @@ class SNSResponse(BaseResponse):
                     "attribute type, the set of supported type prefixes is "
                     "Binary, Number, and String.".format(name))
 
+            transform_value = None
             if 'StringValue' in value:
-                value = value['StringValue']
+                transform_value = value['StringValue']
-            elif 'BinaryValue' in 'Value':
+            elif 'BinaryValue' in value:
-                value = value['BinaryValue']
+                transform_value = value['BinaryValue']
-            if not value:
+            if not transform_value:
                 raise InvalidParameterValue(
                     "The message attribute '{0}' must contain non-empty "
                     "message attribute value for message attribute "
@@ -67,7 +68,7 @@ class SNSResponse(BaseResponse):
 
             # transformation
             transformed_message_attributes[name] = {
-                'Type': data_type, 'Value': value
+                'Type': data_type, 'Value': transform_value
             }
 
         return transformed_message_attributes
@@ -283,10 +284,7 @@ class SNSResponse(BaseResponse):
         phone_number = self._get_param('PhoneNumber')
         subject = self._get_param('Subject')
 
-        try:
         message_attributes = self._parse_message_attributes()
-        except InvalidParameterValue as e:
-            return self._error(e.description), dict(status=e.code)
 
         if phone_number is not None:
             # Check phone is correct syntax (e164)

tests/test_sns/test_publishing_boto3.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import base64
 import json
 
 import boto3
@@ -41,6 +42,83 @@ def test_publish_to_sqs():
     acquired_message.should.equal(expected)
 
 
+@mock_sqs
+@mock_sns
+def test_publish_to_sqs_bad():
+    conn = boto3.client('sns', region_name='us-east-1')
+    conn.create_topic(Name="some-topic")
+    response = conn.list_topics()
+    topic_arn = response["Topics"][0]['TopicArn']
+
+    sqs_conn = boto3.resource('sqs', region_name='us-east-1')
+    sqs_conn.create_queue(QueueName="test-queue")
+
+    conn.subscribe(TopicArn=topic_arn,
+                   Protocol="sqs",
+                   Endpoint="arn:aws:sqs:us-east-1:123456789012:test-queue")
+    message = 'my message'
+    try:
+        # Test missing Value
+        conn.publish(
+            TopicArn=topic_arn, Message=message,
+            MessageAttributes={'store': {'DataType': 'String'}})
+    except ClientError as err:
+        err.response['Error']['Code'].should.equal('InvalidParameterValue')
+    try:
+        # Test empty DataType (if the DataType field is missing entirely
+        # botocore throws an exception during validation)
+        conn.publish(
+            TopicArn=topic_arn, Message=message,
+            MessageAttributes={'store': {
+                'DataType': '',
+                'StringValue': 'example_corp'
+            }})
+    except ClientError as err:
+        err.response['Error']['Code'].should.equal('InvalidParameterValue')
+    try:
+        # Test empty Value
+        conn.publish(
+            TopicArn=topic_arn, Message=message,
+            MessageAttributes={'store': {
+                'DataType': 'String',
+                'StringValue': ''
+            }})
+    except ClientError as err:
+        err.response['Error']['Code'].should.equal('InvalidParameterValue')
+
+
+@mock_sqs
+@mock_sns
+def test_publish_to_sqs_msg_attr_byte_value():
+    conn = boto3.client('sns', region_name='us-east-1')
+    conn.create_topic(Name="some-topic")
+    response = conn.list_topics()
+    topic_arn = response["Topics"][0]['TopicArn']
+
+    sqs_conn = boto3.resource('sqs', region_name='us-east-1')
+    queue = sqs_conn.create_queue(QueueName="test-queue")
+
+    conn.subscribe(TopicArn=topic_arn,
+                   Protocol="sqs",
+                   Endpoint="arn:aws:sqs:us-east-1:123456789012:test-queue")
+    message = 'my message'
+    conn.publish(
+        TopicArn=topic_arn, Message=message,
+        MessageAttributes={'store': {
+            'DataType': 'Binary',
+            'BinaryValue': b'\x02\x03\x04'
+        }})
+    messages = queue.receive_messages(MaxNumberOfMessages=5)
+    message_attributes = [
+        json.loads(m.body)['MessageAttributes'] for m in messages]
+    message_attributes.should.equal([{
+        'store': {
+            'Type': 'Binary',
+            'Value': base64.b64encode(b'\x02\x03\x04').decode()
+        }
+    }])
+
+
 @mock_sns
 def test_publish_sms():
     client = boto3.client('sns', region_name='us-east-1')
@@ -153,7 +231,9 @@ def test_publish_to_sqs_in_different_region():
 def test_publish_to_http():
     def callback(request):
         request.headers["Content-Type"].should.equal("application/json")
-        json.loads.when.called_with(request.body).should_not.throw(Exception)
+        json.loads.when.called_with(
+            request.body.decode()
+        ).should_not.throw(Exception)
         return 200, {}, ""
 
     responses.add_callback(
@@ -263,6 +343,7 @@ def test_filtering_exact_string_multiple_message_attributes():
         'store': {'Type': 'String', 'Value': 'example_corp'},
         'event': {'Type': 'String', 'Value': 'order_cancelled'}}])
 
+
 @mock_sqs
 @mock_sns
 def test_filtering_exact_string_OR_matching():
@@ -287,6 +368,7 @@ def test_filtering_exact_string_OR_matching():
         {'store': {'Type': 'String', 'Value': 'example_corp'}},
         {'store': {'Type': 'String', 'Value': 'different_corp'}}])
 
+
 @mock_sqs
 @mock_sns
 def test_filtering_exact_string_AND_matching_positive():
@@ -311,6 +393,7 @@ def test_filtering_exact_string_AND_matching_positive():
         'store': {'Type': 'String', 'Value': 'example_corp'},
         'event': {'Type': 'String', 'Value': 'order_cancelled'}}])
 
+
 @mock_sqs
 @mock_sns
 def test_filtering_exact_string_AND_matching_no_match():
@@ -332,6 +415,7 @@ def test_filtering_exact_string_AND_matching_no_match():
         json.loads(m.body)['MessageAttributes'] for m in messages]
     message_attributes.should.equal([])
 
+
 @mock_sqs
 @mock_sns
 def test_filtering_exact_string_no_match():
@@ -350,6 +434,7 @@ def test_filtering_exact_string_no_match():
         json.loads(m.body)['MessageAttributes'] for m in messages]
     message_attributes.should.equal([])
 
+
 @mock_sqs
 @mock_sns
 def test_filtering_exact_string_no_attributes_no_match():