Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unescaped SNS MessageAttributes when message headers are used #1307

Open
martinssipenko opened this issue Jul 12, 2023 · 1 comment · May be fixed by #1308
Open

Unescaped SNS MessageAttributes when message headers are used #1307

martinssipenko opened this issue Jul 12, 2023 · 1 comment · May be fixed by #1308

Comments

@martinssipenko
Copy link

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 martinssipenko linked a pull request Jul 12, 2023 that will close this issue
@martinssipenko
Copy link
Author

@makasim could you please take a look at this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant