Skip to content

Commit ecd4a6b

Browse files
committed
fix: resolve race condition in get_dir_file_info
This adds a null check for get_file_info() to gracefully ignore files deleted mid-scan, fixing random test failures in the Commands component. Also includes a unit test for this behavior.
1 parent 8c164d0 commit ecd4a6b

2 files changed

Lines changed: 47 additions & 20 deletions

File tree

system/Helpers/filesystem_helper.php

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ function directory_mirror(string $originDir, string $targetDir, bool $overwrite
8585
$dirLen = strlen($originDir);
8686

8787
/** @var SplFileInfo $file */
88-
foreach (new RecursiveIteratorIterator(
89-
new RecursiveDirectoryIterator($originDir, FilesystemIterator::SKIP_DOTS),
90-
RecursiveIteratorIterator::SELF_FIRST,
91-
) as $file) {
88+
foreach (
89+
new RecursiveIteratorIterator(
90+
new RecursiveDirectoryIterator($originDir, FilesystemIterator::SKIP_DOTS),
91+
RecursiveIteratorIterator::SELF_FIRST,
92+
) as $file
93+
) {
9294
$origin = $file->getPathname();
9395
$target = $targetDir . substr($origin, $dirLen);
9496

@@ -159,10 +161,12 @@ function delete_files(string $path, bool $delDir = false, bool $htdocs = false,
159161
$path = rtrim($path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR;
160162

161163
try {
162-
foreach (new RecursiveIteratorIterator(
163-
new RecursiveDirectoryIterator($path, RecursiveDirectoryIterator::SKIP_DOTS),
164-
RecursiveIteratorIterator::CHILD_FIRST,
165-
) as $object) {
164+
foreach (
165+
new RecursiveIteratorIterator(
166+
new RecursiveDirectoryIterator($path, RecursiveDirectoryIterator::SKIP_DOTS),
167+
RecursiveIteratorIterator::CHILD_FIRST,
168+
) as $object
169+
) {
166170
$filename = $object->getFilename();
167171
if (! $hidden && $filename[0] === '.') {
168172
continue;
@@ -212,10 +216,12 @@ function get_filenames(
212216
$sourceDir = rtrim($sourceDir, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR;
213217

214218
try {
215-
foreach (new RecursiveIteratorIterator(
216-
new RecursiveDirectoryIterator($sourceDir, RecursiveDirectoryIterator::SKIP_DOTS | FilesystemIterator::FOLLOW_SYMLINKS),
217-
RecursiveIteratorIterator::SELF_FIRST,
218-
) as $name => $object) {
219+
foreach (
220+
new RecursiveIteratorIterator(
221+
new RecursiveDirectoryIterator($sourceDir, RecursiveDirectoryIterator::SKIP_DOTS | FilesystemIterator::FOLLOW_SYMLINKS),
222+
RecursiveIteratorIterator::SELF_FIRST,
223+
) as $name => $object
224+
) {
219225
$basename = pathinfo($name, PATHINFO_BASENAME);
220226
if (! $hidden && $basename[0] === '.') {
221227
continue;
@@ -281,8 +287,10 @@ function get_dir_file_info(string $sourceDir, bool $topLevelOnly = true, bool $r
281287
if (is_dir($sourceDir . $file) && $file[0] !== '.' && $topLevelOnly === false) {
282288
get_dir_file_info($sourceDir . $file . DIRECTORY_SEPARATOR, $topLevelOnly, true);
283289
} elseif ($file[0] !== '.') {
284-
$fileData[$file] = get_file_info($sourceDir . $file);
285-
$fileData[$file]['relative_path'] = $relativePath;
290+
if (($info = get_file_info($sourceDir . $file)) !== null) {
291+
$fileData[$file] = $info;
292+
$fileData[$file]['relative_path'] = $relativePath;
293+
}
286294
}
287295
}
288296

@@ -401,18 +409,18 @@ function symbolic_permissions(int $perms): string
401409

402410
// Owner
403411
$symbolic .= ((($perms & 0x0100) !== 0) ? 'r' : '-')
404-
. ((($perms & 0x0080) !== 0) ? 'w' : '-')
405-
. ((($perms & 0x0040) !== 0) ? ((($perms & 0x0800) !== 0) ? 's' : 'x') : ((($perms & 0x0800) !== 0) ? 'S' : '-'));
412+
. ((($perms & 0x0080) !== 0) ? 'w' : '-')
413+
. ((($perms & 0x0040) !== 0) ? ((($perms & 0x0800) !== 0) ? 's' : 'x') : ((($perms & 0x0800) !== 0) ? 'S' : '-'));
406414

407415
// Group
408416
$symbolic .= ((($perms & 0x0020) !== 0) ? 'r' : '-')
409-
. ((($perms & 0x0010) !== 0) ? 'w' : '-')
410-
. ((($perms & 0x0008) !== 0) ? ((($perms & 0x0400) !== 0) ? 's' : 'x') : ((($perms & 0x0400) !== 0) ? 'S' : '-'));
417+
. ((($perms & 0x0010) !== 0) ? 'w' : '-')
418+
. ((($perms & 0x0008) !== 0) ? ((($perms & 0x0400) !== 0) ? 's' : 'x') : ((($perms & 0x0400) !== 0) ? 'S' : '-'));
411419

412420
// World
413421
$symbolic .= ((($perms & 0x0004) !== 0) ? 'r' : '-')
414-
. ((($perms & 0x0002) !== 0) ? 'w' : '-')
415-
. ((($perms & 0x0001) !== 0) ? ((($perms & 0x0200) !== 0) ? 't' : 'x') : ((($perms & 0x0200) !== 0) ? 'T' : '-'));
422+
. ((($perms & 0x0002) !== 0) ? 'w' : '-')
423+
. ((($perms & 0x0001) !== 0) ? ((($perms & 0x0200) !== 0) ? 't' : 'x') : ((($perms & 0x0200) !== 0) ? 'T' : '-'));
416424

417425
return $symbolic;
418426
}

tests/system/Helpers/FilesystemHelperTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,25 @@ public function testGetDirFileInfoFailure(): void
477477
$this->assertSame($expected, get_dir_file_info(SUPPORTPATH . 'Files#baker'));
478478
}
479479

480+
public function testGetDirFileInfoIgnoresDirectoriesWhenTopLevelOnlyIsTrue(): void
481+
{
482+
$dir = WRITEPATH . 'test_get_dir_file_info';
483+
mkdir($dir . '/subdir', 0777, true);
484+
file_put_contents($dir . '/file.txt', 'test');
485+
486+
$result = get_dir_file_info($dir, true);
487+
488+
// Clean up
489+
unlink($dir . '/file.txt');
490+
rmdir($dir . '/subdir');
491+
rmdir($dir);
492+
493+
// 'subdir' is a directory. When $topLevelOnly is true, get_dir_file_info passes it to get_file_info,
494+
// which returns null because is_file() is false. The function should ignore it completely.
495+
$this->assertArrayHasKey('file.txt', $result);
496+
$this->assertArrayNotHasKey('subdir', $result);
497+
}
498+
480499
public function testGetFileInfo(): void
481500
{
482501
$file = SUPPORTPATH . 'Files/baker/banana.php';

0 commit comments

Comments
 (0)