-
Notifications
You must be signed in to change notification settings - Fork 356
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
base: master
Are you sure you want to change the base?
Conversation
$this->filesystem->write('foo', ''); | ||
$this->filesystem->rename('foo', 'foo%_encode'); | ||
|
||
$this->assertTrue($this->filesystem->has('foo')); |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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?
Update AwsS3Test.php
There was a problem hiding this 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() |
There was a problem hiding this comment.
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.
The rename method fails when using non url safe chars in the source parameter.
I just url encode the
CopySource
parameter in the API call