Fix NullPointerException when publishing an SNS batch without a topic ARN - #12457
Open
anjeongkyun wants to merge 1 commit into
Open
Fix NullPointerException when publishing an SNS batch without a topic ARN#12457anjeongkyun wants to merge 1 commit into
anjeongkyun wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SnsInterceptorreads the topic ARN to derive the topic name for DSM. The single-publish path already handles a missing ARN (a publish to a phone number has neithertopicArnnortargetArn), but the batch path dereferences it straight away:PublishBatchRequestbuilds fine without a topic ARN, and the interceptor runs before marshalling, so a request that AWS would have rejected with a validation error instead fails with an NPE thrown from the agent. Neither SDK wraps exceptions frommodifyRequest/beforeMarshalling, so it surfaces to the caller as-is.Without the change, the new test fails like this:
Both the v1 and v2 interceptors have it, so both are fixed the same way the single-publish path already does: skip the injection and leave the request alone.
I checked the other ARN parsing in the aws-java modules while I was here.
AwsSdkClientDecoratorguards it in v1 (if (null != topicArn)) and usesOptional.mapin v2, so the two batch branches were the only unguarded ones.Tests mirror the existing
SNS message to phone number doesn't leak exceptioncase.aws-java-sns-1.0andaws-java-sns-2.0test tasks pass,spotlessApplyis clean.Could a maintainer add
inst: awsandtype: bug? I can't set labels on this repo.