SNS message parsing broken

[Legogris]

The Message parsing relies on the parameter SigningCertURL:

aws-sdk-net/sdk/src/Services/SimpleNotificationService/Custom/Util/Message.cs

Line 73 in e538f4d

message.SigningCertURL = ValidateCertUrl(extractField("SigningCertURL"));

However, actual SNS messages provide it as SigningCertUrl (notice the casing difference).

This makes the instantiation always fail with legitimate messages, and since ParseMessage is the only public way to set these fields the current implementation is practically unusable.

We fixed it by making our own copy of this file and replacing the JSON parsing with Newtonsoft.Json:

        public static Message ParseMessage(string messageText)
        {
            var message = new Message();

            var settings = new JsonSerializerSettings { DateParseHandling = DateParseHandling.None };
            JObject json = (JObject)JsonConvert.DeserializeObject(messageText, settings);

            Func<string, string> extractField = ((fieldName) =>
                {
                    var val = json.GetValue(fieldName, StringComparison.OrdinalIgnoreCase)?.Value<string>();
                    if (string.IsNullOrEmpty(val)) {
                        return null;
                    }
                    return val;
                });

I would make a PR, but I assume there is a reason why these libs aren't using Newtonsoft.Json (more common in the community) in the first place?

[klaytaybai]

Were you trying to use SNS with Lambda? I'm aware of a discrepancy that exists with Lambda that we probably would not be able to change due to backwards compatibility issues and consistency with the service and other SDKs.

[Legogris]

@klaytaybai What discrepancies are you talking about specifically? AFAIK LAambda/SNS is a pretty standard pattern to use in chains and pipelines.
If there is anything blocking this, I would suggest deprecating or even removing the class altogether as a broken implementation can be worse than none at all.

[klaytaybai]

This blog post about invoking Lambda functions with SNS subscriptions indicates that the format it uses is "SigningCertUrl". Everywhere else that I am aware of uses the standard "SigningCertURL." If you have a different use case where you are seeing this difference, that would be important to know.

I'd like to add a disclaimer: My claim about likely not being able to change the Lambda subscriptions is mostly my opinion based on similar conversations I have heard. I don't have much say in the decision regarding deprecations like that. Also, the context I was using was a change in the service, not in the SDK. That was a poor judgement on my part to not clearly define the context of my thoughts there.

I'll label this as a bug in the SDK because we are not accounting for both spellings. We should be able to patch it up pretty easily. Thanks for bringing it to our attention.

[Legogris]

If you have a different use case where you are seeing this difference, that would be important to know.

I have a Lambda that uses SNS messages as a trigger. Messages I was receiving on SNS uses SigningCertUrl. Maybe this has changed historically at some point, but right now the SDK doesn't seem to handle messages originating from SNS.

[sstevenkang]

There is a discrepancy between what SNS sends back to clients and what it sends to Lambda function:
https://docs.aws.amazon.com/sns/latest/dg/json-formats.html#http-notification-json
https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-sns

Are you by any chance using our Lambda libraries or rolling your own unarmshalling logic?