Fix get-object-part #401
Fix get-object-part #401
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #401 +/- ##
==========================================
+ Coverage 89.05% 89.09% +0.03%
==========================================
Files 21 21
Lines 6160 6170 +10
==========================================
+ Hits 5486 5497 +11
+ Misses 674 673 -1
|
source/s3_client.c
Outdated
| struct aws_byte_cursor part_number_query_str = aws_byte_cursor_from_c_str("partNumber"); | ||
| if (aws_byte_cursor_find_exact(&path_and_query, &part_number_query_str, &found_string) == |
There was a problem hiding this comment.
find isn't going to cut it. This would match: "rampartNumbering.s3.us-west-2.amazonaws.com"
I'm sure we have query param helpers somewhere in our codebase
There was a problem hiding this comment.
I tried to find one, but there is only a uri parsing with query param, which requires the whole uri, I can add something to the uri.h to support it
There was a problem hiding this comment.
| /* The first split on '?' for path and query is path, the second is query */ | ||
| if (aws_byte_cursor_next_split(&path_and_query, '?', &sub_string) == true) { | ||
| aws_byte_cursor_next_split(&path_and_query, '?', &sub_string); | ||
| struct aws_uri_param param; | ||
| AWS_ZERO_STRUCT(param); | ||
| struct aws_byte_cursor part_number_query_str = aws_byte_cursor_from_c_str("partNumber"); | ||
| while (aws_query_string_next_param(sub_string, ¶m)) { | ||
| if (aws_byte_cursor_eq(¶m.key, &part_number_query_str)) { | ||
| return aws_s3_meta_request_default_new( | ||
| client->allocator, | ||
| client, | ||
| AWS_S3_REQUEST_TYPE_GET_OBJECT, | ||
| content_length, | ||
| false /*should_compute_content_md5*/, | ||
| options); | ||
| } | ||
| } |
There was a problem hiding this comment.
Debatable: It would be better to invest in a better helper like aws_http_headers_has/ aws_http_headers_get. This logic will need to be repeated wherever we need to extract a single parameter from query params.
Or, we could still use find and try to locate &partNumber= or ?partNumber=.
There was a problem hiding this comment.
I have no strong feelings about it.
My take is we are used as a client level http lib, and this is the first time we ever really want to parse the query params, maybe let's hold it back until the next time we want to reuse it?
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.