-
Notifications
You must be signed in to change notification settings - Fork 98
feat: Add Lambda-Runtime-Invocation-Id support for cross-wiring protection #221
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -333,6 +333,7 @@ TEST(InvocationRequestTest, default_fields_are_empty) | |
| EXPECT_TRUE(req.cognito_identity.empty()); | ||
| EXPECT_TRUE(req.function_arn.empty()); | ||
| EXPECT_TRUE(req.tenant_id.empty()); | ||
| EXPECT_TRUE(req.invocation_id.empty()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we expect it to be empty? or does it reflect the missing req.invocation_id being passed in run_handler?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test asserts the default-constructed state of invocation_request (all fields empty before get_next() populates them) |
||
| } | ||
|
|
||
| // --- version tests (no AWS SDK needed) --- | ||
|
|
@@ -353,3 +354,11 @@ TEST(VersionTest, version_format) | |
| } | ||
| EXPECT_EQ(2, dots); | ||
| } | ||
|
|
||
| // --- invocation_id cross-wiring protection tests --- | ||
|
|
||
| TEST(InvocationRequestTest, invocation_id_default_empty) | ||
| { | ||
| invocation_request req; | ||
| EXPECT_TRUE(req.invocation_id.empty()); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use default argument values. Instead, add an overload and make this one call it with a default value.
Parameters with default values make it hard to change this API without breaking backwards compatibility.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment goes for
post_successThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your input, you're right, let's do that