Merge pull request #62782 from nextcloud/backport/62594/stable34

[stable34] More logging around partfile rename failures

Author: AndyScherzinger

apps/dav/lib/Connector/Sabre/File.php

@@ -321,7 +321,14 @@ public function put($data) {
 					$renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath);
 					$fileExists = $storage->file_exists($internalPath);
 					if ($renameOkay === false || $fileExists === false) {
-						Server::get(LoggerInterface::class)->error('renaming part file to final file failed $renameOkay: ' . ($renameOkay ? 'true' : 'false') . ', $fileExists: ' . ($fileExists ? 'true' : 'false') . ')', ['app' => 'webdav']);
+						Server::get(LoggerInterface::class)
+							->error('renaming part file to final file failed $renameOkay: ' . ($renameOkay ? 'true' : 'false') . ', $fileExists: ' . ($fileExists ? 'true' : 'false') . ')', [
+								'app' => 'webdav',
+								'source_storage' => $partStorage->getId(),
+								'target_storage' => $storage->getId(),
+								'source_internal_path' => $internalPartPath,
+								'target_internal_path' => $internalPath,
+							]);
 						throw new Exception($this->l10n->t('Could not rename part file to final file'));
 					}
 				} catch (ForbiddenException $ex) {

lib/private/Files/Storage/Local.php

@@ -354,18 +354,20 @@ public function rename(string $source, string $target): bool {
 		$srcParent = dirname($source);
 		$dstParent = dirname($target);
 
+		$logger = Server::get(LoggerInterface::class);
+
 		if (!$this->isUpdatable($srcParent)) {
-			Server::get(LoggerInterface::class)->error('unable to rename, source directory is not writable : ' . $srcParent, ['app' => 'core']);
+			$logger->error('unable to rename, source directory is not writable : ' . $srcParent, ['app' => 'core']);
 			return false;
 		}
 
 		if (!$this->isUpdatable($dstParent)) {
-			Server::get(LoggerInterface::class)->error('unable to rename, destination directory is not writable : ' . $dstParent, ['app' => 'core']);
+			$logger->error('unable to rename, destination directory is not writable : ' . $dstParent, ['app' => 'core']);
 			return false;
 		}
 
 		if (!$this->file_exists($source)) {
-			Server::get(LoggerInterface::class)->error('unable to rename, file does not exists : ' . $source, ['app' => 'core']);
+			$logger->error('unable to rename, file does not exists : ' . $source, ['app' => 'core']);
 			return false;
 		}
 
@@ -377,20 +379,43 @@ public function rename(string $source, string $target): bool {
 			}
 		}
 
+		$absoluteSource = $this->getSourcePath($source);
+		$absoluteTarget = $this->getSourcePath($target);
+
 		if ($this->is_dir($source)) {
-			$this->checkTreeForForbiddenItems($this->getSourcePath($source));
+			$this->checkTreeForForbiddenItems($absoluteSource);
 		}
 
-		if (@rename($this->getSourcePath($source), $this->getSourcePath($target))) {
+		if (@rename($absoluteSource, $absoluteTarget)) {
 			if ($this->caseInsensitive) {
 				if (mb_strtolower($target) === mb_strtolower($source) && !$this->file_exists($target)) {
 					return false;
 				}
 			}
 			return true;
+		} else {
+			$logger->error('failed to rename ' . $absoluteSource . ' to ' . $absoluteTarget . ', trying copy+delete fallback instead', [
+				'app' => 'core',
+				'last_error' => error_get_last(),
+			]);
 		}
 
-		return $this->copy($source, $target) && $this->unlink($source);
+		if (!$this->copy($source, $target)) {
+			$logger->error('failed to copy ' . $absoluteSource . ' to ' . $absoluteTarget . ' as part of rename fallback', [
+				'app' => 'core',
+				'last_error' => error_get_last(),
+			]);
+			return false;
+		}
+
+		if (!$this->unlink($source)) {
+			$logger->error('failed to delete ' . $absoluteSource . ' as part of rename fallback', [
+				'app' => 'core',
+				'last_error' => error_get_last(),
+			]);
+			return false;
+		}
+		return true;
 	}
 
 	#[\Override]