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

[Filesystem] Fix trying to mirror a directory into itself or in his child #30116

Open
wants to merge 2 commits into
base: 3.4
from

Conversation

Projects
None yet
5 participants
@XuruDragon
Copy link
Contributor

XuruDragon commented Feb 8, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none / PR #29857
License MIT
Doc PR n/a

This this the continuity of #29857 by @Fleuv

Fix a bug while trying to mirror a directory into itself or in a child

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Feb 8, 2019

@XuruDragon XuruDragon force-pushed the XuruDragon:fix-filesystem-mirroir branch 3 times, most recently from 12c07b8 to 55e8d21 Feb 8, 2019

@@ -567,6 +567,10 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o
}
foreach ($iterator as $file) {
if ($file->getPathName() === $targetDir) {

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Feb 16, 2019

Member

I'm not sure how/if this works with relative directories, but we can care in a later PR if anyone is up to.

@@ -567,6 +567,10 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o
}
foreach ($iterator as $file) {
if ($file->getPathName() === $targetDir) {

This comment has been minimized.

@fabpot

fabpot Feb 21, 2019

Member

AFAICS, $targetDir is not normalized where ->getPathName() always return a nomalized version of the path, so the comparison can fail for the same directory in many cases. Can we try to be more "robust"? Perhaps by using realpath()?

@XuruDragon XuruDragon force-pushed the XuruDragon:fix-filesystem-mirroir branch 2 times, most recently from 711ba7f to fe32a0a Feb 21, 2019

Fleuv and others added some commits Jan 11, 2019

Skipping iterations in the mirror() method where the iterated file's …
…path name is equal to the target path

Added a new test what is called testMirrorAvoidCopyingTargetDirectoryIfInSourceDirectory

Adjustments to comply with the Symfony Code Standards

@XuruDragon XuruDragon force-pushed the XuruDragon:fix-filesystem-mirroir branch from fe32a0a to 785dc05 Feb 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment