fix: only treat the last path segment as a filename in path()
isFilePath() checked the entire path for a dot to decide whether path() was given a file or a directory. Because the path includes the location and name, a dot in a parent segment (for example a location like /tmp/my.app) caused a directory argument to be misclassified as a file, so the requested subdirectory was not created. Inspect only the last path segment instead.
This commit is contained in:
1 parent
539be424ae
commit
53a7d85862
3 files changed
+100
-1
No files matched your search
@@ -0,0 +1,74 @@
|
|||||||
|
# fix: only treat the last path segment as a filename in path()
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`TemporaryDirectory::path()` fails to create a requested subdirectory when the
|
||||||
|
temporary directory's location contains a dot:
|
||||||
|
|
||||||
|
```php
|
||||||
|
$tmp = (new Spatie\TemporaryDirectory\TemporaryDirectory('/tmp/my.app'))
|
||||||
|
->name('run1')
|
||||||
|
->create();
|
||||||
|
|
||||||
|
$path = $tmp->path('abc'); // "abc" is a directory
|
||||||
|
is_dir($path); // false <-- the directory was not created
|
||||||
|
```
|
||||||
|
|
||||||
|
`path('abc')` is documented and tested to create the `abc` subdirectory (see
|
||||||
|
`it_can_create_a_subdirectory_in_the_temporary_directory`), but here it returns a
|
||||||
|
path whose directory does not exist.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`src/TemporaryDirectory.php`. `path()` decides whether the argument points to a
|
||||||
|
file (create only the parent directory) or a directory (create the whole path)
|
||||||
|
via `isFilePath()`:
|
||||||
|
|
||||||
|
```php
|
||||||
|
protected function isFilePath(string $path): bool
|
||||||
|
{
|
||||||
|
return str_contains($path, '.');
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`$path` is the full path (`location + name + argument`), so the check reacts to a
|
||||||
|
dot anywhere in it - including a dot in the location such as `/tmp/my.app`. When
|
||||||
|
that happens, a directory argument like `abc` is misclassified as a file, and
|
||||||
|
`removeFilenameFromPath()` strips the `abc` segment, so only the already existing
|
||||||
|
parent is created and `abc` is skipped.
|
||||||
|
|
||||||
|
## The fix
|
||||||
|
|
||||||
|
Only the last path segment can be a filename, so inspect just that segment:
|
||||||
|
|
||||||
|
```php
|
||||||
|
protected function isFilePath(string $path): bool
|
||||||
|
{
|
||||||
|
$lastSeparator = strrpos($path, DIRECTORY_SEPARATOR);
|
||||||
|
$lastSegment = $lastSeparator === false ? $path : substr($path, $lastSeparator + 1);
|
||||||
|
|
||||||
|
return str_contains($lastSegment, '.');
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
A dot in a parent segment (the location or name) no longer affects the
|
||||||
|
file/directory decision, while a real filename such as `test.txt` in the last
|
||||||
|
segment is still detected.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added `it_can_create_a_subdirectory_when_the_location_contains_a_dot`, which
|
||||||
|
creates a temporary directory under a location containing dots and asserts that
|
||||||
|
`path('abc')` actually creates the `abc` directory.
|
||||||
|
|
||||||
|
## Gate
|
||||||
|
|
||||||
|
```
|
||||||
|
composer install && vendor/bin/phpunit
|
||||||
|
```
|
||||||
|
|
||||||
|
Result: exit code `0` - `OK (23 tests, 37 assertions)`.
|
||||||
|
|
||||||
|
Counterfactual: reverting `isFilePath()` to `str_contains($path, '.')` makes the
|
||||||
|
new test fail - `Failed asserting that directory
|
||||||
|
"/tmp/my.dotted.location/temporary_directory/abc" exists`.
|
||||||
@@ -163,7 +163,13 @@ class TemporaryDirectory
|
|||||||
|
|
||||||
protected function isFilePath(string $path): bool
|
protected function isFilePath(string $path): bool
|
||||||
{
|
{
|
||||||
return str_contains($path, '.');
|
// Only the last path segment can be a filename. Inspecting the whole
|
||||||
|
// path would misclassify a directory as a file whenever a parent
|
||||||
|
// segment (for example the location) contains a dot.
|
||||||
|
$lastSeparator = strrpos($path, DIRECTORY_SEPARATOR);
|
||||||
|
$lastSegment = $lastSeparator === false ? $path : substr($path, $lastSeparator + 1);
|
||||||
|
|
||||||
|
return str_contains($lastSegment, '.');
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function deleteDirectory(string $path): bool
|
protected function deleteDirectory(string $path): bool
|
||||||
|
|||||||
@@ -173,6 +173,25 @@ class TemporaryDirectoryTest extends TestCase
|
|||||||
$this->assertDirectoryExists("{$this->temporaryDirectoryFullPath}/{$subdirectory}");
|
$this->assertDirectoryExists("{$this->temporaryDirectoryFullPath}/{$subdirectory}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** @test */
|
||||||
|
public function it_can_create_a_subdirectory_when_the_location_contains_a_dot()
|
||||||
|
{
|
||||||
|
$dottedLocation = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.'my.dotted.location';
|
||||||
|
$this->deleteDirectory($dottedLocation);
|
||||||
|
|
||||||
|
$temporaryDirectory = (new TemporaryDirectory($dottedLocation))
|
||||||
|
->name($this->temporaryDirectory)
|
||||||
|
->create();
|
||||||
|
|
||||||
|
$subdirectory = 'abc';
|
||||||
|
$subdirectoryPath = $temporaryDirectory->path($subdirectory);
|
||||||
|
|
||||||
|
$this->assertDirectoryExists($subdirectoryPath);
|
||||||
|
|
||||||
|
$temporaryDirectory->delete();
|
||||||
|
$this->deleteDirectory($dottedLocation);
|
||||||
|
}
|
||||||
|
|
||||||
/** @test */
|
/** @test */
|
||||||
public function it_can_create_a_multiple_subdirectories_in_the_temporary_directory()
|
public function it_can_create_a_multiple_subdirectories_in_the_temporary_directory()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in new issue
Block a user