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

change encoding of x-amz-copy-source in copy_object #912

Merged
merged 8 commits into from
May 10, 2020
Merged

change encoding of x-amz-copy-source in copy_object #912

merged 8 commits into from
May 10, 2020

Conversation

johnnyvf24
Copy link
Contributor

Hello Minio Team,

Talked with @harshavardhana and it seems the encoding is a bit off for the minio python sdk. It seems the value of the x-amz-copy-source does not need to encode the individual / characters in a provided path.

minio/api.py Outdated Show resolved Hide resolved
minio/api.py Outdated Show resolved Hide resolved
@johnnyvf24
Copy link
Contributor Author

Given my bad commits, I'd definitely compress them on merging.

@kannappanr kannappanr requested a review from vadmeste May 5, 2020 19:38
@vadmeste
Copy link
Member

vadmeste commented May 6, 2020

@johnnyvf24 I just tested copy_object() and it seems it works fine. Could you say what's specific in your case ?

@johnnyvf24
Copy link
Contributor Author

@vadmeste how did you test it? I have yet to test AWS, but based on their JS SDK, it seems that the actual slashes are not to be encoded. In my case I am using this python client for a minio instance and a netapp storagegrid S3 compatible service. This client has worked great for everything except for copy object. In my particular case I was trying to copy an object such as sample/path/to/object from one bucket to another in the netapp storage service. At first I thought it was a netapp problem but based on code I have seen from other clients I now think this is a minio-py difference in interpretation of implementation.

@johnnyvf24
Copy link
Contributor Author

@vadmeste any update on this?

@harshavardhana
Copy link
Member

@johnnyvf24 I just tested copy_object() and it seems it works fine. Could you say what's specific in your case ?

The issue is with NetApp Storage Grid which is not forthcoming in its parsing of x-amz-copy-source here, although there is no need to convert / to be %2F it was implicitly done by minio-py which sort of breaks NetApp Storage Grid product.

NOTE: without this change minio-py works fine with MinIO and AWS S3 because of both support %2F as well automatically.

But it doesn't hurt to fix minio-py

Copy link
Collaborator

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr kannappanr merged commit 165d3d6 into minio:master May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants