Skip to content

Commit 9ab6cc2

Browse files
committed
fix: add more logging around failed local renames
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 532348c commit 9ab6cc2

1 file changed

Lines changed: 31 additions & 6 deletions

File tree

lib/private/Files/Storage/Local.php

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,18 +355,20 @@ public function rename(string $source, string $target): bool {
355355
$srcParent = dirname($source);
356356
$dstParent = dirname($target);
357357

358+
$logger = Server::get(LoggerInterface::class);
359+
358360
if (!$this->isUpdatable($srcParent)) {
359-
Server::get(LoggerInterface::class)->error('unable to rename, source directory is not writable : ' . $srcParent, ['app' => 'core']);
361+
$logger->error('unable to rename, source directory is not writable : ' . $srcParent, ['app' => 'core']);
360362
return false;
361363
}
362364

363365
if (!$this->isUpdatable($dstParent)) {
364-
Server::get(LoggerInterface::class)->error('unable to rename, destination directory is not writable : ' . $dstParent, ['app' => 'core']);
366+
$logger->error('unable to rename, destination directory is not writable : ' . $dstParent, ['app' => 'core']);
365367
return false;
366368
}
367369

368370
if (!$this->file_exists($source)) {
369-
Server::get(LoggerInterface::class)->error('unable to rename, file does not exists : ' . $source, ['app' => 'core']);
371+
$logger->error('unable to rename, file does not exists : ' . $source, ['app' => 'core']);
370372
return false;
371373
}
372374

@@ -378,20 +380,43 @@ public function rename(string $source, string $target): bool {
378380
}
379381
}
380382

383+
$absoluteSource = $this->getSourcePath($source);
384+
$absoluteTarget = $this->getSourcePath($target);
385+
381386
if ($this->is_dir($source)) {
382-
$this->checkTreeForForbiddenItems($this->getSourcePath($source));
387+
$this->checkTreeForForbiddenItems($absoluteSource);
383388
}
384389

385-
if (@rename($this->getSourcePath($source), $this->getSourcePath($target))) {
390+
if (@rename($absoluteSource, $absoluteTarget)) {
386391
if ($this->caseInsensitive) {
387392
if (mb_strtolower($target) === mb_strtolower($source) && !$this->file_exists($target)) {
388393
return false;
389394
}
390395
}
391396
return true;
397+
} else {
398+
$logger->error('failed to rename ' . $absoluteSource . ' to ' . $absoluteTarget . ', trying copy+delete fallback instead', [
399+
'app' => 'core',
400+
'last_error' => error_get_last(),
401+
]);
392402
}
393403

394-
return $this->copy($source, $target) && $this->unlink($source);
404+
if (!$this->copy($source, $target)) {
405+
$logger->error('failed to copy ' . $absoluteSource . ' to ' . $absoluteTarget . ' as part of rename fallback', [
406+
'app' => 'core',
407+
'last_error' => error_get_last(),
408+
]);
409+
return false;
410+
}
411+
412+
if (!$this->unlink($source)) {
413+
$logger->error('failed to delete ' . $absoluteSource . ' as part of rename fallback', [
414+
'app' => 'core',
415+
'last_error' => error_get_last(),
416+
]);
417+
return false;
418+
}
419+
return true;
395420
}
396421

397422
#[\Override]

0 commit comments

Comments
 (0)