Skip to content

feat(s3): support Filter rules in PutBucketNotificationConfiguration#178

Merged
hectorvent merged 3 commits intohectorvent:mainfrom
fguery:fix/s3-notification-filter-crash
Apr 3, 2026
Merged

feat(s3): support Filter rules in PutBucketNotificationConfiguration#178
hectorvent merged 3 commits intohectorvent:mainfrom
fguery:fix/s3-notification-filter-crash

Conversation

@fguery
Copy link
Copy Markdown
Contributor

@fguery fguery commented Apr 2, 2026

Summary

PutBucketNotificationConfiguration silently drops queue/topic configurations when the
XML body contains <Filter> elements (used for key prefix/suffix filtering). The generic
XmlParser.getElementText() throws on container elements like <Filter><S3Key><FilterRule>...,
and the blanket catch (XMLStreamException ignored) swallows the error, aborting the parse
mid-stream. The notification config is stored as empty.

This PR adds a dedicated NotificationConfigurationParser that handles the full notification
schema in a single StAX pass — parsing, storing, evaluating, and round-tripping filter rules. I wasn't
able to use it to parse nested elements, and I didn't want to do too many changes there.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

Verified with AWS CLI v2.26.1 against a local Floci dev instance.

Before: get-bucket-notification-configuration returns empty QueueConfigurations: []
when the PUT body contains <Filter> elements.

After:

  • Filter rules are preserved in the round-trip (PUT → GET returns FilterRules intact)
  • Prefix and suffix filters are evaluated when dispatching notifications to SQS/SNS
  • Tested: uploading incoming/data.csv with filter prefix=incoming/ + suffix=.csv
    delivers exactly 1 SQS message; outgoing/data.csv and incoming/data.txt are correctly
    filtered out

Wire format follows:
https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketNotificationConfiguration.html

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@hectorvent hectorvent added the s3 label Apr 2, 2026
@hectorvent
Copy link
Copy Markdown
Owner

Hi @fguery,

Thanks for taking the time to contribute! We've seen your PR and will take a look soon.

To ensure this implementation stays robust, could you also open a PR in our compatibility suite at https://github.com/hectorvent/floci-compatibility-tests? This helps us validate that your changes keep working perfectly as the project evolves.

The Floci team

@fguery
Copy link
Copy Markdown
Contributor Author

fguery commented Apr 2, 2026

I think mtpettyp way of handling the XML parser is a bit cleaner; I can do that too -- up to you :)
The compatibility one should work either way though.

@hectorvent
Copy link
Copy Markdown
Owner

I think mtpettyp way of handling the XML parser is a bit cleaner; I can do that too -- up to you :) The compatibility one should work either way though.

I think mtpettyp way of handling the XML parser is a bit cleaner; I can do that too -- up to you :) The compatibility one should work either way though.

Yeah, sounds good. Could bring that idea to this PR? Thanks

@hectorvent
Copy link
Copy Markdown
Owner

Thanks @fguery

  PASS  S3 GetBucketNotificationConfiguration (queue filter round-trip)
  PASS  S3 GetBucketNotificationConfiguration (topic filter round-trip)

@hectorvent hectorvent merged commit ef06fc3 into hectorvent:main Apr 3, 2026
6 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants