Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Filesystem] Fix mirroring a directory with a relative path and a custom iterator#26771
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
5e288ac to26d858fCompare5147080 to2799f3fCompare835bdfc to748e3f7Compare9f14b18 to855d866Compare03d4673 to023704eComparefabpot commentedJul 18, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
We've removed usage or |
fxbt commentedJul 19, 2018
I wasn't aware of this, but it makes sense. What do you think the correct approach is ? Throwing an exception to avoid mirroring a bunch of useless data ? |
fabpot commentedJul 19, 2018
Throwing might be a good idea (calling |
91bdfb7 toad7062cComparefxbt commentedJul 19, 2018
An exception is now thrown and I tried to be explicit with the error message. The fabbot failure is a false negative. |
nicolas-grekas commentedJul 26, 2018
(could you please rebase on latest 2.8 and fix the CS issues fabbot is reporting?) |
fxbt commentedJul 26, 2018
Done! |
fabpot left a comment
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.
LGTM, I'm going to merge it in master.
fabpot commentedOct 10, 2018
Thank you@fxbt. |
… path and a custom iterator (fxbt)This PR was submitted for the 2.8 branch but it was squashed and merged into the 4.2-dev branch instead (closes#26771).Discussion----------[Filesystem] Fix mirroring a directory with a relative path and a custom iterator| Q | A| ------------- | ---| Branch? | 2.8 up to 4.1| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |The Filesystem::mirror method with a Finder iterator doesn't work if the origin directory is relative.This is a case where it won't work:```$dir = '/data/../data/';$finder = (new Finder())->in($dir)->getIterator();(new Filesystem())->mirror($dir, '/tmp', $finder);```The finder will return file objects like this :```SplFileInfo { pathname: "/data/file.tmp" ...}```But the following line will fail because because the `$file->getPathname()` and the `$originDir` are differents.```$target = $targetDir.substr($file->getPathname(), $originDirLen);// $file->getPathname() = '/data/file.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp' instead of '/tpm/file.tpm'```In some case, it's even worse. If the filename length is bigger than the `$originDirLen`, the target file will be a file with a completely wrong name:```// $file->getPathname() = '/data/file123456789.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp/56789.tmp' instead of '/tpm/file123456789.tmp'```I fixed this on my side by using the realpath function everytime i'm calling the mirror method, but i doubt this is the desired behavior.Commits-------27b673c [Filesystem] Fix mirroring a directory with a relative path and a custom iterator
Uh oh!
There was an error while loading.Please reload this page.
The Filesystem::mirror method with a Finder iterator doesn't work if the origin directory is relative.
This is a case where it won't work:
The finder will return file objects like this :
But the following line will fail because because the
$file->getPathname()and the$originDirare differents.In some case, it's even worse. If the filename length is bigger than the
$originDirLen, the target file will be a file with a completely wrong name:I fixed this on my side by using the realpath function everytime i'm calling the mirror method, but i doubt this is the desired behavior.