Skip to content

Commit ceca32a

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 ceca32a

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

lib/private/Files/Storage/Local.php

Lines changed: 26 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,23 @@ public function copy(string $source, string $target): bool {
439440
}
440441
}
441442

443+
/**
444+
* The stale entry can belong to any parent, so naming $sourcePath or dirname() alone
445+
* is not enough, while naming nothing would drop the cache of the whole process.
446+
*/
447+
private function clearRealpathCache(string $sourcePath): void {
448+
$root = rtrim($this->datadir, '/');
449+
$path = $sourcePath;
450+
while (str_starts_with($path, $root . '/')) {
451+
clearstatcache(true, $path);
452+
$parent = dirname($path);
453+
if ($parent === $path) {
454+
return;
455+
}
456+
$path = $parent;
457+
}
458+
}
459+
442460
#[\Override]
443461
public function fopen(string $path, string $mode) {
444462
$sourcePath = $this->getSourcePath($path);
@@ -450,6 +468,14 @@ public function fopen(string $path, string $mode) {
450468
$this->unlink($path);
451469
}
452470
$result = @fopen($sourcePath, $mode);
471+
if ($result === false) {
472+
// fopen() resolves through the realpath cache, so a false can just mean this
473+
// process still has the path cached as a file after another one turned it into
474+
// a directory, for up to realpath_cache_ttl. The retry stays unsuppressed so
475+
// the global error handler reports the reason if it fails for real.
476+
$this->clearRealpathCache($sourcePath);
477+
$result = fopen($sourcePath, $mode);
478+
}
453479
umask($oldMask);
454480
return $result;
455481
}

tests/lib/Files/Storage/LocalTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,51 @@ 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+
* The type change has to happen out of process: PHP drops its own realpath cache
170+
* entry when it is the one calling unlink() and mkdir(), so doing it here would
171+
* leave nothing stale and the test would pass either way.
172+
*/
173+
public static function dataStaleRealpathCache(): array {
174+
return [
175+
// invalidating only the direct parent passes the first and fails the second
176+
'stale direct parent' => ['staleName' => 'folder', 'filePath' => 'folder/a.txt'],
177+
'stale grandparent' => ['staleName' => 'nickname', 'filePath' => 'nickname/folder/a.txt'],
178+
];
179+
}
180+
181+
#[\PHPUnit\Framework\Attributes\DataProvider('dataStaleRealpathCache')]
182+
public function testFopenRecoversFromStaleRealpathCache(string $staleName, string $filePath): void {
183+
if (!function_exists('exec')) {
184+
$this->markTestSkipped('exec() is required to change the path type out of process');
185+
}
186+
187+
$stalePath = $this->tmpDir . $staleName;
188+
$file = $this->tmpDir . $filePath;
189+
190+
// opened only to get the "not a directory" entry into the realpath cache
191+
$this->instance->file_put_contents($staleName, 'its a file');
192+
$handle = $this->instance->fopen($staleName, 'r');
193+
$this->assertIsResource($handle);
194+
fclose($handle);
195+
196+
if ((realpath_cache_get()[$stalePath]['is_dir'] ?? null) !== false) {
197+
$this->markTestSkipped('the realpath cache of this setup does not hold the directory flag');
198+
}
199+
200+
exec(
201+
'rm -f ' . escapeshellarg($stalePath)
202+
. ' && mkdir -p ' . escapeshellarg(dirname($file))
203+
. ' && printf abc > ' . escapeshellarg($file),
204+
$output,
205+
$status
206+
);
207+
$this->assertSame(0, $status, 'failed to replace the file with a directory');
208+
209+
$handle = $this->instance->fopen($filePath, 'r');
210+
$this->assertIsResource($handle, 'fopen() has to recover from the stale realpath cache entry');
211+
$this->assertSame('abc', stream_get_contents($handle));
212+
fclose($handle);
213+
}
167214
}

0 commit comments

Comments
 (0)