Add URI S3 Client unit tests #2901
Merged
Conversation
This file contains 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
sbiscigl
commented
Mar 21, 2024
| @@ -40,6 +40,7 @@ def main(): | |||
| "aws-cpp-sdk-dynamodb-integration-tests", | |||
| "aws-cpp-sdk-sqs-integration-tests", | |||
| "aws-cpp-sdk-s3-integration-tests", | |||
| "aws-cpp-sdk-s3-unit-tests", | |||
There was a problem hiding this comment.
im adding to the run integ tests right now. likely this should be a completely seperate step in our CI, and we should have other clients in it. We can create a seperate sep once we have enough client unit tests to justify it.
sbiscigl
commented
Mar 21, 2024
| //We have to mock requset because it is used to create the return body, it actually isnt used. | ||
| auto mockRequest = Aws::MakeShared<Standard::StandardHttpRequest>(ALLOCATION_TAG, "mockuri", HttpMethod::HTTP_GET); | ||
| mockRequest->SetResponseStreamFactory([]() -> IOStream* { | ||
| IOStream* stream = Aws::New<StringStream>(ALLOCATION_TAG, "response-string", std::ios_base::in | std::ios_base::binary); |
There was a problem hiding this comment.
"MEMORY LEAK" -- you shout
it is deleted when mockResponse is deleted in turn calling the destructor on its bodyStream, which in turn calls delete on the underlying stream.
SergeyRyabinin
approved these changes
Mar 21, 2024
sbiscigl
force-pushed
the
s3-uri-unit-tests
branch
4 times, most recently
from
146937e
to
bbfdd27
Compare
sbiscigl
force-pushed
the
s3-uri-unit-tests
branch
from
bbfdd27
to
2c51217
Compare
SergeyRyabinin
approved these changes
Mar 22, 2024
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.
Description of changes:
Adds tests for a possible URI traversal issue that the PHP SDK saw an issue with. Besides adding tests this adds a new target
aws-cpp-sdk-s3-unit-teststhat will run tests with a S3 client with a mocked http client.Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.