From 05615bfd473f1eafa4ec253779568ab044467ceb Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sat, 10 Jan 2015 00:06:30 +0100 Subject: [PATCH] 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 --- lib/private/files/filesystem.php | 2 +- tests/lib/files/filesystem.php | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php index ed2be59c09..506813f73f 100644 --- 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 1b84db0fc0..888690adb0 100644 --- 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:\\'),