Skip to content

Commit

Permalink
Make Folder::inPath() even more strict.
Browse files Browse the repository at this point in the history
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, and throws an
exception in case it is not an absolute path.
  • Loading branch information
ndm2 committed Aug 26, 2016
1 parent fea8417 commit f1b5d55
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 7 deletions.
11 changes: 6 additions & 5 deletions src/Filesystem/Folder.php
Expand Up @@ -16,6 +16,7 @@

use DirectoryIterator;
use Exception;
use InvalidArgumentException;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;

Expand Down Expand Up @@ -418,15 +419,15 @@ public function inCakePath($path = '')
/**
* Returns true if the Folder is in the given path.
*
* @param string $path The absolute path to check that the current `pwd()` resides within. If omitted
* (or empty), the current top level directory is assumed.
* @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.
*/
public function inPath($path = '', $reverse = false)
public function inPath($path, $reverse = false)
{
if (empty($path)) {
$path = realpath(DS);
if (!Folder::isAbsolute($path)) {
throw new InvalidArgumentException('The $path argument is expected to be an absolute path.');
}

$dir = Folder::slashTerm($path);
Expand Down
31 changes: 29 additions & 2 deletions tests/TestCase/Filesystem/FolderTest.php
Expand Up @@ -102,8 +102,9 @@ public function testInPath()
$result = $Base->pwd();
$this->assertEquals($basePath, $result);


// is "/" in "/tests/test_app/"
$result = $Base->inPath('', true);
$result = $Base->inPath(realpath(DS), true);
$this->assertFalse($result, true);

// is "/tests/test_app/" in "/tests/test_app/"
Expand All @@ -128,7 +129,7 @@ public function testInPath()


// is "/tests/test_app/" in "/"
$result = $Base->inPath();
$result = $Base->inPath(realpath(DS));
$this->assertTrue($result);

// is "/tests/test_app/" in "/tests/test_app/"
Expand All @@ -154,6 +155,32 @@ public function testInPath()
$this->assertFalse($result);
}

/**
* Data provider for the testInPathInvalidPathArgument test
*
* @return array
*/
public function inPathInvalidPathArgumentDataProvider()
{
return [
[''],
['relative/path/'],
['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);
}

/**
* test creation of single and multiple paths.
*
Expand Down

0 comments on commit f1b5d55

Please sign in to comment.