Keep file id on move

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

@@ -34,12 +34,20 @@
 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 @@ public function createFile($name, $data = null) {
 			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 @@ public function createFile($name, $data = null) {
 			$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 @@ public function createDirectory($name) {
 			}
 		} 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,14 +200,19 @@ public function createDirectory($name) {
 	 * @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 {
 				$this->fileView->verifyPath($this->path, $name);
 				$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 @@ public function getChild($name, $info = null) {
 	 * 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 @@ public function getQuotaInfo() {
 		}
 	}
 
+	/**
+	 * 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;
+	}
 }

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

@@ -54,6 +54,7 @@
 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 @@ private function emitPostHooks($exists, $path = null) {
 	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");

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

@@ -45,6 +45,7 @@
 use OCP\Files\StorageNotAvailableException;
 use OCP\IConfig;
 use OCP\IRequest;
+use OCA\DAV\Upload\FutureFile;
 
 class FilesPlugin extends ServerPlugin {
 
@@ -177,6 +178,7 @@ public function initialize(\Sabre\DAV\Server $server) {
 			}
 		});
 		$this->server->on('beforeMove', [$this, 'checkMove']);
+		$this->server->on('beforeMove', [$this, 'beforeMoveFutureFile']);
 	}
 
 	/**
@@ -284,6 +286,10 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
 		$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 @@ public function sendFileIdHeader($filePath, \Sabre\DAV\INode $node = null) {
 			}
 		}
 	}
+
+	/**
+	 * 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;
+	}
+
 }