diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..c15ed68 --- /dev/null +++ b/PR.md @@ -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`. diff --git a/src/TemporaryDirectory.php b/src/TemporaryDirectory.php index c65d938..3088dfd 100644 --- a/src/TemporaryDirectory.php +++ b/src/TemporaryDirectory.php @@ -163,7 +163,13 @@ class TemporaryDirectory 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 diff --git a/tests/TemporaryDirectoryTest.php b/tests/TemporaryDirectoryTest.php index 88035fb..4ff79fa 100644 --- a/tests/TemporaryDirectoryTest.php +++ b/tests/TemporaryDirectoryTest.php @@ -173,6 +173,25 @@ class TemporaryDirectoryTest extends TestCase $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 */ public function it_can_create_a_multiple_subdirectories_in_the_temporary_directory() {