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

AwsS3 Fix rename method for url not safe source #633

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexgt9
Copy link

@alexgt9 alexgt9 commented Jan 3, 2020

The rename method fails when using non url safe chars in the source parameter.

Error executing "CopyObject" on "https://****.amazonaws.com/FileWithNonUrlSafeChar%_.pdf"; AWS HTTP error: Client error: PUT https://eu-west-1-sg-prod-files.s3.eu-west-1.amazonaws.com/FileWithNonUrlSafeChar%_.pdf resulted in a 400 Bad Request response:
InvalidArgumentInvalid copy source encoding.x-amz-copy-source</Argu (truncated...)

I just url encode the CopySource parameter in the API call

$this->filesystem->write('foo', '');
$this->filesystem->rename('foo', 'foo%_encode');

$this->assertTrue($this->filesystem->has('foo'));
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the original name not be present ?

Copy link
Author

Choose a reason for hiding this comment

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

You are right 👌
Anyway there is no environment where this tests for S3 are executed?

@alexgt9 alexgt9 changed the title Add test for rename url not safe source S3 Fix rename method for url not safe source Jan 7, 2020
@alexgt9 alexgt9 changed the title S3 Fix rename method for url not safe source AwsS3 Fix rename method for url not safe source Jan 7, 2020
Copy link
Contributor

@Nek- Nek- left a comment

Choose a reason for hiding this comment

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

Hello, thank you very much for trying to improve Gaufrette.

I didn't test your fix but I see that the test passed without the change you did in the code.

/**
* @test
*/
public function shouldRenameAnObject()
Copy link
Contributor

Choose a reason for hiding this comment

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

This test pass with and without the fix. The test name is also not clear regarding what it fixes.

@PedroTroller PedroTroller self-assigned this Apr 5, 2023
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.

4 participants