Merge pull request #4524 from nextcloud/downstream-27508

Keep file id on move

4 files changed, 187 insertions, 106 deletions

diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php
index 25cca40a889..435f47ee561 100644
--- a/apps/dav/lib/Connector/Sabre/Directory.php
+++ b/apps/dav/lib/Connector/Sabre/Directory.php
@@ -34,12 +34,20 @@ use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
 use OCA\DAV\Connector\Sabre\Exception\FileLocked;
 use OCP\Files\ForbiddenException;
+use OCP\Files\InvalidPathException;
+use OCP\Files\StorageNotAvailableException;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
 use Sabre\DAV\Exception\Locked;
+use Sabre\DAV\Exception\ServiceUnavailable;
+use Sabre\DAV\INode;
+use Sabre\DAV\Exception\BadRequest;
+use OC\Files\Mount\MoveableMount;
+use Sabre\DAV\IFile;
+use Sabre\DAV\Exception\NotFound;
 
 class Directory extends \OCA\DAV\Connector\Sabre\Node
-	implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota {
+	implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota, \Sabre\DAV\IMoveTarget {
 
 	/**
 	 * Cached directory content
@@ -113,9 +121,9 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 			if (isset($_SERVER['HTTP_OC_CHUNKED'])) {
 
 				// exit if we can't create a new file and we don't updatable existing file
-				$info = \OC_FileChunking::decodeName($name);
+				$chunkInfo = \OC_FileChunking::decodeName($name);
 				if (!$this->fileView->isCreatable($this->path) &&
-					!$this->fileView->isUpdatable($this->path . '/' . $info['name'])
+					!$this->fileView->isUpdatable($this->path . '/' . $chunkInfo['name'])
 				) {
 					throw new \Sabre\DAV\Exception\Forbidden();
 				}
@@ -130,14 +138,18 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 			$this->fileView->verifyPath($this->path, $name);
 
 			$path = $this->fileView->getAbsolutePath($this->path) . '/' . $name;
-			// using a dummy FileInfo is acceptable here since it will be refreshed after the put is complete
-			$info = new \OC\Files\FileInfo($path, null, null, array(), null);
+			// in case the file already exists/overwriting
+			$info = $this->fileView->getFileInfo($this->path . '/' . $name);
+			if (!$info) {
+				// use a dummy FileInfo which is acceptable here since it will be refreshed after the put is complete
+				$info = new \OC\Files\FileInfo($path, null, null, [], null);
+			}
 			$node = new \OCA\DAV\Connector\Sabre\File($this->fileView, $info);
 			$node->acquireLock(ILockingProvider::LOCK_SHARED);
 			return $node->put($data);
 		} catch (\OCP\Files\StorageNotAvailableException $e) {
 			throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
-		} catch (\OCP\Files\InvalidPathException $ex) {
+		} catch (InvalidPathException $ex) {
 			throw new InvalidPath($ex->getMessage());
 		} catch (ForbiddenException $ex) {
 			throw new Forbidden($ex->getMessage(), $ex->getRetry());
@@ -168,7 +180,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 			}
 		} catch (\OCP\Files\StorageNotAvailableException $e) {
 			throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
-		} catch (\OCP\Files\InvalidPathException $ex) {
+		} catch (InvalidPathException $ex) {
 			throw new InvalidPath($ex->getMessage());
 		} catch (ForbiddenException $ex) {
 			throw new Forbidden($ex->getMessage(), $ex->getRetry());
@@ -188,6 +200,11 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 	 * @throws \Sabre\DAV\Exception\ServiceUnavailable
 	 */
 	public function getChild($name, $info = null) {
+		if (!$this->info->isReadable()) {
+			// avoid detecting files through this way
+			throw new NotFound();
+		}
+
 		$path = $this->path . '/' . $name;
 		if (is_null($info)) {
 			try {
@@ -195,7 +212,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 				$info = $this->fileView->getFileInfo($path);
 			} catch (\OCP\Files\StorageNotAvailableException $e) {
 				throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
-			} catch (\OCP\Files\InvalidPathException $ex) {
+			} catch (InvalidPathException $ex) {
 				throw new InvalidPath($ex->getMessage());
 			} catch (ForbiddenException $e) {
 				throw new \Sabre\DAV\Exception\Forbidden();
@@ -221,12 +238,19 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 	 * Returns an array with all the child nodes
 	 *
 	 * @return \Sabre\DAV\INode[]
+	 * @throws \Sabre\DAV\Exception\Locked
+	 * @throws \OCA\DAV\Connector\Sabre\Exception\Forbidden
 	 */
 	public function getChildren() {
 		if (!is_null($this->dirContent)) {
 			return $this->dirContent;
 		}
 		try {
+			if (!$this->info->isReadable()) {
+				// return 403 instead of 404 because a 404 would make
+				// the caller believe that the collection itself does not exist
+				throw new Forbidden('No read permissions');
+			}
 			$folderContent = $this->fileView->getDirectoryContent($this->path);
 		} catch (LockedException $e) {
 			throw new Locked();
@@ -311,4 +335,109 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
 		}
 	}
 
+	/**
+	 * Moves a node into this collection.
+	 *
+	 * It is up to the implementors to:
+	 *   1. Create the new resource.
+	 *   2. Remove the old resource.
+	 *   3. Transfer any properties or other data.
+	 *
+	 * Generally you should make very sure that your collection can easily move
+	 * the move.
+	 *
+	 * If you don't, just return false, which will trigger sabre/dav to handle
+	 * the move itself. If you return true from this function, the assumption
+	 * is that the move was successful.
+	 *
+	 * @param string $targetName New local file/collection name.
+	 * @param string $fullSourcePath Full path to source node
+	 * @param INode $sourceNode Source node itself
+	 * @return bool
+	 * @throws BadRequest
+	 * @throws ServiceUnavailable
+	 * @throws Forbidden
+	 * @throws FileLocked
+	 * @throws \Sabre\DAV\Exception\Forbidden
+	 */
+	public function moveInto($targetName, $fullSourcePath, INode $sourceNode) {
+		if (!$sourceNode instanceof Node) {
+			// it's a file of another kind, like FutureFile
+			if ($sourceNode instanceof IFile) {
+				// fallback to default copy+delete handling
+				return false;
+			}
+			throw new BadRequest('Incompatible node types');
+		}
+
+		if (!$this->fileView) {
+			throw new ServiceUnavailable('filesystem not setup');
+		}
+
+		$destinationPath = $this->getPath() . '/' . $targetName;
+
+
+		$targetNodeExists = $this->childExists($targetName);
+
+		// at getNodeForPath we also check the path for isForbiddenFileOrDir
+		// with that we have covered both source and destination
+		if ($sourceNode instanceof Directory && $targetNodeExists) {
+			throw new \Sabre\DAV\Exception\Forbidden('Could not copy directory ' . $sourceNode->getName() . ', target exists');
+		}
+
+		list($sourceDir,) = \Sabre\HTTP\URLUtil::splitPath($sourceNode->getPath());
+		$destinationDir = $this->getPath();
+
+		$sourcePath = $sourceNode->getPath();
+
+		$isMovableMount = false;
+		$sourceMount = \OC::$server->getMountManager()->find($this->fileView->getAbsolutePath($sourcePath));
+		$internalPath = $sourceMount->getInternalPath($this->fileView->getAbsolutePath($sourcePath));
+		if ($sourceMount instanceof MoveableMount && $internalPath === '') {
+			$isMovableMount = true;
+		}
+
+		try {
+			$sameFolder = ($sourceDir === $destinationDir);
+			// if we're overwriting or same folder
+			if ($targetNodeExists || $sameFolder) {
+				// note that renaming a share mount point is always allowed
+				if (!$this->fileView->isUpdatable($destinationDir) && !$isMovableMount) {
+					throw new \Sabre\DAV\Exception\Forbidden();
+				}
+			} else {
+				if (!$this->fileView->isCreatable($destinationDir)) {
+					throw new \Sabre\DAV\Exception\Forbidden();
+				}
+			}
+
+			if (!$sameFolder) {
+				// moving to a different folder, source will be gone, like a deletion
+				// note that moving a share mount point is always allowed
+				if (!$this->fileView->isDeletable($sourcePath) && !$isMovableMount) {
+					throw new \Sabre\DAV\Exception\Forbidden();
+				}
+			}
+
+			$fileName = basename($destinationPath);
+			try {
+				$this->fileView->verifyPath($destinationDir, $fileName);
+			} catch (InvalidPathException $ex) {
+				throw new InvalidPath($ex->getMessage());
+			}
+
+			$renameOkay = $this->fileView->rename($sourcePath, $destinationPath);
+			if (!$renameOkay) {
+				throw new \Sabre\DAV\Exception\Forbidden('');
+			}
+		} catch (StorageNotAvailableException $e) {
+			throw new ServiceUnavailable($e->getMessage());
+		} catch (ForbiddenException $ex) {
+			throw new Forbidden($ex->getMessage(), $ex->getRetry());
+		} catch (LockedException $e) {
+			throw new FileLocked($e->getMessage(), $e->getCode(), $e);
+		}
+
+		return true;
+	}
 }
diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php
index d0a01ef255b..25c455a1bb7 100644
--- a/apps/dav/lib/Connector/Sabre/File.php
+++ b/apps/dav/lib/Connector/Sabre/File.php
@@ -54,6 +54,7 @@ use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotImplemented;
 use Sabre\DAV\Exception\ServiceUnavailable;
 use Sabre\DAV\IFile;
+use Sabre\DAV\Exception\NotFound;
 
 class File extends Node implements IFile {
 
@@ -307,6 +308,10 @@ class File extends Node implements IFile {
 	public function get() {
 		//throw exception if encryption is disabled but files are still encrypted
 		try {
+			if (!$this->info->isReadable()) {
+				// do a if the file did not exist
+				throw new NotFound();
+			}
 			$res = $this->fileView->fopen(ltrim($this->path, '/'), 'rb');
 			if ($res === false) {
 				throw new ServiceUnavailable("Could not open file");
diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
index 1b1f43df9ea..a4f3f363a5f 100644
--- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
@@ -45,6 +45,7 @@ use \Sabre\HTTP\ResponseInterface;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IConfig;
 use OCP\IRequest;
+use OCA\DAV\Upload\FutureFile;
 
 class FilesPlugin extends ServerPlugin {
 
@@ -177,6 +178,7 @@ class FilesPlugin extends ServerPlugin {
 			}
 		});
 		$this->server->on('beforeMove', [$this, 'checkMove']);
+		$this->server->on('beforeMove', [$this, 'beforeMoveFutureFile']);
 	}
 
 	/**
@@ -284,6 +286,10 @@ class FilesPlugin extends ServerPlugin {
 		$httpRequest = $this->server->httpRequest;
 
 		if ($node instanceof \OCA\DAV\Connector\Sabre\Node) {
+			if (!$node->getFileInfo()->isReadable()) {
+				// avoid detecting files through this means
+				throw new NotFound();
+			}
 
 			$propFind->handle(self::FILEID_PROPERTYNAME, function() use ($node) {
 				return $node->getFileId();
@@ -436,4 +442,43 @@ class FilesPlugin extends ServerPlugin {
 			}
 		}
 	}
+
+	/**
+	 * Move handler for future file.
+	 *
+	 * This overrides the default move behavior to prevent Sabre
+	 * to delete the target file before moving. Because deleting would
+	 * lose the file id and metadata.
+	 *
+	 * @param string $path source path
+	 * @param string $destination destination path
+	 * @return bool|void false to stop handling, void to skip this handler
+	 */
+	public function beforeMoveFutureFile($path, $destination) {
+		$sourceNode = $this->tree->getNodeForPath($path);
+		if (!$sourceNode instanceof FutureFile) {
+			// skip handling as the source is not a chunked FutureFile
+			return;
+		}
+
+		if (!$this->tree->nodeExists($destination)) {
+			// skip and let the default handler do its work
+			return;
+		}
+
+		// do a move manually, skipping Sabre's default "delete" for existing nodes
+		$this->tree->move($path, $destination);
+
+		// trigger all default events (copied from CorePlugin::move)
+		$this->server->emit('afterMove', [$path, $destination]);
+		$this->server->emit('afterUnbind', [$path]);
+		$this->server->emit('afterBind', [$destination]);
+
+		$response = $this->server->httpResponse;
+		$response->setHeader('Content-Length', '0');
+		$response->setStatus(204);
+
+		return false;
+	}
+
 }
diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php
index 554a7ad86ca..acc6dcc3be3 100644
--- a/apps/dav/lib/Connector/Sabre/ObjectTree.php
+++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php
@@ -185,104 +185,6 @@ class ObjectTree extends \Sabre\DAV\Tree {
 	}
 
 	/**
-	 * Moves a file from one location to another
-	 *
-	 * @param string $sourcePath The path to the file which should be moved
-	 * @param string $destinationPath The full destination path, so not just the destination parent node
-	 * @throws FileLocked
-	 * @throws Forbidden
-	 * @throws InvalidPath
-	 * @throws \Sabre\DAV\Exception\Forbidden
-	 * @throws \Sabre\DAV\Exception\Locked
-	 * @throws \Sabre\DAV\Exception\NotFound
-	 * @throws \Sabre\DAV\Exception\ServiceUnavailable
-	 * @return int
-	 */
-	public function move($sourcePath, $destinationPath) {
-		if (!$this->fileView) {
-			throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup');
-		}
-
-		$infoDestination = $this->fileView->getFileInfo(dirname($destinationPath));
-		if (dirname($destinationPath) === dirname($sourcePath)) {
-			$sourcePermission = $infoDestination && $infoDestination->isUpdateable();
-			$destinationPermission = $sourcePermission;
-		} else {
-			$infoSource = $this->fileView->getFileInfo($sourcePath);
-			if ($this->fileView->file_exists($destinationPath)) {
-				$destinationPermission = $infoDestination && $infoDestination->isUpdateable();
-			} else {
-				$destinationPermission = $infoDestination && $infoDestination->isCreatable();
-			}
-			$sourcePermission =  $infoSource && $infoSource->isDeletable();
-		}
-
-		if (!$destinationPermission || !$sourcePermission) {
-			throw new Forbidden('No permissions to move object.');
-		}
-
-		$targetNodeExists = $this->nodeExists($destinationPath);
-		$sourceNode = $this->getNodeForPath($sourcePath);
-		if ($sourceNode instanceof \Sabre\DAV\ICollection && $targetNodeExists) {
-			throw new \Sabre\DAV\Exception\Forbidden('Could not copy directory ' . $sourceNode->getName() . ', target exists');
-		}
-		list($sourceDir,) = \Sabre\HTTP\URLUtil::splitPath($sourcePath);
-		list($destinationDir,) = \Sabre\HTTP\URLUtil::splitPath($destinationPath);
-
-		$isMovableMount = false;
-		$sourceMount = $this->mountManager->find($this->fileView->getAbsolutePath($sourcePath));
-		$internalPath = $sourceMount->getInternalPath($this->fileView->getAbsolutePath($sourcePath));
-		if ($sourceMount instanceof MoveableMount && $internalPath === '') {
-			$isMovableMount = true;
-		}
-
-		try {
-			$sameFolder = ($sourceDir === $destinationDir);
-			// if we're overwriting or same folder
-			if ($targetNodeExists || $sameFolder) {
-				// note that renaming a share mount point is always allowed
-				if (!$this->fileView->isUpdatable($destinationDir) && !$isMovableMount) {
-					throw new \Sabre\DAV\Exception\Forbidden();
-				}
-			} else {
-				if (!$this->fileView->isCreatable($destinationDir)) {
-					throw new \Sabre\DAV\Exception\Forbidden();
-				}
-			}
-
-			if (!$sameFolder) {
-				// moving to a different folder, source will be gone, like a deletion
-				// note that moving a share mount point is always allowed
-				if (!$this->fileView->isDeletable($sourcePath) && !$isMovableMount) {
-					throw new \Sabre\DAV\Exception\Forbidden();
-				}
-			}
-
-			$fileName = basename($destinationPath);
-			try {
-				$this->fileView->verifyPath($destinationDir, $fileName);
-			} catch (\OCP\Files\InvalidPathException $ex) {
-				throw new InvalidPath($ex->getMessage());
-			}
-
-			$renameOkay = $this->fileView->rename($sourcePath, $destinationPath);
-			if (!$renameOkay) {
-				throw new \Sabre\DAV\Exception\Forbidden('');
-			}
-		} catch (StorageNotAvailableException $e) {
-			throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage());
-		} catch (ForbiddenException $ex) {
-			throw new Forbidden($ex->getMessage(), $ex->getRetry());
-		} catch (LockedException $e) {
-			throw new FileLocked($e->getMessage(), $e->getCode(), $e);
-		}
-
-		$this->markDirty($sourceDir);
-		$this->markDirty($destinationDir);
-
-	}
-
-	/**
 	 * Copies a file or directory.
 	 *
 	 * This method must work recursively and delete the destination