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.

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

Ensure Storage.object parameter encodes "/" chars (#6510) #7159

Merged
merged 3 commits into from
Mar 9, 2021

Conversation

jamesvl
Copy link
Contributor

@jamesvl jamesvl commented Jan 15, 2021

Add a simple exemption for the Storage object parameter to not treat it as a "path trailer" parameter.

Issue reported in #6510

Background

Changes released in v0.24 (PR 5869 for Issue #4600) allow a parameter, if the last one in a path, to not URI.encode() the path separator (/).

However, the Storage API requires the object parameter to have path separator characters encoded, even though it is the last parameter in a URL path.

Since this seems to be the only API people have reported about, I've added a specific exception for the Storage object parameter to make sure it gets the full URI encoding.

Also closes #6352, which I believe is a duplicate of #6510.

@jamesvl jamesvl requested a review from a team as a code owner January 15, 2021 19:42
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 15, 2021
Simple exemption for that parameter, since it appears to be the only one
affected.

Update unit test to check for properly encoded object parameter (note
that it matches what `gsutil -D rm ...` is doing).

Issue googleapis#6352 is probably a duplicate of googleapis#6510.
@jamesvl
Copy link
Contributor Author

jamesvl commented Feb 19, 2021

Anything I can do to help get this reviewed?

Copy link
Member

@dazuma dazuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't get to this. This looks great! Thanks much! I made a small tweak to make sure it gets applied only to the Storage API (and not any other API that happens to have a parameter called object).

@dazuma dazuma merged commit fc4b282 into googleapis:master Mar 9, 2021
@davidye
Copy link

davidye commented May 11, 2021

@dazuma I think this is still broken for storage_objects_copy and storage_objects_rewrite?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem copying objects with a / in the path with latest client
3 participants