pplu / aws-sdk-perl

A community AWS SDK for Perl Programmers
Other
171 stars 94 forks source link

SNS MessageAttributes do not serialize correctly #267

Open nebulous opened 6 years ago

nebulous commented 6 years ago

In the following test case:

$sns->Publish(
    TopicArn => ...,
    Subject => 'a message with attributes',
    Message => 'some message text',
    MessageAttributes => {
        somename=>{ DataType=>'String', StringValue=>'somevalue' }
    }
);

Net::QueryCaller serializes Paws::SNS::MessageAttributeMap which consumes the Paws::API::StrToObjMapParserrole to:

MessageAttributes.1.Name                "somename",
MessageAttributes.1.Value.DataType      "String",
MessageAttributes.1.Value.StringValue   "some value",

Which results in an error being returned from SNS:

Top level element may not be treated as a list

The aws documentation provides conflicting (or inconsistent between SNS and SQS) accounts of how MessageAttributes should be sent in the body as do different client libraries (boto/ex-aws/etc) so I'm not particularly certain what is correct to send, but in testing the following does successfully post to SNS in the above testcase:

MessageAttributes.entry.1.name                "somename",
MessageAttributes.entry.1.value.DataType      "String",
MessageAttributes.entry.1.value.StringValue   "some value",

This issue appears quite similar to (https://github.com/ex-aws/ex_aws/pull/328)

So, am I simply doingItWrong™? I'd love to be corrected!

nebulous commented 6 years ago

Here's a patch which solves the issue for me.

diff --git a/lib/Paws/Net/QueryCaller.pm b/lib/Paws/Net/QueryCaller.pm
index d88c8df9c..25c5917c2 100644
--- a/lib/Paws/Net/QueryCaller.pm
+++ b/lib/Paws/Net/QueryCaller.pm
@@ -50,9 +50,10 @@ package Paws::Net::QueryCaller;
         } elsif ($params->$att->does('Paws::API::StrToObjMapParser')) {
           my $i = 1;
           foreach my $map_key (keys %{ $params->$att->Map }){
-            $p{ "$key.$i.Name" } = $map_key;
+            my $ent = ref($params->$att) eq 'Paws::SNS::MessageAttributeMap' ? '.entry' : '';
+            $p{ "$key$ent.$i.name" } = $map_key;
             my %complex_value = $self->_to_querycaller_params($params->$att->Map->{ $map_key });
-            map { $p{ "$key.$i.Value.$_" } = $complex_value{$_} } keys %complex_value;
+            map { $p{ "$key$ent.$i.value.$_" } = $complex_value{$_} } keys %complex_value;
             $i++;
           }
         } elsif ($params->$att->does('Paws::API::StrToNativeMapParser')) {
pplu commented 6 years ago

Hi,

Thanks for your report and patch to understand whats wrong.

JB62 commented 6 years ago

$p{ "$key$ent.$i.name" } needs to be $p{ "$key$ent.$i.Name" } and { $p{ "$key$ent.$i.value.$_" } should be { $p{ "$key$ent.$i.Value.$_" } or it breaks SQS message attributes. For me at least, SQS cares about the case of Name and Value, where SNS doesn't seem bothered.

JB62 commented 6 years ago

There appears to be a similar issue with SQS::AddPermission - the Actions array is being translated to Actions.N rather than ActionName.N

$sqs->AddPermission(Actions => ['*'], Label => "MyLabel", AWSAccountIds => ["123456789012"], QueueUrl => "MyQueue");

Should result in
https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue/?Action=AddPermission&Label=MyLabel&AWSAccountId.1=125074342641&ActionName.1=*

but you actually get

https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue/?Action=AddPermission &Label=MyLabel&AWSAccountId.1=125074342641&Action.1=*

This seems to fix it...

--- a/QueryCaller.pm       2018-08-13 13:48:29.121122279 +0100
+++ b/QueryCaller.pm        2018-09-24 13:14:04.743545610 +0100
@@ -36,7 +36,8 @@
           if ($self->_is_internal_type("$1")){
             my $i = 1;
             foreach my $value (@{ $params->$att }){
-              $p{ sprintf($self->array_flatten_string, $key, $i) } = $value;
-              $i++
+              $p{ sprintf($self->array_flatten_string,
+                  ref $params eq 'Paws::SQS::AddPermission' && $key eq 'Actions' ? 'ActionName' : $key, $i) } = $value;
+              $i++;
             }
           } else {
sapphirecat commented 6 years ago

I've run into this problem converting from a (patched copy of) Amazon::SNS to Paws.

The code I added to the former to support message attributes is the following (where $attr is an optional parameter I added to the Publish call):

my $attributes = undef;
if (defined($attr) and ref($attr) eq 'HASH') {
  my $i = 1;

  foreach my $key (keys %$attr) {
    $attributes->{"MessageAttributes.entry.$i.Name"} = $key;
    $attributes->{"MessageAttributes.entry.$i.Value.DataType"} = $attr->{$key}->{'Type'};

    if($attr->{$key}->{'Type'} eq 'Binary') {
      $attributes->{"MessageAttributes.entry.$i.Value.BinaryValue"} = $attr->{$key}->{'Value'};
    } else {
      $attributes->{"MessageAttributes.entry.$i.Value.StringValue"} = $attr->{$key}->{'Value'};
    }

    $i++;
  }
}

However, it's not clear to me how to adjust/monkeypatch the serialization of the message attributes in Paws.

lungching commented 6 years ago

I'm coming across this issue, too. It would be super helpful see this patched.

and-reas-se commented 5 years ago

Also ran into this problem, the first patch posted in the issue fixes it for me.

renatocron commented 4 years ago

I'm also having a issue with this, I was not able to find this before when searching for "Top level element may not be treated as a list" so I will repeat it again

Google read this: Paws Top level element may not be treated as a list error!

After using perl -d a little I solved changing the string generated at _to_querycaller_params when attribute role is Paws::API::StrToObjMapParser

unfortunately the classing too much for me! I don't know what's the correct way to path this also

use utf8;
#use LWP::ConsoleLogger::Everywhere ();
use Paws::Net::LWPCaller;

my $obj = Paws->service(
    'SNS', 
    region => 'us-east-1',
    caller => Paws::Net::LWPCaller->new()
);

my $res = $obj->Publish(
    Message           => 'Hello 好き atenção!',
    MessageAttributes => {
        'AWS.SNS.SMS.SMSType' => {
            DataType    => 'String',
            StringValue => 'Transactional',
        },
    },
    PhoneNumber => '+12345',

);

The error happened in other langs as well https://stackoverflow.com/questions/49311830/how-to-add-message-attributes-to-sns-publish-from-api-gateway-integration-reques

even the Amazon SDK on Ios had it once. It's a bug in the docs.