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

Escape the special char in name correctly. As per the AWS documentat… #579

Closed
wants to merge 1 commit into from

Conversation

skuppa
Copy link
Contributor

@skuppa skuppa commented Nov 6, 2020

Escape the special char in name correctly. As per the AWS documentation, CopyRequest is sending request param and it advised to URLEncode the source name. Using the url.QueryEscape to encode the name instead of url.PahEscape since it does not cover all special char.

…on, CopyRequest is sending request param and it advised to URLEncode the source name. Using the url.QueryEscape to encode the name instead of urlPahEscape since it does not cover all special char.
@skuppa
Copy link
Contributor Author

skuppa commented Nov 6, 2020

Fixes #580

@@ -475,7 +475,7 @@ func (s *S3Backend) mpuCopyPart(from string, to string, mpuId string, bytes stri
params := &s3.UploadPartCopyInput{
Bucket: &s.bucket,
Key: &to,
CopySource: aws.String(pathEscape(from)),
CopySource: aws.String(url.QueryEscape(from)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the QueryEscape to encode URLEncode the params.

@@ -667,7 +667,7 @@ func (s *S3Backend) CopyBlob(param *CopyBlobInput) (*CopyBlobOutput, error) {

params := &s3.CopyObjectInput{
Bucket: &s.bucket,
CopySource: aws.String(pathEscape(from)),
CopySource: aws.String(url.QueryEscape(from)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the QueryEscape to encode URLEncode the params. The from is a leaf node (just name, not a path) hence QueryEscape will escape all special characters. Even if the from has a forward slash (/) then it will be escaped and AWS unescaping and working correctly.

@@ -571,13 +571,6 @@ func mapAwsError(err error) error {
}
}

// note that this is NOT the same as url.PathEscape in golang 1.8,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed since it is not used anywhere.

Copy link
Contributor

@monthonk monthonk left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think we should get this merged, just need a rebase before we can do that

@kahing
Copy link
Owner

kahing commented Nov 3, 2022

merged via 5ae08c0

@kahing kahing closed this Nov 3, 2022
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.

3 participants