Merge pull request #9986 from nextcloud/dav-upload-no-partfile-lock-13

[13] properly lock the target file on dav upload when not using part files

1 files changed, 24 insertions, 16 deletions

diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php
index 6a467e9eff2..49b3debc386 100644
--- a/apps/dav/lib/Connector/Sabre/File.php
+++ b/apps/dav/lib/Connector/Sabre/File.php
@@ -148,12 +148,21 @@ class File extends Node implements IFile {
 			$this->emitPreHooks($exists);
 		}
 
+		$view = \OC\Files\Filesystem::getView();
+
 		// the part file and target file might be on a different storage in case of a single file storage (e.g. single file share)
 		/** @var \OC\Files\Storage\Storage $partStorage */
 		list($partStorage, $internalPartPath) = $this->fileView->resolvePath($partFilePath);
 		/** @var \OC\Files\Storage\Storage $storage */
 		list($storage, $internalPath) = $this->fileView->resolvePath($this->path);
 		try {
+			if (!$needsPartFile) {
+				if ($view && !$this->emitPreHooks($exists)) {
+					throw new Exception('Could not write to final file, canceled by hook');
+				}
+				$this->changeLock(ILockingProvider::LOCK_EXCLUSIVE);
+			}
+
 			$target = $partStorage->fopen($internalPartPath, 'wb');
 			if ($target === false) {
 				\OCP\Util::writeLog('webdav', '\OC\Files\Filesystem::fopen() failed', \OCP\Util::ERROR);
@@ -189,27 +198,26 @@ class File extends Node implements IFile {
 		}
 
 		try {
-			$view = \OC\Files\Filesystem::getView();
-			$run = ($view && $needsPartFile) ? $this->emitPreHooks($exists) : true;
-
-			try {
-				$this->changeLock(ILockingProvider::LOCK_EXCLUSIVE);
-			} catch (LockedException $e) {
-				if ($needsPartFile) {
+			if ($needsPartFile) {
+				if ($view && !$this->emitPreHooks($exists)) {
 					$partStorage->unlink($internalPartPath);
+					throw new Exception('Could not rename part file to final file, canceled by hook');
+				}
+				try {
+					$this->changeLock(ILockingProvider::LOCK_EXCLUSIVE);
+				} catch (LockedException $e) {
+					if ($needsPartFile) {
+						$partStorage->unlink($internalPartPath);
+					}
+					throw new FileLocked($e->getMessage(), $e->getCode(), $e);
 				}
-				throw new FileLocked($e->getMessage(), $e->getCode(), $e);
-			}
 
-			if ($needsPartFile) {
 				// rename to correct path
 				try {
-					if ($run) {
-						$renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath);
-						$fileExists = $storage->file_exists($internalPath);
-					}
-					if (!$run || $renameOkay === false || $fileExists === false) {
-						\OCP\Util::writeLog('webdav', 'renaming part file to final file failed ($run: ' . ( $run ? 'true' : 'false' ) . ', $renameOkay: '  . ( $renameOkay ? 'true' : 'false' ) . ', $fileExists: ' . ( $fileExists ? 'true' : 'false' ) . ')', \OCP\Util::ERROR);
+					$renameOkay = $storage->moveFromStorage($partStorage, $internalPartPath, $internalPath);
+					$fileExists = $storage->file_exists($internalPath);
+					if ($renameOkay === false || $fileExists === false) {
+						\OC::$server->getLogger()->error('renaming part file to final file failed ($run: ' . ( $run ? 'true' : 'false' ) . ', $renameOkay: '  . ( $renameOkay ? 'true' : 'false' ) . ', $fileExists: ' . ( $fileExists ? 'true' : 'false' ) . ')', ['app' => 'webdav']);
 						throw new Exception('Could not rename part file to final file');
 					}
 				} catch (ForbiddenException $ex) {