Skip to content

[EventsBridge] Add validation for malformed Detail value #11990

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
merged 3 commits into from
Dec 6, 2024
Merged

[EventsBridge] Add validation for malformed Detail value #11990

merged 3 commits into from
Dec 6, 2024

Conversation

gregfurman
Copy link
Contributor

@gregfurman gregfurman commented Dec 5, 2024 •
edited
Loading

Motivation

This PR validates an event entry's Detail field to ensure it is a valid JSON object.

From the PutEventsRequestEntry AWS docs:

Detail: A valid JSON object. There is no other schema imposed. The JSON object may contain fields and nested sub-objects.

In addition to the PutEventsResultEntry AWS docs for the error code:

MalformedDetail: The JSON provided is not valid.

Changes

  • Add validation to validate_event that returns MalformedDetail error type response.
  • Add negative test to validate malformed payloads return an error response.

@gregfurman gregfurman changed the title [EventsBridge] Add validation for malformed Detail entry field [EventsBridge] Add validation for malformed Detail value Dec 5, 2024
@gregfurman gregfurman force-pushed the fix/eventsbridge/malformed-detail-field branch from 2f8e2d0 to 1438fdb Compare December 5, 2024 14:44
@gregfurman gregfurman self-assigned this Dec 5, 2024
@gregfurman gregfurman added aws:events Amazon EventBridge semver: patch Non-breaking changes which can be included in patch releases labels Dec 5, 2024
@gregfurman gregfurman added this to the Playground milestone Dec 5, 2024
@gregfurman gregfurman mentioned this pull request Dec 5, 2024
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 8m 22s ⏱️ - 41m 17s
2 287 tests  - 1 543  2 185 ✅  - 1 327  102 💤  - 216  0 ❌ ±0 
2 289 runs   - 1 543  2 185 ✅  - 1 327  104 💤  - 216  0 ❌ ±0 

Results for commit c49d25f. ± Comparison against base commit 6df4dbc.

This pull request removes 1547 and adds 4 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.events.test_events.TestEvents ‑ test_put_event_malformed_detail[ARRAY]
tests.aws.services.events.test_events.TestEvents ‑ test_put_event_malformed_detail[MALFORMED_JSON]
tests.aws.services.events.test_events.TestEvents ‑ test_put_event_malformed_detail[SERIALIZED_STRING]
tests.aws.services.events.test_events.TestEvents ‑ test_put_event_malformed_detail[STRING]

@gregfurman gregfurman marked this pull request as ready for review December 5, 2024 15:38
maxhoheiser
maxhoheiser approved these changes Dec 6, 2024
View reviewed changes
Copy link
Member

@maxhoheiser maxhoheiser left a comment

Choose a reason for hiding this comment

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

Nice catch :)

tests/aws/services/events/test_events.py
)
@pytest.mark.parametrize(
"detail",
["NotJSON", "[]", "{{}", json.dumps("NotJSON")],
Copy link
Member

Choose a reason for hiding this comment

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

nice also testing for plain string - was a bit surprised that this also results in malformed error but it is correct :)

@gregfurman gregfurman merged commit 52c952d into master Dec 6, 2024
33 checks passed
@gregfurman gregfurman deleted the fix/eventsbridge/malformed-detail-field branch December 6, 2024 11:30
@gregfurman gregfurman mentioned this pull request Dec 10, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxhoheiser maxhoheiser maxhoheiser approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev

Assignees

@gregfurman gregfurman

Labels
aws:events Amazon EventBridge semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
Playground
Development

Successfully merging this pull request may close these issues.
2 participants
@gregfurman @maxhoheiser