Skip to content
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

s3: Decode Prefix query param reflected back in response #1901

Closed
wants to merge 1 commit into from
Closed

s3: Decode Prefix query param reflected back in response #1901

wants to merge 1 commit into from

Conversation

tipabu
Copy link
Contributor

@tipabu tipabu commented Nov 25, 2019

Otherwise you get inconsistent decoding when working with non-ASCII keys:

>>> c.list_objects(Bucket='1space-test', Prefix='my\N{SNOWMAN}', Delimiter='\N{SNOWMAN}', Marker='a\N{SNOWMAN}')
{'Contents': ...,
 'Delimiter': '☃',
 'EncodingType': 'url',
 'IsTruncated': False,
 'Marker': 'a☃',
 'MaxKeys': 1000,
 'Name': '1space-test',
 'Prefix': 'my%E2%98%83',
 'ResponseMetadata': ... }

@codecov-io
Copy link

codecov-io commented Nov 25, 2019 •
edited
Loading

Codecov Report

Merging #1901 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1901   +/-   ##
========================================
  Coverage    92.62%   92.62%           
========================================
  Files           53       53           
  Lines        10035    10035           
========================================
  Hits          9295     9295           
  Misses         740      740
Impacted Files Coverage Δ
botocore/handlers.py 96.92% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be2bd7e...a73d9c2. Read the comment docs.

tipabu added a commit to tipabu/s3-tests that referenced this pull request Nov 25, 2019
@tipabu
Fix test_bucket_list_prefix_unreadable
5c82ddc 
Following boto/botocore#726, list_objects will
automatically include a encoding-type=url query param. However, the
client does not decode all of the response elements properly -- notably,
Prefix would remain encoded.

Hopefully this will be fixed soon-ish (I've got a patch proposed at
boto/botocore#1901) but in the meantime, use the
work-around suggested in boto/boto3#816 of
unregistering the set_list_objects_encoding_type_url handler.

Signed-off-by: Tim Burke <tim.burke@gmail.com>
@tipabu tipabu mentioned this pull request Nov 25, 2019
Open
@tipabu
Copy link
Contributor Author

tipabu commented Apr 9, 2020

Anything else I should do to help move this forward?

@tipabu
Copy link
Contributor Author

tipabu commented Apr 4, 2022

It's been a couple years now... Anything I can do to help move this forward?

tipabu added a commit to tipabu/s3-tests that referenced this pull request Apr 4, 2022
@tipabu
Fix test_bucket_list_prefix_unreadable
c0f90da 
Following boto/botocore#726, list_objects will
automatically include a encoding-type=url query param. However, the
client does not decode all of the response elements properly -- notably,
Prefix would remain encoded.

Hopefully this will be fixed soon-ish (I've got a patch proposed at
boto/botocore#1901) but in the meantime, use the
work-around suggested in boto/boto3#816 of
unregistering the set_list_objects_encoding_type_url handler.

Signed-off-by: Tim Burke <tim.burke@gmail.com>
@tim-finnigan
Copy link
Contributor

Hi @tipabu thanks for your patience. I wanted to check in here as we are going through old PRs and this one appears to have fallen off of the radar. I brought this up for discussion with the team, and the consensus was that more information is needed here before moving forward.

If this is still a problem that needs addressing, can you create a corresponding issue for tracking? We would like to gather more info on the use cases for this, whether there are workarounds, and possible approaches for addressing the issue.

@tim-finnigan tim-finnigan added the response-requested Waiting on additional info and feedback. label May 1, 2023
@tim-finnigan
Copy link
Contributor

Closing as we have not heard back regarding the previous comment. The team can revisit this PR if necessary but as mentioned above, more information is needed and should be provided in a tracking issue for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
response-requested Waiting on additional info and feedback.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tipabu @codecov-io @tim-finnigan