Skip to content

Commit

Permalink
bug #22321 [Filesystem] Fixed makePathRelative (ausi)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 2.7 branch (closes #22321).

Discussion
----------

[Filesystem] Fixed makePathRelative

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Updating to Symfony 3.2.7 @agoat noticed a bug with `Filesystem::makePathRelative()` in contao/core-bundle#751:

- In Symfony 3.2.6 `makePathRelative('aa/cc', 'bb/cc')` returned correctly `../../aa/cc`
- In Symfony 3.2.7 the same method call returns `./`

I think this issue was introduced with #22133.

While working on the fix I noticed some other issues too:

- An unnecessary if construct that did nothing, fc745f4
- Missing normalization of `./` path segments, 15982d4
- `../` got ignored at the beginning of relative paths, 9586e88
- The documentation of the method only allowed absolute paths, but there are already unit tests ([FilesystemTest.php:1097](https://github.com/symfony/symfony/blob/ab93feae3f9a16c4f18c5736435d18fa36338d2c/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php#L1097)) that test the behavior of relative paths, cec473e

This pull request fixes all these issues and adds tests for them.

Commits
-------

2bc1150 [Filesystem] Fixed makePathRelative
  • Loading branch information
fabpot committed Sep 17, 2017
2 parents b973ecf + 2bc1150 commit 3b42d88
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 20 deletions.
38 changes: 18 additions & 20 deletions src/Symfony/Component/Filesystem/Filesystem.php
Expand Up @@ -368,34 +368,37 @@ public function makePathRelative($endPath, $startPath)
$startPath = str_replace('\\', '/', $startPath);
}

$stripDriveLetter = function ($path) {
if (strlen($path) > 2 && ':' === $path[1] && '/' === $path[2] && ctype_alpha($path[0])) {
return substr($path, 2);
}

return $path;
};

$endPath = $stripDriveLetter($endPath);
$startPath = $stripDriveLetter($startPath);

// Split the paths into arrays
$startPathArr = explode('/', trim($startPath, '/'));
$endPathArr = explode('/', trim($endPath, '/'));

if ('/' !== $startPath[0]) {
array_shift($startPathArr);
}

if ('/' !== $endPath[0]) {
array_shift($endPathArr);
}

$normalizePathArray = function ($pathSegments) {
$normalizePathArray = function ($pathSegments, $absolute) {
$result = array();

foreach ($pathSegments as $segment) {
if ('..' === $segment) {
if ('..' === $segment && ($absolute || count($result))) {
array_pop($result);
} else {
} elseif ('.' !== $segment) {
$result[] = $segment;
}
}

return $result;
};

$startPathArr = $normalizePathArray($startPathArr);
$endPathArr = $normalizePathArray($endPathArr);
$startPathArr = $normalizePathArray($startPathArr, static::isAbsolutePath($startPath));
$endPathArr = $normalizePathArray($endPathArr, static::isAbsolutePath($endPath));

// Find for which directory the common path stops
$index = 0;
Expand All @@ -410,13 +413,8 @@ public function makePathRelative($endPath, $startPath)
$depth = count($startPathArr) - $index;
}

// When we need to traverse from the start, and we are starting from a root path, don't add '../'
if ('/' === $startPath[0] && 0 === $index && 0 === $depth) {
$traverser = '';
} else {
// Repeated "../" for each level need to reach the common path
$traverser = str_repeat('../', $depth);
}
// Repeated "../" for each level need to reach the common path
$traverser = str_repeat('../', $depth);

$endPathRemainder = implode('/', array_slice($endPathArr, $index));

Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
Expand Up @@ -850,6 +850,7 @@ public function providePathsForMakePathRelative()
array('/var/lib/symfony/src/Symfony', '/var/lib/symfony/src/Symfony/Component/', '../'),
array('var/lib/symfony/', 'var/lib/symfony/src/Symfony/Component', '../../../'),
array('/usr/lib/symfony/', '/var/lib/symfony/src/Symfony/Component', '../../../../../../usr/lib/symfony/'),
array('usr/lib/symfony/', 'var/lib/symfony/src/Symfony/Component', '../../../../../../usr/lib/symfony/'),
array('/var/lib/symfony/src/Symfony/', '/var/lib/symfony/', 'src/Symfony/'),
array('/aa/bb', '/aa/bb', './'),
array('/aa/bb', '/aa/bb/', './'),
Expand Down Expand Up @@ -881,6 +882,17 @@ public function providePathsForMakePathRelative()
array('C:/aa/bb/../../cc', 'C:/aa/../dd/..', 'cc/'),
array('C:/../aa/bb/cc', 'C:/aa/dd/..', 'bb/cc/'),
array('C:/../../aa/../bb/cc', 'C:/aa/dd/..', '../bb/cc/'),
array('aa/bb', 'aa/cc', '../bb/'),
array('aa/cc', 'bb/cc', '../../aa/cc/'),
array('aa/bb', 'aa/./cc', '../bb/'),
array('aa/./bb', 'aa/cc', '../bb/'),
array('aa/./bb', 'aa/./cc', '../bb/'),
array('../../', '../../', './'),
array('../aa/bb/', 'aa/bb/', '../../../aa/bb/'),
array('../../../', '../../', '../'),
array('', '', './'),
array('', 'aa/', '../'),
array('aa/', '', 'aa/'),
);

if ('\\' === DIRECTORY_SEPARATOR) {
Expand Down

0 comments on commit 3b42d88

Please sign in to comment.