Simplify isValidPath and add unit tests

The check for invalid paths is actually over-complicated and performed twice resulting in a performance penalty. Additionally, I decided to add unit-tests to that function.

Part of https://github.com/owncloud/core/issues/13221

diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php
index ed2be59c092e6ed8c046a959caf242a5af9485b8..506813f73f0dda3012ae5312074aa8635a2fc64d 100644 (file)
--- a/lib/private/files/filesystem.php
+++ b/lib/private/files/filesystem.php
@@ -502,7 +502,7 @@ class Filesystem {
                if (!$path || $path[0] !== '/') {
                        $path = '/' . $path;
                }
-               if (strstr($path, '/../') || strrchr($path, '/') === '/..') {
+               if (strpos($path, '/../') !== FALSE || strrchr($path, '/') === '/..') {
                        return false;
                }
                return true;

diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php
index 1b84db0fc0da95196e73ddcf7550a4cc1b41c799..888690adb0ee1cfd56d669d96e1f8bf4c04feb5b 100644 (file)
--- a/tests/lib/files/filesystem.php
+++ b/tests/lib/files/filesystem.php
@@ -154,6 +154,39 @@ class Filesystem extends \Test\TestCase {
                $this->assertEquals($expected, \OC\Files\Filesystem::normalizePath($path, $stripTrailingSlash));
        }
 
+       public function isValidPathData() {
+               return array(
+                       array('/', true),
+                       array('/path', true),
+                       array('/foo/bar', true),
+                       array('/foo//bar/', true),
+                       array('/foo////bar', true),
+                       array('/foo//\///bar', true),
+                       array('/foo/bar/.', true),
+                       array('/foo/bar/./', true),
+                       array('/foo/bar/./.', true),
+                       array('/foo/bar/././', true),
+                       array('/foo/bar/././..bar', true),
+                       array('/foo/bar/././..bar/a', true),
+                       array('/foo/bar/././..', false),
+                       array('/foo/bar/././../', false),
+                       array('/foo/bar/.././', false),
+                       array('/foo/bar/../../', false),
+                       array('/foo/bar/../..\\', false),
+                       array('..', false),
+                       array('../', false),
+                       array('../foo/bar', false),
+                       array('..\foo/bar', false),
+               );
+       }
+
+       /**
+        * @dataProvider isValidPathData
+        */
+       public function testIsValidPath($path, $expected) {
+               $this->assertSame($expected, \OC\Files\Filesystem::isValidPath($path));
+       }
+
        public function normalizePathWindowsAbsolutePathData() {
                return array(
                        array('C:/', 'C:\\'),