From c57dfb4bfefdaccfff4e8ba22c45f3610613b3f6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 27 Jul 2026 10:34:43 +0200 Subject: [PATCH 1/3] perf: Optimize Filesystem::normalizePath Cache entries as often it's the same path getting normalized again and again. Skip unicode normalization when no unicode is present, use simple string replace instead of regex when possible. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- lib/private/Files/Filesystem.php | 71 ++++++++++++++++++++------ lib/private/Files/Utils/PathHelper.php | 43 ++++++++-------- 2 files changed, 77 insertions(+), 37 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 022467e754877..49c99f3a75767 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -12,6 +12,7 @@ use OC\Files\Storage\Storage; use OC\Files\Storage\StorageFactory; use OC\User\NoUserException; +use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\Node\FilesystemTornDownEvent; use OCP\Files\InvalidPathException; @@ -48,6 +49,14 @@ class Filesystem { */ private static ?FilenameValidator $validator = null; + /** + * Memoized results of normalizePath(), keyed by input path + flags + * + * @psalm-suppress ImpureStaticProperty This class has a reset method + * @var CappedMemoryCache + */ + private static ?CappedMemoryCache $normalizedPathCache = null; + /** * @psalm-suppress ImpureStaticProperty This class has a reset method */ @@ -616,7 +625,7 @@ public static function hasUpdated($path, $time) { * @psalm-taint-escape file * @return string */ - public static function normalizePath($path, $stripTrailingSlash = true, $isAbsolutePath = false, $keepUnicode = false) { + public static function normalizePath($path, $stripTrailingSlash = true, $isAbsolutePath = false, bool $keepUnicode = false) { /** * FIXME: This is a workaround for existing classes and files which call * this function with another type than a valid string. This @@ -629,31 +638,60 @@ public static function normalizePath($path, $stripTrailingSlash = true, $isAbsol return '/'; } - //normalize unicode if possible - if (!$keepUnicode) { + // paths repeat heavily within a request (same file touched by permission + // checks, cache lookups, hooks, ...), so memoize the result + $cacheKey = $keepUnicode ? "1\0$path" : "0\0$path"; + $cacheKey = ($stripTrailingSlash ? "1\0" : "0\0") . $cacheKey; + if (self::$normalizedPathCache === null) { + self::$normalizedPathCache = new CappedMemoryCache(); + } + if (isset(self::$normalizedPathCache[$cacheKey])) { + return self::$normalizedPathCache[$cacheKey]; + } + + // normalize unicode if possible, skipping the ICU normalizer entirely for + // plain ASCII paths, which are already in normalization form C + if (!$keepUnicode && preg_match('/[\x80-\xff]/', $path)) { $path = \OC_Util::normalizeUnicode($path); } - //add leading slash, if it is already there we strip it anyway - $path = '/' . $path; + // Add leading slash if it's missing + if ($path[0] !== '/') { + $path = '/' . $path; + } - $patterns = [ - '#\\\\#s', // no windows style '\\' slashes - '#/\.(/\.)*/#s', // remove '/./' - '#\//+#s', // remove sequence of slashes - '#/\.$#s', // remove trailing '/.' - ]; + // No null bytes + if (str_contains($path, chr(0))) { + $path = str_replace(chr(0), '', $path); + } - do { - $count = 0; - $path = preg_replace($patterns, '/', $path, -1, $count); - } while ($count > 0); + // No windows style slashes + if (str_contains($path, '\\')) { + $path = str_replace('\\', '/', $path); + } - //remove trailing slash + // most paths reaching here are already clean, so skip the regex engine entirely + // when neither a duplicate slash nor a '.' segment is present + if (str_contains($path, '//') || str_contains($path, '/.')) { + $patterns = [ + '#/\.(/\.)*/#s', // remove '/./' + '#\//+#s', // remove sequence of slashes + '#/\.$#s', // remove trailing '/.' + ]; + + do { + $count = 0; + $path = preg_replace($patterns, '/', $path, -1, $count); + } while ($count > 0); + } + + // remove trailing slash if ($stripTrailingSlash && strlen($path) > 1) { $path = rtrim($path, '/'); } + self::$normalizedPathCache[$cacheKey] = $path; + return $path; } @@ -729,5 +767,6 @@ public static function reset(): void { self::$loaded = false; self::$mounts = null; self::$validator = null; + self::$normalizedPathCache = new CappedMemoryCache(); } } diff --git a/lib/private/Files/Utils/PathHelper.php b/lib/private/Files/Utils/PathHelper.php index ac29ff20e84f2..a5ba0218ff62c 100644 --- a/lib/private/Files/Utils/PathHelper.php +++ b/lib/private/Files/Utils/PathHelper.php @@ -11,48 +11,49 @@ class PathHelper { /** * Make a path relative to a root path, or return null if the path is outside the root - * - * @param string $root - * @param string $path - * @return ?string */ - public static function getRelativePath(string $root, string $path) { + public static function getRelativePath(string $root, string $path): ?string { if ($root === '' || $root === '/') { return self::normalizePath($path); } if ($path === $root) { return '/'; - } elseif (!str_starts_with($path, $root . '/')) { + } + + if (!str_starts_with($path, $root . '/')) { return null; - } else { - $path = substr($path, strlen($root)); - return self::normalizePath($path); } + + $path = substr($path, strlen($root)); + return self::normalizePath($path); } - /** - * @param string $path - * @return string - */ public static function normalizePath(string $path): string { if ($path === '' || $path === '/') { return '/'; } + // No null bytes - $path = str_replace(chr(0), '', $path); - //no windows style slashes - $path = str_replace('\\', '/', $path); - //add leading slash + if (str_contains($path, chr(0))) { + $path = str_replace(chr(0), '', $path); + } + + // No windows style slashes + if (str_contains($path, '\\')) { + $path = str_replace('\\', '/', $path); + } + + // Add leading slash if it's missing if ($path[0] !== '/') { $path = '/' . $path; } - //remove duplicate slashes + + // Remove duplicate slashes while (str_contains($path, '//')) { $path = str_replace('//', '/', $path); } - //remove trailing slash - $path = rtrim($path, '/'); - return $path; + // Remove trailing slash + return rtrim($path, '/'); } } From 4ca01c0fe2032244b96b5b094316715ecc908c29 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 27 Jul 2026 11:04:48 +0200 Subject: [PATCH 2/3] perf: Call simpler PathHelper::normalizedPath on trusted input Signed-off-by: Carl Schwan --- build/psalm-baseline.xml | 3 --- lib/private/Files/Mount/MountPoint.php | 6 +++--- lib/private/Files/Node/Node.php | 17 +++------------- lib/private/Files/Storage/DAV.php | 3 ++- .../Files/Storage/Wrapper/Encryption.php | 5 +++-- lib/private/Files/Storage/Wrapper/Jail.php | 20 +++++++++++++------ 6 files changed, 25 insertions(+), 29 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 7e59e0d212c8e..3b29ccdb643d9 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3741,9 +3741,6 @@ - - - diff --git a/lib/private/Files/Mount/MountPoint.php b/lib/private/Files/Mount/MountPoint.php index 22a36fd583472..17fce74fe8031 100644 --- a/lib/private/Files/Mount/MountPoint.php +++ b/lib/private/Files/Mount/MountPoint.php @@ -8,9 +8,9 @@ namespace OC\Files\Mount; -use OC\Files\Filesystem; use OC\Files\Storage\Storage; use OC\Files\Storage\StorageFactory; +use OC\Files\Utils\PathHelper; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IStorageFactory; @@ -197,7 +197,7 @@ public function getNumericStorageId() { */ #[\Override] public function getInternalPath($path) { - $path = Filesystem::normalizePath($path, true, false, true); + $path = PathHelper::normalizePath($path); if ($this->mountPoint === $path || $this->mountPoint . '/' === $path) { $internalPath = ''; } else { @@ -212,7 +212,7 @@ public function getInternalPath($path) { * @return string */ private function formatPath($path) { - $path = Filesystem::normalizePath($path); + $path = PathHelper::normalizePath($path); if (strlen($path) > 1) { $path .= '/'; } diff --git a/lib/private/Files/Node/Node.php b/lib/private/Files/Node/Node.php index a599833567fbf..89391a30a5a97 100644 --- a/lib/private/Files/Node/Node.php +++ b/lib/private/Files/Node/Node.php @@ -29,31 +29,20 @@ // FIXME: this class really should be abstract (+1) class Node implements INode { /** - * @var View $view - */ - protected $view; - - protected IRootFolder $root; - - /** - * @param View $view - * @param \OCP\Files\IRootFolder $root * @param string $path * @param FileInfo $fileInfo */ public function __construct( - IRootFolder $root, - $view, + protected IRootFolder $root, + protected View $view, protected $path, protected ?FileInfo $fileInfo = null, protected ?INode $parent = null, private bool $infoHasSubMountsIncluded = true, ) { - if (Filesystem::normalizePath($view->getRoot()) !== '/') { + if (PathHelper::normalizePath($this->view->getRoot()) !== '/') { throw new PreConditionNotMetException('The view passed to the node should not have any fake root set'); } - $this->view = $view; - $this->root = $root; } /** diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index efcceba7e6892..0ae3b8948ebc3 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -12,6 +12,7 @@ use Icewind\Streams\CallbackWrapper; use Icewind\Streams\IteratorDirectory; use OC\Files\Filesystem; +use OC\Files\Utils\PathHelper; use OC\MemCache\ArrayCache; use OC\OCM\OCMSignatoryManager; use OCP\AppFramework\Http; @@ -990,7 +991,7 @@ public function cleanPath(string $path): string { if ($path === '') { return $path; } - $path = Filesystem::normalizePath($path); + $path = PathHelper::normalizePath($path); // remove leading slash return substr($path, 1); } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 20af8b1d06277..e7518b34ae84a 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -16,6 +16,7 @@ use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\Storage\Common; use OC\Files\Storage\LocalTempFileTrait; +use OC\Files\Utils\PathHelper; use OC\Memcache\ArrayCache; use OCP\Cache\CappedMemoryCache; use OCP\Encryption\Exceptions\InvalidHeaderException; @@ -784,7 +785,7 @@ public function hash(string $type, string $path, bool $raw = false): string|fals * @return string full path including mount point */ protected function getFullPath(string $path): string { - return Filesystem::normalizePath($this->mountPoint . '/' . $path); + return PathHelper::normalizePath($this->mountPoint . '/' . $path); } /** @@ -898,7 +899,7 @@ protected function copyKeys(string $source, string $target): bool { * check if path points to a files version */ protected function isVersion(string $path): bool { - $normalized = Filesystem::normalizePath($path); + $normalized = PathHelper::normalizePath($path); return substr($normalized, 0, strlen('/files_versions/')) === '/files_versions/'; } diff --git a/lib/private/Files/Storage/Wrapper/Jail.php b/lib/private/Files/Storage/Wrapper/Jail.php index c17bd1f750ded..8707427c26e32 100644 --- a/lib/private/Files/Storage/Wrapper/Jail.php +++ b/lib/private/Files/Storage/Wrapper/Jail.php @@ -11,8 +11,8 @@ use OC\Files\Cache\Wrapper\CacheJail; use OC\Files\Cache\Wrapper\JailPropagator; use OC\Files\Cache\Wrapper\JailWatcher; -use OC\Files\Filesystem; use OC\Files\Storage\FailedStorage; +use OC\Files\Utils\PathHelper; use OCP\Files\Cache\ICache; use OCP\Files\Cache\IPropagator; use OCP\Files\Cache\IWatcher; @@ -30,10 +30,10 @@ * This restricts access to a subfolder of the wrapped storage with the subfolder becoming the root folder new storage */ class Jail extends Wrapper { - /** - * @var string - */ - protected $rootPath; + protected string $rootPath; + + /** Normalized, slash-trimmed version of $rootPath */ + private ?string $normalizedRootPath = null; /** * @param array $parameters ['storage' => $storage, 'root' => $root] @@ -47,7 +47,15 @@ public function __construct(array $parameters) { } public function getUnjailedPath(string $path): string { - return trim(Filesystem::normalizePath($this->rootPath . '/' . $path), '/'); + if ($this->normalizedRootPath === null) { + $this->normalizedRootPath = trim(PathHelper::normalizePath($this->rootPath), '/'); + } + + $normalizedPath = trim(PathHelper::normalizePath($path), '/'); + if ($this->normalizedRootPath === '') { + return $normalizedPath; + } + return $normalizedPath === '' ? $this->normalizedRootPath : $this->normalizedRootPath . '/' . $normalizedPath; } /** From 149d8f942cdb6644c771bff95dd0df00aee45bf4 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 27 Jul 2026 12:11:03 +0200 Subject: [PATCH 3/3] perf: Remove cache It's inneficient when there are too many entries which happen often. Signed-off-by: Carl Schwan --- lib/private/Files/Filesystem.php | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 49c99f3a75767..f547f738285e9 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -49,14 +49,6 @@ class Filesystem { */ private static ?FilenameValidator $validator = null; - /** - * Memoized results of normalizePath(), keyed by input path + flags - * - * @psalm-suppress ImpureStaticProperty This class has a reset method - * @var CappedMemoryCache - */ - private static ?CappedMemoryCache $normalizedPathCache = null; - /** * @psalm-suppress ImpureStaticProperty This class has a reset method */ @@ -638,17 +630,6 @@ public static function normalizePath($path, $stripTrailingSlash = true, $isAbsol return '/'; } - // paths repeat heavily within a request (same file touched by permission - // checks, cache lookups, hooks, ...), so memoize the result - $cacheKey = $keepUnicode ? "1\0$path" : "0\0$path"; - $cacheKey = ($stripTrailingSlash ? "1\0" : "0\0") . $cacheKey; - if (self::$normalizedPathCache === null) { - self::$normalizedPathCache = new CappedMemoryCache(); - } - if (isset(self::$normalizedPathCache[$cacheKey])) { - return self::$normalizedPathCache[$cacheKey]; - } - // normalize unicode if possible, skipping the ICU normalizer entirely for // plain ASCII paths, which are already in normalization form C if (!$keepUnicode && preg_match('/[\x80-\xff]/', $path)) { @@ -690,8 +671,6 @@ public static function normalizePath($path, $stripTrailingSlash = true, $isAbsol $path = rtrim($path, '/'); } - self::$normalizedPathCache[$cacheKey] = $path; - return $path; } @@ -767,6 +746,5 @@ public static function reset(): void { self::$loaded = false; self::$mounts = null; self::$validator = null; - self::$normalizedPathCache = new CappedMemoryCache(); } }