php-enqueue / enqueue-dev

Message Queue, Job Queue, Broadcasting, WebSockets packages for PHP, Symfony, Laravel, Magento. DEVELOPMENT REPOSITORY - provided by Forma-Pro
https://enqueue.forma-pro.com/
MIT License
2.17k stars 429 forks source link

Unescaped SNS MessageAttributes when message headers are used #1307

Open martinssipenko opened 1 year ago

martinssipenko commented 1 year ago

Hello,

We have recently stumbled upon an issue. We use AWS SNS/SQS setup to send messages to SNS topic and to consume them from SQS queues. We use SnsQs transport to do it. We also use message headers to add extra information to the messages we send through SNS/SQS. Additionally, we have recently started to look into using SNS subscription filters so that our services would be able to subscribe to events that they are interested in. To do so we are adding a custom message attribute to our messages using SnsQsMessage::setMessageAttributes() method.

However, once we enabled a Subscription filter policy on our SNS topic subscription we noticed that evens sent to the SNS topic are not getting through to the SQS queue, even tho the filtering was set up correctly. We also noticed that the value of NumberOfNotificationsFilteredOut-InvalidAttributes CloudWatch metric is non-zero, which lead us to believe that something is wrong with message attributes. While conducting a deeper investigation we found out that AWS SNS requires message attributes values to be escaped in case Subscription filter policies are being used. We also found similar reports of the issue on AWS forum.

We believe the problem is that the value of json_encode is not being escaped using addslashes() function. We modified the enqueue library code locally and found that adding addslashes() works. We also noticed that the addition of stripslashes() would be required in SnsQsConsumer class to remove the previously added slashes.

However, we believe such a change would be breaking backward compatibility as the messages that have been previously sent without addslashes() would become unconsumable if stripslashes() is introduced.

Another potential solution could be to bease64 encode the result of json_encode() and on message retrieval to base64 decode it only if it does not start with [ character (it was not base64 encoded). This would at least allow previously sent messages to be consumed without issues, however, the downside of such approach is that the raw message becomes less readable.

We would like to make a contribution to fixing this issue but currently are stuck due to the BC break issue. Perhaps maintainers or other contributors to this project have ideas on how to proceed further?

Below is the code we used to replicate the issue:

Send message:

<?php

require_once __DIR__ . '/vendor/autoload.php';

use Enqueue\SnsQs\SnsQsConnectionFactory;

$factory = new SnsQsConnectionFactory([
    'key'    => 'AAAAAAAAAAAAAAAAAAAA',
    'secret' => 'kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk',
    'region' => 'us-east-2',
]);

$context = $factory->createContext();

$inTopic = $context->createTopic('in');
$context->setTopicArn($inTopic, 'arn:aws:sns:us-east-2:000000000000:test-topic');

$message = $context->createMessage('Hello world 123!');
$message->setHeader('My-Custom-Header', 'my "value');

$message->setMessageAttributes([
    'EventName' => [
        'DataType'    => 'String',
        'StringValue' => 'events/test/dummy',
    ]
]);

$context->createProducer()->send($inTopic, $message);

Get message:

<?php

require_once __DIR__ . '/vendor/autoload.php';

use Enqueue\SnsQs\SnsQsConnectionFactory;

$factory = new SnsQsConnectionFactory([
    'key'    => 'AAAAAAAAAAAAAAAAAAAA',
    'secret' => 'kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk',
    'region' => 'us-east-2',
]);

$context = $factory->createContext();

$out1Queue = $context->createQueue('example-queue');
$consumer = $context->createConsumer($out1Queue);

$message = $consumer->receiveNoWait();

var_dump($message);

if (null !== $message) {
    $consumer->acknowledge($message);
}

Our example-queue SQS queue is subscribed to test-topic SNS topic and the following Subscription filter policy is used:

{
  "EventName": [
    "events/test/dummy"
  ]
}
martinssipenko commented 1 year ago

@makasim could you please take a look at this?