bug #22321 [Filesystem] Fixed makePathRelative (ausi)

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, fc745f45949fdb8d5aa590618ec73537721f99b4
- Missing normalization of `./` path segments, 15982d4b083723555cfa149368eaaae9609d0e22
- `../` got ignored at the beginning of relative paths, 9586e880d69f613b10e23dd53cea877e622b221a
- The documentation of the method only allowed absolute paths, but there are already unit tests ([FilesystemTest.php:1097](ab93feae3f/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php (L1097))) that test the behavior of relative paths, cec473eeb099c074b5883e7187f74663402f9d87

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

Commits
-------

2bc11505f4 [Filesystem] Fixed makePathRelative
This commit is contained in:
Fabien Potencier 2017-09-17 08:14:54 -07:00
commit 3b42d8859e
2 changed files with 30 additions and 20 deletions

View File

@ -368,25 +368,28 @@ class Filesystem
$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;
}
}
@ -394,8 +397,8 @@ class Filesystem
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;
@ -410,13 +413,8 @@ class Filesystem
$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));

View File

@ -850,6 +850,7 @@ class FilesystemTest extends FilesystemTestCase
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/', './'),
@ -881,6 +882,17 @@ class FilesystemTest extends FilesystemTestCase
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) {