Repository: softwaremill/elasticmq (scala)

Issue:
The behavior on sending message-attributes with empty value is different from that of SQS
When I send a message to sqs with empty value, sqs returns error (InvalidParameterValue).
```
cat attributes.json
{
  "piyo": {
    "DataType": "String",
    "StringValue": ""
  }
}
aws sqs send-message \
    --queue-url https://sqs.ap-northeast-1.amazonaws.com/XXXXXXXXXX/elastic-mq-debug \
    --message-body bar \
    --message-attributes file://attributes.json
An error occurred (InvalidParameterValue) when calling the SendMessage operation: Message (user) attribute 'piyo' must contain a non-empty value of type 'String'.
```

But elastmq does not return error.

```
 java -jar elasticmq-server-0.15.7.jar
...

aws --endpoint-url http://localhost:9324 sqs create-queue --queue-name foo
{
    "QueueUrl": "http://localhost:9324/queue/foo"
}
cat attributes.json
{
  "piyo": {
    "DataType": "String",
    "StringValue": ""
  }
}
aws --endpoint-url http://localhost:9324 \
    sqs send-message \
    --queue-url http://localhost:9324/queue/foo \
    --message-body bar \
    --message-attributes file://attributes.json
{
    "MD5OfMessageBody": "37b51d194a7513e45b56f6524f2d51f2",
    "MD5OfMessageAttributes": "4166cc16b73099be3abe8e2b0f544c99",
    "MessageId": "62aa4db3-9666-4421-a1d8-b988f76b7ae1"
}
echo $?
0
```

Pull request description:
Fixes #373 

According to doc, message attribute value/data type can't be an empty String. Same applies to message body

