Skip to content

feat: add validation for SNS batch entry IDs and corresponding tests #13556

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.

Sign up for GitHub

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

Jump to bottom
Merged
bentsku merged 3 commits into from
Jan 2, 2026
Merged

feat: add validation for SNS batch entry IDs and corresponding tests #13556

bentsku merged 3 commits into from
Jan 2, 2026

Conversation

@shubhiscoding
Copy link
Contributor

@shubhiscoding shubhiscoding commented Dec 21, 2025

Motivation

Fixes #13526

SNS publish_batch was not validating batch entry IDs according to AWS specifications. AWS requires entry IDs to:

  • Only contain alphanumeric characters, hyphens, and underscores
  • Be at most 80 characters long

Currently, LocalStack accepts invalid IDs like "message.id" (contains dot) or IDs exceeding 80 characters, which AWS rejects with InvalidBatchEntryId errors.

Changes

  • Added BATCH_ENTRY_ID_REGEX constant to validate ID character pattern
  • Implemented validate_batch_entry_id() function with length and pattern validation
  • Integrated validation into publish_batch() method after duplicate ID check
  • Raises InvalidBatchEntryIdException with appropriate error messages matching AWS behavior

Tests

  • Added test_publish_batch_invalid_entry_id_simple (LocalStack-only test) - validates basic functionality
  • Added test_publish_batch_invalid_entry_id (AWS-validated snapshot test) - ensures AWS parity
  • Both tests cover:
    • Invalid characters (dot, special chars)
    • Length violations (81+ characters)
    • Valid IDs (80 chars max, hyphens, underscores)

Related

Remaining validation gaps identified during implementation (to be addressed in separate issues):

  • Message required/empty validation
  • Subject empty string validation
  • MessageDeduplicationId/MessageGroupId length + pattern for FIFO topics

bentsku
bentsku requested changes Dec 22, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your second contribution!

The change looks good in principle! But same issue as #13554, we require our tests to be validated against AWS, and it doesn't seem to be the case here. Could you please run them and push the changes?

Also, as a side note, we currently have a in progress re-implementation of SNS in the v2 module of the sns service, it might be good to port this validation logic there too. 👍

Let me know if you have any problem, and thanks again, this looks good!

edit: forgot to add to both PR, it is holidays season here and we might have some delay in reviewing your changes, and might come back to it in January.

@bentsku bentsku self-assigned this Dec 22, 2025
@shubhiscoding
Copy link
Contributor Author

shubhiscoding commented Dec 23, 2025 •
edited
Loading

forgot about v2 part in the earlier commit, I've ported the validation logic to the v2 provider in this last commit. The same test passes for both v1 and v2 (verified with PROVIDER_OVERRIDE_SNS=v2).

@bentsku bentsku added aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Jan 2, 2026
bentsku
bentsku approved these changes Jan 2, 2026
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for addressing the comments and sorry for the delay in reviewing due to the holidays!

Congratulations on the contribution, and welcome to the list of contributors!

@bentsku bentsku merged commit 9166eee into localstack:main Jan 2, 2026
50 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bentsku bentsku bentsku approved these changes

@baermat baermat Awaiting requested review from baermat baermat is a code owner

Assignees

@bentsku bentsku

Labels

aws:sns Amazon Simple Notification Service docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: SNS batch entry IDs are not correctly validated

2 participants

@shubhiscoding @bentsku