-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Adding s3_list_objects_encoding_type_url handler to ListObjectsV2 #1552
Changes from all commits
f9eef86
3231b9a
8ef58cf
a0d2f3f
321aece
9d31414
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 |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"category": "s3", | ||
"type": "enhancement", | ||
"description": "Adds encoding and decoding handlers for ListObjectsV2 `#1552 <https://github.com/boto/botocore/issues/1552>`__" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -730,21 +730,38 @@ def decode_list_object(parsed, context, **kwargs): | |
# Amazon S3 includes this element in the response, and returns encoded key | ||
# name values in the following response elements: | ||
# Delimiter, Marker, Prefix, NextMarker, Key. | ||
_decode_list_object( | ||
top_level_keys=['Delimiter', 'Marker', 'NextMarker'], | ||
nested_keys=[('Contents', 'Key'), ('CommonPrefixes', 'Prefix')], | ||
parsed=parsed, | ||
context=context | ||
) | ||
|
||
def decode_list_object_v2(parsed, context, **kwargs): | ||
# From the documentation: If you specify encoding-type request parameter, | ||
# Amazon S3 includes this element in the response, and returns encoded key | ||
# name values in the following response elements: | ||
# Delimiter, Prefix, ContinuationToken, Key, and StartAfter. | ||
_decode_list_object( | ||
top_level_keys=['Delimiter', 'Prefix', 'ContinuationToken', 'StartAfter'], | ||
nested_keys=[('Contents', 'Key'), ('CommonPrefixes', 'Prefix')], | ||
parsed=parsed, | ||
context=context | ||
) | ||
|
||
def _decode_list_object(top_level_keys, nested_keys, parsed, context): | ||
if parsed.get('EncodingType') == 'url' and \ | ||
context.get('encoding_type_auto_set'): | ||
# URL decode top-level keys in the response if present. | ||
top_level_keys = ['Delimiter', 'Marker', 'NextMarker'] | ||
for key in top_level_keys: | ||
if key in parsed: | ||
parsed[key] = unquote_str(parsed[key]) | ||
# URL decode nested keys from the response if present. | ||
nested_keys = [('Contents', 'Key'), ('CommonPrefixes', 'Prefix')] | ||
for (top_key, child_key) in nested_keys: | ||
if top_key in parsed: | ||
for member in parsed[top_key]: | ||
member[child_key] = unquote_str(member[child_key]) | ||
|
||
|
||
def convert_body_to_file_like_object(params, **kwargs): | ||
if 'Body' in params: | ||
if isinstance(params['Body'], six.string_types): | ||
|
@@ -880,6 +897,8 @@ def remove_subscribe_to_shard(class_attributes, **kwargs): | |
|
||
('before-parameter-build.s3.ListObjects', | ||
set_list_objects_encoding_type_url), | ||
('before-parameter-build.s3.ListObjectsV2', | ||
set_list_objects_encoding_type_url), | ||
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. It would be great if we can add an integration test for this. Honestly, I would recommend adding one similar to the one that added the customization: https://github.com/boto/botocore/pull/726/files#diff-02f637532aa2fba41fa91ac0763345b4 |
||
('before-call.s3.PutBucketTagging', calculate_md5), | ||
('before-call.s3.PutBucketLifecycle', calculate_md5), | ||
('before-call.s3.PutBucketLifecycleConfiguration', calculate_md5), | ||
|
@@ -943,6 +962,7 @@ def remove_subscribe_to_shard(class_attributes, **kwargs): | |
('before-parameter-build.route53', fix_route53_ids), | ||
('before-parameter-build.glacier', inject_account_id), | ||
('after-call.s3.ListObjects', decode_list_object), | ||
('after-call.s3.ListObjectsV2', decode_list_object_v2), | ||
|
||
# Cloudsearchdomain search operation will be sent by HTTP POST | ||
('request-created.cloudsearchdomain.Search', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -873,6 +873,69 @@ def test_decode_list_objects_with_delimiter(self): | |
handlers.decode_list_object(parsed, context=context) | ||
self.assertEqual(parsed['Delimiter'], u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_decode_list_objects_v2(self): | ||
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. I like the unit tests that we added. It would be also great if we can add an integration test just like this one but using 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. Added an integration test. |
||
parsed = { | ||
'Contents': [{'Key': "%C3%A7%C3%B6s%25asd%08"}], | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['Contents'][0]['Key'], u'\xe7\xf6s%asd\x08') | ||
|
||
def test_decode_list_objects_v2_does_not_decode_without_context(self): | ||
parsed = { | ||
'Contents': [{'Key': "%C3%A7%C3%B6s%25asd"}], | ||
'EncodingType': 'url', | ||
} | ||
handlers.decode_list_object_v2(parsed, context={}) | ||
self.assertEqual(parsed['Contents'][0]['Key'], u'%C3%A7%C3%B6s%25asd') | ||
|
||
def test_decode_list_objects_v2_with_delimiter(self): | ||
parsed = { | ||
'Delimiter': "%C3%A7%C3%B6s%25%20asd%08+c", | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['Delimiter'], u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_decode_list_objects_v2_with_prefix(self): | ||
parsed = { | ||
'Prefix': "%C3%A7%C3%B6s%25%20asd%08+c", | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['Prefix'], u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_decode_list_objects_v2_with_continuationtoken(self): | ||
parsed = { | ||
'ContinuationToken': "%C3%A7%C3%B6s%25%20asd%08+c", | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['ContinuationToken'], u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_decode_list_objects_v2_with_startafter(self): | ||
parsed = { | ||
'StartAfter': "%C3%A7%C3%B6s%25%20asd%08+c", | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['StartAfter'], u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_decode_list_objects_v2_with_common_prefixes(self): | ||
parsed = { | ||
'CommonPrefixes': [{'Prefix': "%C3%A7%C3%B6s%25%20asd%08+c"}], | ||
'EncodingType': 'url', | ||
} | ||
context = {'encoding_type_auto_set': True} | ||
handlers.decode_list_object_v2(parsed, context=context) | ||
self.assertEqual(parsed['CommonPrefixes'][0]['Prefix'], | ||
u'\xe7\xf6s% asd\x08 c') | ||
|
||
def test_get_bucket_location_optional(self): | ||
# This handler should no-op if another hook (i.e. stubber) has already | ||
# filled in response | ||
|
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.
You also need to register the decoding after the response is received (line 945).
One handler injects the encoding type (
set_list_objects_encoding_type_url
), and the other handler (decode_list_object
) decodes objects if we injected the encoding type param. We need both.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.
Ah got it.