Patch:
diff --git a/core/src/main/scala/org/elasticmq/Limits.scala b/core/src/main/scala/org/elasticmq/Limits.scala
index d6bd95bcf..0af018760 100644
--- a/core/src/main/scala/org/elasticmq/Limits.scala
+++ b/core/src/main/scala/org/elasticmq/Limits.scala
@@ -7,6 +7,7 @@ trait Limits {
   def queueNameLengthLimit: Limit[Int]
   def batchSizeLimit: Limit[Int]
   def numberOfMessagesLimit: Limit[RangeLimit[Int]]
+  def nonEmptyAttributesLimit: Limit[Boolean]
   def bodyValidCharactersLimit: Limit[List[RangeLimit[Int]]]
   def numberAttributeValueLimit: Limit[RangeLimit[BigDecimal]]
   def messageWaitTimeLimit: Limit[RangeLimit[Long]]
@@ -17,6 +18,7 @@ case object StrictSQSLimits extends Limits {
   override val queueNameLengthLimit: Limit[Int] = LimitedValue(80)
   override val batchSizeLimit: Limit[Int] = LimitedValue(10)
   override val numberOfMessagesLimit: Limit[RangeLimit[Int]] = LimitedValue(RangeLimit(1, 10))
+  override val nonEmptyAttributesLimit: Limit[Boolean] = LimitedValue(true)
   override val bodyValidCharactersLimit: Limit[List[RangeLimit[Int]]] = LimitedValue(
     List(
       RangeLimit(0x9, 0x9),
@@ -37,6 +39,7 @@ case object RelaxedSQSLimits extends Limits {
   override val queueNameLengthLimit: Limit[Int] = NoLimit
   override val batchSizeLimit: Limit[Int] = NoLimit
   override val numberOfMessagesLimit: Limit[RangeLimit[Int]] = NoLimit
+  override val nonEmptyAttributesLimit: Limit[Boolean] = NoLimit
   override val bodyValidCharactersLimit: Limit[List[RangeLimit[Int]]] = NoLimit
   override val numberAttributeValueLimit: Limit[RangeLimit[BigDecimal]] = NoLimit
   override val messageWaitTimeLimit: Limit[RangeLimit[Long]] = NoLimit
@@ -68,25 +71,57 @@ object Limits {
       "ReadCountOutOfRange"
     )
 
-  def verifyMessageStringAttribute(stringAttribute: String, limits: Limits): Either[String, Unit] =
+  def verifyMessageStringAttribute(
+      attributeName: String,
+      attributeValue: String,
+      limits: Limits
+  ): Either[String, Unit] =
+    for {
+      _ <- validateWhenLimitAvailable(limits.nonEmptyAttributesLimit)(
+        shouldValidate => if (shouldValidate) attributeValue.nonEmpty else true,
+        s"Attribute '$attributeName' must contain a non-empty value of type 'String'"
+      )
+      _ <- validateCharactersAndLength(attributeValue, limits)
+    } yield ()
+
+  def verifyMessageBody(body: String, limits: Limits): Either[String, Unit] =
+    for {
+      _ <- validateWhenLimitAvailable(limits.nonEmptyAttributesLimit)(
+        shouldValidate => if (shouldValidate) body.nonEmpty else true,
+        "The request must contain the parameter MessageBody."
+      )
+      _ <- validateCharactersAndLength(body, limits)
+    } yield ()
+
+  private def validateCharactersAndLength(stringValue: String, limits: Limits): Either[String, Unit] =
     for {
       _ <- validateWhenLimitAvailable(limits.bodyValidCharactersLimit)(
         rangeLimits =>
-          stringAttribute
+          stringValue
             .codePoints()
             .iterator
             .asScala
             .forall(codePoint => rangeLimits.exists(range => range.isBetween(codePoint))),
         "InvalidMessageContents"
       )
-      _ <- verifyMessageLength(stringAttribute.length, limits)
+      _ <- verifyMessageLength(stringValue.length, limits)
     } yield ()
 
-  def verifyMessageNumberAttribute(stringNumberValue: String, limits: Limits): Either[String, Unit] = {
-    validateWhenLimitAvailable(limits.numberAttributeValueLimit)(
-      limit => Try(BigDecimal(stringNumberValue)).toOption.exists(value => limit.isBetween(value)),
-      s"Number attribute value $stringNumberValue should be in range (-10**128..10**126)"
-    )
+  def verifyMessageNumberAttribute(
+      stringNumberValue: String,
+      attributeName: String,
+      limits: Limits
+  ): Either[String, Unit] = {
+    for {
+      _ <- validateWhenLimitAvailable(limits.nonEmptyAttributesLimit)(
+        shouldValidate => if (shouldValidate) stringNumberValue.nonEmpty else true,
+        s"Attribute '$attributeName' must contain a non-empty value of type 'Number'"
+      )
+      _ <- validateWhenLimitAvailable(limits.numberAttributeValueLimit)(
+        limit => Try(BigDecimal(stringNumberValue)).toOption.exists(value => limit.isBetween(value)),
+        s"Number attribute value $stringNumberValue should be in range (-10**128..10**126)"
+      )
+    } yield ()
   }
 
   def verifyMessageWaitTime(messageWaitTime: Long, limits: Limits): Either[String, Unit] = {
diff --git a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageDirectives.scala b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageDirectives.scala
index c431b0766..60f2b259e 100644
--- a/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageDirectives.scala
+++ b/rest/rest-sqs/src/main/scala/org/elasticmq/rest/sqs/SendMessageDirectives.scala
@@ -84,18 +84,20 @@ trait SendMessageDirectives { this: ElasticMQDirectives with SQSLimitsModule =>
           val strValue =
             parameters("MessageAttribute." + i + ".Value.StringValue")
           Limits
-            .verifyMessageStringAttribute(strValue, sqsLimits)
+            .verifyMessageStringAttribute(name, strValue, sqsLimits)
             .fold(error => throw new SQSException(error), identity)
           StringMessageAttribute(strValue, customDataType)
         case "Number" =>
           val strValue =
             parameters("MessageAttribute." + i + ".Value.StringValue")
           Limits
-            .verifyMessageNumberAttribute(strValue, sqsLimits)
+            .verifyMessageNumberAttribute(strValue, name, sqsLimits)
             .fold(error => throw new SQSException(error), identity)
           NumberMessageAttribute(strValue, customDataType)
         case "Binary" =>
           BinaryMessageAttribute.fromBase64(parameters("MessageAttribute." + i + ".Value.BinaryValue"), customDataType)
+        case "" =>
+          throw new SQSException(s"Attribute '$name' must contain a non-empty attribute type")
         case _ =>
           throw new Exception("Currently only handles String, Number and Binary typed attributes")
       }
@@ -124,7 +126,7 @@ trait SendMessageDirectives { this: ElasticMQDirectives with SQSLimitsModule =>
     val messageAttributes = getMessageAttributes(parameters)
     val messageSystemAttributes = getMessageSystemAttributes(parameters)
 
-    Limits.verifyMessageStringAttribute(body, sqsLimits).fold(error => throw new SQSException(error), identity)
+    Limits.verifyMessageBody(body, sqsLimits).fold(error => throw new SQSException(error), identity)
 
     val messageGroupId = parameters.get(MessageGroupIdParameter) match {
       // MessageGroupId is only supported for FIFO queues


Test patch:
diff --git a/core/src/test/scala/org/elasticmq/LimitsTest.scala b/core/src/test/scala/org/elasticmq/LimitsTest.scala
index e25710af1..dd5b3e563 100644
--- a/core/src/test/scala/org/elasticmq/LimitsTest.scala
+++ b/core/src/test/scala/org/elasticmq/LimitsTest.scala
@@ -59,16 +59,18 @@ class LimitsTest extends AnyWordSpec with Matchers with EitherValues {
   "Validation of message string attribute in strict mode" should {
     "pass if string attribute contains only allowed characters" in {
       val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x10efff).map(_.toChar).mkString
-      Limits.verifyMessageStringAttribute(testString, StrictSQSLimits) shouldBe Right(())
+      Limits.verifyMessageStringAttribute("attribute1", testString, StrictSQSLimits) shouldBe Right(())
     }
 
-    "pass if the string is empty" in {
-      Limits.verifyMessageStringAttribute("", StrictSQSLimits) shouldBe Right(())
+    "fail if the string is empty" in {
+      Limits.verifyMessageStringAttribute("attribute1", "", StrictSQSLimits) shouldBe Left(
+        "Attribute 'attribute1' must contain a non-empty value of type 'String'"
+      )
     }
 
     "fail if string contains any not allowed character" in {
       val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x19, 0x10efff).map(_.toChar).mkString
-      val error = Limits.verifyMessageStringAttribute(testString, StrictSQSLimits).left.value
+      val error = Limits.verifyMessageStringAttribute("attribute1", testString, StrictSQSLimits).left.value
       error shouldBe "InvalidMessageContents"
     }
   }
@@ -76,47 +78,62 @@ class LimitsTest extends AnyWordSpec with Matchers with EitherValues {
   "Validation of message string attribute in relaxed mode" should {
     "pass if string attribute contains only allowed characters" in {
       val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x10efff).map(_.toChar).mkString
-      Limits.verifyMessageStringAttribute(testString, RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageStringAttribute("attribute1", testString, RelaxedSQSLimits) shouldBe Right(())
     }
 
     "pass if the string is empty" in {
-      Limits.verifyMessageStringAttribute("", RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageStringAttribute("attribute1", "", RelaxedSQSLimits) shouldBe Right(())
     }
 
     "pass if string contains any not allowed character" in {
       val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x19, 0x10efff).map(_.toChar).mkString
-      Limits.verifyMessageStringAttribute(testString, RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageStringAttribute("attribute1", testString, RelaxedSQSLimits) shouldBe Right(())
     }
   }
 
   "Validation of message number attribute in strict mode" should {
     "pass if the number is between the limits (-10^128 - 10^126)" in {
-      Limits.verifyMessageNumberAttribute(BigDecimal(10).pow(126).toString(), StrictSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute((-BigDecimal(10).pow(128)).toString(), StrictSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute(BigDecimal(0).toString(), StrictSQSLimits) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(
+        BigDecimal(10).pow(126).toString(),
+        "numAttribute",
+        StrictSQSLimits
+      ) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(
+        (-BigDecimal(10).pow(128)).toString(),
+        "numAttribute",
+        StrictSQSLimits
+      ) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(BigDecimal(0).toString(), "numAttribute", StrictSQSLimits) shouldBe Right(())
       Limits.verifyMessageNumberAttribute(
         BigDecimal(Random.nextDouble()).toString(),
+        "numAttribute",
         StrictSQSLimits
       ) shouldBe Right(
         ()
       )
     }
 
+    "fail if the number is an empty string" in {
+      val emptyStringNumber = ""
+      val error = Limits.verifyMessageNumberAttribute(emptyStringNumber, "numAttribute", StrictSQSLimits)
+      error shouldBe Left("Attribute 'numAttribute' must contain a non-empty value of type 'Number'")
+    }
+
     "fail if the number is bigger than the upper bound" in {
       val overUpperBound = BigDecimal(10, MathContext.UNLIMITED).pow(126) + BigDecimal(0.1)
-      val error = Limits.verifyMessageNumberAttribute(overUpperBound.toString, StrictSQSLimits)
+      val error = Limits.verifyMessageNumberAttribute(overUpperBound.toString, "numAttribute", StrictSQSLimits)
       error shouldBe Left(s"Number attribute value $overUpperBound should be in range (-10**128..10**126)")
     }
 
     "fail if the number is below the lower bound" in {
       val belowLowerBound = -BigDecimal(10, MathContext.UNLIMITED).pow(128) - BigDecimal(0.1)
       val error =
-        Limits.verifyMessageNumberAttribute(belowLowerBound.toString, StrictSQSLimits)
+        Limits.verifyMessageNumberAttribute(belowLowerBound.toString, "numAttribute", StrictSQSLimits)
       error shouldBe Left(s"Number attribute value $belowLowerBound should be in range (-10**128..10**126)")
     }
 
     "fail if the number can't be parsed" in {
-      val error = Limits.verifyMessageNumberAttribute("12312312a", StrictSQSLimits).left.value
+      val error = Limits.verifyMessageNumberAttribute("12312312a", "numAttribute", StrictSQSLimits).left.value
       error shouldBe s"Number attribute value 12312312a should be in range (-10**128..10**126)"
     }
   }
@@ -125,11 +142,19 @@ class LimitsTest extends AnyWordSpec with Matchers with EitherValues {
     "always pass the validation" in {
       val belowLowerBound = -BigDecimal(10).pow(128) - BigDecimal(0.1)
       val overUpperBound = BigDecimal(10).pow(126) + BigDecimal(0.1)
-      Limits.verifyMessageNumberAttribute(belowLowerBound.toString, RelaxedSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute(BigDecimal(10).pow(126).toString(), RelaxedSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute((-BigDecimal(10).pow(128)).toString(), RelaxedSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute(overUpperBound.toString, RelaxedSQSLimits) shouldBe Right(())
-      Limits.verifyMessageNumberAttribute("12312312a", RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(belowLowerBound.toString, "numAttribute", RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(
+        BigDecimal(10).pow(126).toString(),
+        "numAttribute",
+        RelaxedSQSLimits
+      ) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(
+        (-BigDecimal(10).pow(128)).toString(),
+        "numAttribute",
+        RelaxedSQSLimits
+      ) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute(overUpperBound.toString, "numAttribute", RelaxedSQSLimits) shouldBe Right(())
+      Limits.verifyMessageNumberAttribute("12312312a", "numAttribute", RelaxedSQSLimits) shouldBe Right(())
     }
   }
 
@@ -232,4 +257,39 @@ class LimitsTest extends AnyWordSpec with Matchers with EitherValues {
       error shouldBe "InvalidParameterValue"
     }
   }
+
+  "Validation of message body in strict mode" should {
+    "pass if string attribute contains only allowed characters" in {
+      val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x10efff).map(_.toChar).mkString
+      Limits.verifyMessageBody(testString, StrictSQSLimits) shouldBe Right(())
+    }
+
+    "fail if the string is empty" in {
+      Limits.verifyMessageBody("", StrictSQSLimits) shouldBe Left(
+        "The request must contain the parameter MessageBody."
+      )
+    }
+
+    "fail if string contains any not allowed character" in {
+      val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x19, 0x10efff).map(_.toChar).mkString
+      val error = Limits.verifyMessageBody(testString, StrictSQSLimits).left.value
+      error shouldBe "InvalidMessageContents"
+    }
+  }
+
+  "Validation of message body in relaxed mode" should {
+    "pass if string attribute contains only allowed characters" in {
+      val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x10efff).map(_.toChar).mkString
+      Limits.verifyMessageBody(testString, RelaxedSQSLimits) shouldBe Right(())
+    }
+
+    "pass if the string is empty" in {
+      Limits.verifyMessageBody("", RelaxedSQSLimits) shouldBe Right(())
+    }
+
+    "pass if string contains any not allowed character" in {
+      val testString = List(0x9, 0xa, 0xd, 0x21, 0xe005, 0x19, 0x10efff).map(_.toChar).mkString
+      Limits.verifyMessageBody(testString, RelaxedSQSLimits) shouldBe Right(())
+    }
+  }
 }
diff --git a/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonJavaSdkTestSuite.scala b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonJavaSdkTestSuite.scala
index eb28bbf2f..ce36790e0 100644
--- a/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonJavaSdkTestSuite.scala
+++ b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/AmazonJavaSdkTestSuite.scala
@@ -1390,6 +1390,14 @@ class AmazonJavaSdkTestSuite extends SqsClientServerCommunication with Matchers
     result.isLeft should be(true)
   }
 
+  test("should return an error if strict & message body is empty") {
+    strictOnlyShouldThrowException { cli =>
+      val queueUrl = cli.createQueue(new CreateQueueRequest("testQueue1")).getQueueUrl
+
+      cli.sendMessage(new SendMessageRequest(queueUrl, ""))
+    }
+  }
+
   test("should return an error if strict & sending an invalid character") {
     strictOnlyShouldThrowException { cli =>
       // Given
diff --git a/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/MessageAttributesTests.scala b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/MessageAttributesTests.scala
new file mode 100644
index 000000000..0ad913517
--- /dev/null
+++ b/rest/rest-sqs-testing-amazon-java-sdk/src/test/scala/org/elasticmq/rest/sqs/MessageAttributesTests.scala
@@ -0,0 +1,81 @@
+package org.elasticmq.rest.sqs
+
+import com.amazonaws.services.sqs.model._
+import org.scalatest.OptionValues
+import org.scalatest.matchers.should.Matchers
+
+import scala.collection.JavaConverters.{collectionAsScalaIterableConverter, mapAsJavaMapConverter}
+
+class MessageAttributesTests extends SqsClientServerCommunication with Matchers with OptionValues {
+
+  test("Sending message with empty attribute value and String data type should result in error") {
+    val queueUrl = client.createQueue(new CreateQueueRequest("testQueue1")).getQueueUrl
+
+    val ex = the[AmazonSQSException] thrownBy client.sendMessage(
+      new SendMessageRequest(queueUrl, "Message 1")
+        .addMessageAttributesEntry("attribute1", new MessageAttributeValue().withStringValue("").withDataType("String"))
+    )
+
+    ex.getMessage should include("Attribute 'attribute1' must contain a non-empty value of type 'String")
+  }
+
+  test("Sending message with empty attribute value and Number data type should result in error") {
+    val queueUrl = client.createQueue(new CreateQueueRequest("testQueue1")).getQueueUrl
+
+    val ex = the[AmazonSQSException] thrownBy client.sendMessage(
+      new SendMessageRequest(queueUrl, "Message 1")
+        .addMessageAttributesEntry("attribute1", new MessageAttributeValue().withStringValue("").withDataType("Number"))
+    )
+
+    ex.getMessage should include("Attribute 'attribute1' must contain a non-empty value of type 'Number")
+  }
+
+  test("Sending message with empty attribute type should result in error") {
+    val queueUrl = client.createQueue(new CreateQueueRequest("testQueue1")).getQueueUrl
+
+    val ex = the[AmazonSQSException] thrownBy client.sendMessage(
+      new SendMessageRequest(queueUrl, "Message 1")
+        .addMessageAttributesEntry("attribute1", new MessageAttributeValue().withStringValue("value").withDataType(""))
+    )
+
+    ex.getMessage should include("Attribute 'attribute1' must contain a non-empty attribute type")
+  }
+
+  test(
+    "Sending message in batch should result in accepting only those messages that do not have empty message attributes"
+  ) {
+    val queueUrl = client.createQueue(new CreateQueueRequest("testQueue1")).getQueueUrl
+
+    val resp = client.sendMessageBatch(
+      new SendMessageBatchRequest(queueUrl).withEntries(
+        new SendMessageBatchRequestEntry("1", "Message 1").withMessageAttributes(
+          Map(
+            "attribute1" -> new MessageAttributeValue().withStringValue("value1").withDataType("String")
+          ).asJava
+        ),
+        new SendMessageBatchRequestEntry("2", "Message 2").withMessageAttributes(
+          Map(
+            "attribute1" -> new MessageAttributeValue().withStringValue("").withDataType("String")
+          ).asJava
+        ),
+        new SendMessageBatchRequestEntry("3", "Message 3").withMessageAttributes(
+          Map(
+            "attribute1" -> new MessageAttributeValue().withStringValue("value1").withDataType("String")
+          ).asJava
+        )
+      )
+    )
+
+    resp.getSuccessful should have size 2
+
+    val receiveMessageResponse =
+      client.receiveMessage(new ReceiveMessageRequest().withQueueUrl(queueUrl).withMaxNumberOfMessages(3))
+    receiveMessageResponse.getMessages.asScala should have size 2
+    receiveMessageResponse.getMessages.asScala
+      .map(message => message.getBody)
+      .foreach(messageBody => {
+        messageBody should (be("Message 1") or be("Message 3"))
+      })
+  }
+
+}