summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRoeland Jago Douma <roeland@famdouma.nl>2018-10-08 22:51:40 +0200
committerRoeland Jago Douma <roeland@famdouma.nl>2018-10-10 13:23:59 +0200
commitd5bf2c4523f0f80c6b7d2e084d7b4199c4f19de1 (patch)
tree9d86c6dec058e9c937479e146ed26a916a7d51ce
parent1dbd7172c31b2d611e49c7c8a60a05da134a4733 (diff)
downloadnextcloud-server-d5bf2c4523f0f80c6b7d2e084d7b4199c4f19de1.tar.gz
nextcloud-server-d5bf2c4523f0f80c6b7d2e084d7b4199c4f19de1.zip
Move normalizePath to regexes instead of looping
This is IMO a bit more readable and it seems to make the code faster. Tested it on the company instance where there are over 3k calls to this function. It shaves off around 10ms. The advantage here is that the pattern gets optimized by php itsel and cached. Also looking for all patterns at the same time and especially no longer looping for /./ patterns should save time. Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
-rw-r--r--lib/private/Files/Filesystem.php41
-rw-r--r--tests/lib/Files/FilesystemTest.php129
2 files changed, 84 insertions, 86 deletions
diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php
index 12e59e5ba2d..ba9c85deeee 100644
--- a/lib/private/Files/Filesystem.php
+++ b/lib/private/Files/Filesystem.php
@@ -812,7 +812,7 @@ class Filesystem {
return self::$normalizedPathCache[$cacheKey];
}
- if ($path == '') {
+ if ($path === '') {
return '/';
}
@@ -821,38 +821,29 @@ class Filesystem {
$path = \OC_Util::normalizeUnicode($path);
}
- //no windows style slashes
- $path = str_replace('\\', '/', $path);
+ //add leading slash, if it is already there we strip it anyway
+ $path = '/' . $path;
- //add leading slash
- if ($path[0] !== '/') {
- $path = '/' . $path;
- }
+ $patterns = [
+ '/\\\\/s', // no windows style slashes
+ '/\/\.(\/\.)?\//s', // remove '/./'
+ '/\/{2,}/s', // remove squence of slashes
+ '/\/\.$/s', // remove trailing /.
+ ];
- // remove '/./'
- // ugly, but str_replace() can't replace them all in one go
- // as the replacement itself is part of the search string
- // which will only be found during the next iteration
- while (strpos($path, '/./') !== false) {
- $path = str_replace('/./', '/', $path);
- }
- // remove sequences of slashes
- $path = preg_replace('#/{2,}#', '/', $path);
+ do {
+ $count = 0;
+ $path = preg_replace($patterns, '/', $path, -1, $count);
+ } while ($count > 0);
//remove trailing slash
- if ($stripTrailingSlash and strlen($path) > 1) {
+ if ($stripTrailingSlash && strlen($path) > 1) {
$path = rtrim($path, '/');
}
- // remove trailing '/.'
- if (substr($path, -2) == '/.') {
- $path = substr($path, 0, -2);
- }
-
- $normalizedPath = $path;
- self::$normalizedPathCache[$cacheKey] = $normalizedPath;
+ self::$normalizedPathCache[$cacheKey] = $path;
- return $normalizedPath;
+ return $path;
}
/**
diff --git a/tests/lib/Files/FilesystemTest.php b/tests/lib/Files/FilesystemTest.php
index a98af220ba1..6fc6fbe9d7c 100644
--- a/tests/lib/Files/FilesystemTest.php
+++ b/tests/lib/Files/FilesystemTest.php
@@ -115,73 +115,80 @@ class FilesystemTest extends \Test\TestCase {
}
public function normalizePathData() {
- return array(
- array('/', ''),
- array('/', '/'),
- array('/', '//'),
- array('/', '/', false),
- array('/', '//', false),
-
- array('/path', '/path/'),
- array('/path/', '/path/', false),
- array('/path', 'path'),
-
- array('/foo/bar', '/foo//bar/'),
- array('/foo/bar/', '/foo//bar/', false),
- array('/foo/bar', '/foo////bar'),
- array('/foo/bar', '/foo/////bar'),
- array('/foo/bar', '/foo/bar/.'),
- array('/foo/bar', '/foo/bar/./'),
- array('/foo/bar/', '/foo/bar/./', false),
- array('/foo/bar', '/foo/bar/./.'),
- array('/foo/bar', '/foo/bar/././'),
- array('/foo/bar/', '/foo/bar/././', false),
- array('/foo/bar', '/foo/./bar/'),
- array('/foo/bar/', '/foo/./bar/', false),
- array('/foo/.bar', '/foo/.bar/'),
- array('/foo/.bar/', '/foo/.bar/', false),
- array('/foo/.bar/tee', '/foo/.bar/tee'),
+ return [
+ ['/', ''],
+ ['/', '/'],
+ ['/', '//'],
+ ['/', '/', false],
+ ['/', '//', false],
+
+ ['/path', '/path/'],
+ ['/path/', '/path/', false],
+ ['/path', 'path'],
+
+ ['/foo/bar', '/foo//bar/'],
+ ['/foo/bar/', '/foo//bar/', false],
+ ['/foo/bar', '/foo////bar'],
+ ['/foo/bar', '/foo/////bar'],
+ ['/foo/bar', '/foo/bar/.'],
+ ['/foo/bar', '/foo/bar/./'],
+ ['/foo/bar/', '/foo/bar/./', false],
+ ['/foo/bar', '/foo/bar/./.'],
+ ['/foo/bar', '/foo/bar/././'],
+ ['/foo/bar/', '/foo/bar/././', false],
+ ['/foo/bar', '/foo/./bar/'],
+ ['/foo/bar/', '/foo/./bar/', false],
+ ['/foo/.bar', '/foo/.bar/'],
+ ['/foo/.bar/', '/foo/.bar/', false],
+ ['/foo/.bar/tee', '/foo/.bar/tee'],
+
+ ['/foo/bar', '/.///././//./foo/.///././//./bar/./././.'],
+ ['/foo/bar/', '/.///././//./foo/.///././//./bar/./././.', false],
+ ['/foo/bar', '/.///././//./foo/.///././//./bar/././././'],
+ ['/foo/bar/', '/.///././//./foo/.///././//./bar/././././', false],
// Windows paths
- array('/', ''),
- array('/', '\\'),
- array('/', '\\', false),
- array('/', '\\\\'),
- array('/', '\\\\', false),
-
- array('/path', '\\path'),
- array('/path', '\\path', false),
- array('/path', '\\path\\'),
- array('/path/', '\\path\\', false),
-
- array('/foo/bar', '\\foo\\\\bar\\'),
- array('/foo/bar/', '\\foo\\\\bar\\', false),
- array('/foo/bar', '\\foo\\\\\\\\bar'),
- array('/foo/bar', '\\foo\\\\\\\\\\bar'),
- array('/foo/bar', '\\foo\\bar\\.'),
- array('/foo/bar', '\\foo\\bar\\.\\'),
- array('/foo/bar/', '\\foo\\bar\\.\\', false),
- array('/foo/bar', '\\foo\\bar\\.\\.'),
- array('/foo/bar', '\\foo\\bar\\.\\.\\'),
- array('/foo/bar/', '\\foo\\bar\\.\\.\\', false),
- array('/foo/bar', '\\foo\\.\\bar\\'),
- array('/foo/bar/', '\\foo\\.\\bar\\', false),
- array('/foo/.bar', '\\foo\\.bar\\'),
- array('/foo/.bar/', '\\foo\\.bar\\', false),
- array('/foo/.bar/tee', '\\foo\\.bar\\tee'),
+ ['/', ''],
+ ['/', '\\'],
+ ['/', '\\', false],
+ ['/', '\\\\'],
+ ['/', '\\\\', false],
+
+ ['/path', '\\path'],
+ ['/path', '\\path', false],
+ ['/path', '\\path\\'],
+ ['/path/', '\\path\\', false],
+
+ ['/foo/bar', '\\foo\\\\bar\\'],
+ ['/foo/bar/', '\\foo\\\\bar\\', false],
+ ['/foo/bar', '\\foo\\\\\\\\bar'],
+ ['/foo/bar', '\\foo\\\\\\\\\\bar'],
+ ['/foo/bar', '\\foo\\bar\\.'],
+ ['/foo/bar', '\\foo\\bar\\.\\'],
+ ['/foo/bar/', '\\foo\\bar\\.\\', false],
+ ['/foo/bar', '\\foo\\bar\\.\\.'],
+ ['/foo/bar', '\\foo\\bar\\.\\.\\'],
+ ['/foo/bar/', '\\foo\\bar\\.\\.\\', false],
+ ['/foo/bar', '\\foo\\.\\bar\\'],
+ ['/foo/bar/', '\\foo\\.\\bar\\', false],
+ ['/foo/.bar', '\\foo\\.bar\\'],
+ ['/foo/.bar/', '\\foo\\.bar\\', false],
+ ['/foo/.bar/tee', '\\foo\\.bar\\tee'],
// Absolute windows paths NOT marked as absolute
- array('/C:', 'C:\\'),
- array('/C:/', 'C:\\', false),
- array('/C:/tests', 'C:\\tests'),
- array('/C:/tests', 'C:\\tests', false),
- array('/C:/tests', 'C:\\tests\\'),
- array('/C:/tests/', 'C:\\tests\\', false),
+ ['/C:', 'C:\\'],
+ ['/C:/', 'C:\\', false],
+ ['/C:/tests', 'C:\\tests'],
+ ['/C:/tests', 'C:\\tests', false],
+ ['/C:/tests', 'C:\\tests\\'],
+ ['/C:/tests/', 'C:\\tests\\', false],
+ ['/C:/tests/bar', 'C:\\tests\\.\\.\\bar'],
+ ['/C:/tests/bar/', 'C:\\tests\\.\\.\\bar\\.\\', false],
// normalize does not resolve '..' (by design)
- array('/foo/..', '/foo/../'),
- array('/foo/..', '\\foo\\..\\'),
- );
+ ['/foo/..', '/foo/../'],
+ ['/foo/..', '\\foo\\..\\'],
+ ];
}
/**