Skip to content

Commit deec1b8

Browse files
committed
fix(LocalStorage): Clear realpath cache on fopen failure and re-try access
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
1 parent 7afb439 commit deec1b8

2 files changed

Lines changed: 92 additions & 0 deletions

File tree

lib/private/Files/Storage/Local.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCP\Server;
2525
use OCP\Util;
2626
use Psr\Log\LoggerInterface;
27+
use function fopen;
2728

2829
/**
2930
* for local filestore, we only have to map the paths
@@ -439,6 +440,30 @@ public function copy(string $source, string $target): bool {
439440
}
440441
}
441442

443+
/**
444+
* Invalidate the realpath cache for a path and its parents inside the data directory.
445+
*
446+
* Three narrower variants look equivalent but are not, so do not simplify this
447+
* without running testFopenRecoversFromStaleRealpathCache: naming only $sourcePath
448+
* misses the stale entry, which belongs to a parent; naming only dirname() repairs
449+
* a stale direct parent but not a stale grandparent; and omitting the name discards
450+
* the realpath cache of the entire process, every resolved source file included,
451+
* rather than the handful of entries that can be stale. Parents above the data
452+
* directory cannot change type while the process runs, so the walk stops there.
453+
*/
454+
private function clearRealpathCache(string $sourcePath): void {
455+
$root = rtrim($this->datadir, '/');
456+
$path = $sourcePath;
457+
while (str_starts_with($path, $root . '/')) {
458+
clearstatcache(true, $path);
459+
$parent = dirname($path);
460+
if ($parent === $path) {
461+
return;
462+
}
463+
$path = $parent;
464+
}
465+
}
466+
442467
#[\Override]
443468
public function fopen(string $path, string $mode) {
444469
$sourcePath = $this->getSourcePath($path);
@@ -450,6 +475,17 @@ public function fopen(string $path, string $mode) {
450475
$this->unlink($path);
451476
}
452477
$result = @fopen($sourcePath, $mode);
478+
if ($result === false) {
479+
// A path that was a file can turn into a directory in another process. Until
480+
// realpath_cache_ttl expires, this process still has it cached as a file, and
481+
// because fopen() resolves through that cache, it refuses to look inside and
482+
// fails as if the file did not exist. Dropping the stale entries and opening
483+
// again is what fixes that. The retry deliberately stays unsuppressed, so the
484+
// global error handler reports path and reason when an open really fails. NFS
485+
// clients cache the same thing below PHP, out of reach of clearstatcache().
486+
$this->clearRealpathCache($sourcePath);
487+
$result = fopen($sourcePath, $mode);
488+
}
453489
umask($oldMask);
454490
return $result;
455491
}

tests/lib/Files/Storage/LocalTest.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,60 @@ public function testMoveNestedJail(): void {
164164
$jail3->moveFromStorage($jail2, 'file.txt', 'file.txt');
165165
$this->assertTrue($this->instance->file_exists('target/file.txt'));
166166
}
167+
168+
/**
169+
* A path that changes from file to directory in another process leaves a stale
170+
* "not a directory" entry behind in the realpath cache of this process. Opening a
171+
* file below such a path then fails with ENOENT for up to realpath_cache_ttl
172+
* seconds, even though stat() reports the file as present.
173+
*
174+
* The type change has to happen out of process on purpose: PHP drops its own
175+
* realpath cache entry when it is the one calling unlink() and mkdir(), so doing
176+
* it here would leave nothing stale to recover from and the test would pass either
177+
* way. This is what made the file drop integration suite fail intermittently, one
178+
* php -S worker having cached the path while another one turned it into a folder.
179+
*/
180+
public static function dataStaleRealpathCache(): array {
181+
return [
182+
// Invalidating only the direct parent passes the first case and fails the
183+
// second, so both depths have to stay covered.
184+
'stale direct parent' => ['staleName' => 'folder', 'filePath' => 'folder/a.txt'],
185+
'stale grandparent' => ['staleName' => 'nickname', 'filePath' => 'nickname/folder/a.txt'],
186+
];
187+
}
188+
189+
#[\PHPUnit\Framework\Attributes\DataProvider('dataStaleRealpathCache')]
190+
public function testFopenRecoversFromStaleRealpathCache(string $staleName, string $filePath): void {
191+
if (!function_exists('exec')) {
192+
$this->markTestSkipped('exec() is required to change the path type out of process');
193+
}
194+
195+
$stalePath = $this->tmpDir . $staleName;
196+
$file = $this->tmpDir . $filePath;
197+
198+
// Reading it while it is still a file is what puts the "not a directory" entry
199+
// into the realpath cache, so the handle is opened only for that side effect
200+
$this->instance->file_put_contents($staleName, 'its a file');
201+
$handle = $this->instance->fopen($staleName, 'r');
202+
$this->assertIsResource($handle);
203+
fclose($handle);
204+
205+
if ((realpath_cache_get()[$stalePath]['is_dir'] ?? null) !== false) {
206+
$this->markTestSkipped('the realpath cache of this setup does not hold the directory flag');
207+
}
208+
209+
exec(
210+
'rm -f ' . escapeshellarg($stalePath)
211+
. ' && mkdir -p ' . escapeshellarg(dirname($file))
212+
. ' && printf abc > ' . escapeshellarg($file),
213+
$output,
214+
$status
215+
);
216+
$this->assertSame(0, $status, 'failed to replace the file with a directory');
217+
218+
$handle = $this->instance->fopen($filePath, 'r');
219+
$this->assertIsResource($handle, 'fopen() has to recover from the stale realpath cache entry');
220+
$this->assertSame('abc', stream_get_contents($handle));
221+
fclose($handle);
222+
}
167223
}

0 commit comments

Comments
 (0)