From 87d86aaed956edc5feb066eee69253b75bb67ada Mon Sep 17 00:00:00 2001 From: ndm2 Date: Fri, 26 Aug 2016 13:28:02 +0200 Subject: [PATCH] Fix/tighten `Folder::inPath()` checks. The current checks are way too relaxed, and are more like testing for a substring, which makes it easy for invalid paths to slip trough, for example `/foo/var/www` is falsely tested to reside in `/var/www`. Passing an empty path never worked properly, it was triggering a warning, didn't worked on Windows, and the behavior that the current top level directory would be assumed for empty paths wasn't documented. Similar is true for relative paths. While they did match at one point, this was incorrect behavior, and matching actual path fragments seems out of scope for this method. This change makes the `$path` argument required, requires it to be an absolute path, and throws an exception in case a non-absolute path is being passed. --- lib/Cake/Test/Case/Utility/FolderTest.php | 82 +++++++++++++++++++---- lib/Cake/Utility/Folder.php | 17 +++-- 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/lib/Cake/Test/Case/Utility/FolderTest.php b/lib/Cake/Test/Case/Utility/FolderTest.php index 1d53713ae..e417abff9 100644 --- a/lib/Cake/Test/Case/Utility/FolderTest.php +++ b/lib/Cake/Test/Case/Utility/FolderTest.php @@ -109,28 +109,86 @@ class FolderTest extends CakeTestCase { * @return void */ public function testInPath() { - $path = dirname(dirname(__FILE__)); - $inside = dirname($path) . DS; + // "/Test/test_app/" + $basePath = CAKE . 'Test' . DS . 'test_app' . DS; + $Base = new Folder($basePath); - $Folder = new Folder($path); + $result = $Base->pwd(); + $this->assertEquals($basePath, $result); - $result = $Folder->pwd(); - $this->assertEquals($path, $result); + // is "/" in "/Test/test_app/" + $result = $Base->inPath(realpath(DS), true); + $this->assertFalse($result, true); - $result = Folder::isSlashTerm($inside); + // is "/Test/test_app/" in "/Test/test_app/" + $result = $Base->inPath($basePath, true); $this->assertTrue($result); - $result = $Folder->realpath('Test/'); - $this->assertEquals($path . DS . 'Test' . DS, $result); - - $result = $Folder->inPath('Test' . DS); + // is "/Test/test_app" in "/Test/test_app/" + $result = $Base->inPath(mb_substr($basePath, 0, -1), true); $this->assertTrue($result); - $result = $Folder->inPath(DS . 'non-existing' . $inside); + // is "/Test/test_app/sub" in "/Test/test_app/" + $result = $Base->inPath($basePath . 'sub', true); + $this->assertTrue($result); + + // is "/Test" in "/Test/test_app/" + $result = $Base->inPath(dirname($basePath), true); $this->assertFalse($result); - $result = $Folder->inPath($path . DS . 'Model', true); + // is "/Test/other/(...)Test/test_app" in "/Test/test_app/" + $result = $Base->inPath(TMP . 'tests' . DS . 'other' . DS . $basePath, true); + $this->assertFalse($result); + + // is "/Test/test_app/" in "/" + $result = $Base->inPath(realpath(DS)); $this->assertTrue($result); + + // is "/Test/test_app/" in "/Test/test_app/" + $result = $Base->inPath($basePath); + $this->assertTrue($result); + + // is "/Test/test_app/" in "/Test/test_app" + $result = $Base->inPath(mb_substr($basePath, 0, -1)); + $this->assertTrue($result); + + // is "/Test/test_app/" in "/Test" + $result = $Base->inPath(dirname($basePath)); + $this->assertTrue($result); + + // is "/Test/test_app/" in "/Test/test_app/sub" + $result = $Base->inPath($basePath . 'sub'); + $this->assertFalse($result); + + // is "/other/Test/test_app/" in "/Test/test_app/" + $VirtualBase = new Folder(); + $VirtualBase->path = '/other/Test/test_app'; + $result = $VirtualBase->inPath('/Test/test_app/'); + $this->assertFalse($result); + } + +/** + * Data provider for the testInPathInvalidPathArgument test + * + * @return array + */ + public function inPathInvalidPathArgumentDataProvider() { + return array( + array(''), + array('relative/path/'), + array('unknown://stream-wrapper') + ); + } + +/** + * @dataProvider inPathInvalidPathArgumentDataProvider + * @param string $path + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The $path argument is expected to be an absolute path. + */ + public function testInPathInvalidPathArgument($path) { + $Folder = new Folder(); + $Folder->inPath($path); } /** diff --git a/lib/Cake/Utility/Folder.php b/lib/Cake/Utility/Folder.php index 1b59ba155..861a2d637 100644 --- a/lib/Cake/Utility/Folder.php +++ b/lib/Cake/Utility/Folder.php @@ -385,7 +385,7 @@ class Folder { } /** - * Returns true if the File is in a given CakePath. + * Returns true if the Folder is in the given Cake path. * * @param string $path The path to check. * @return bool @@ -399,21 +399,26 @@ class Folder { } /** - * Returns true if the File is in given path. + * Returns true if the Folder is in the given path. * - * @param string $path The path to check that the current pwd() resides with in. - * @param bool $reverse Reverse the search, check that pwd() resides within $path. + * @param string $path The absolute path to check that the current `pwd()` resides within. + * @param bool $reverse Reverse the search, check if the given `$path` resides within the current `pwd()`. * @return bool + * @throws \InvalidArgumentException When the given `$path` argument is not an absolute path. * @link http://book.cakephp.org/2.0/en/core-utility-libraries/file-folder.html#Folder::inPath */ public function inPath($path = '', $reverse = false) { + if (!Folder::isAbsolute($path)) { + throw new InvalidArgumentException(__d('cake_dev', 'The $path argument is expected to be an absolute path.')); + } + $dir = Folder::slashTerm($path); $current = Folder::slashTerm($this->pwd()); if (!$reverse) { - $return = preg_match('/^(.*)' . preg_quote($dir, '/') . '(.*)/', $current); + $return = preg_match('/^' . preg_quote($dir, '/') . '(.*)/', $current); } else { - $return = preg_match('/^(.*)' . preg_quote($current, '/') . '(.*)/', $dir); + $return = preg_match('/^' . preg_quote($current, '/') . '(.*)/', $dir); } return (bool)$return; }