Fix FutureFile MOVE to keep destination node

Sabre usually deletes the target node on MOVE before proceeding with the
actual move operation. This fix prevents this to happen in case the
source node is a FutureFile.

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

@@ -120,9 +120,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();
 				}
@@ -137,8 +137,12 @@ 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);

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']);
 	}
 
 	/**
@@ -436,4 +438,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;
+	}
+
 }

apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php

@@ -32,6 +32,8 @@ use Sabre\DAV\PropPatch;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
 use Test\TestCase;
+use OCA\DAV\Upload\FutureFile;
+use OCA\DAV\Connector\Sabre\Directory;
 
 /**
  * Copyright (c) 2015 Vincent Petry <pvince81@owncloud.com>
@@ -107,6 +109,12 @@ class FilesPluginTest extends TestCase {
 			$this->request,
 			$this->previewManager
 		);
+
+		$response = $this->getMockBuilder(ResponseInterface::class)
+				->disableOriginalConstructor()
+				->getMock();
+		$this->server->httpResponse = $response;
+
 		$this->plugin->initialize($this->server);
 	}
 
@@ -535,4 +543,59 @@ class FilesPluginTest extends TestCase {
 
 		$this->assertEquals("false", $propFind->get(self::HAS_PREVIEW_PROPERTYNAME));
 	}
+
+	public function testBeforeMoveFutureFileSkip() {
+		$node = $this->createMock(Directory::class);
+
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->with('source')
+			->will($this->returnValue($node));
+		$this->server->httpResponse->expects($this->never())
+			->method('setStatus');
+
+		$this->assertNull($this->plugin->beforeMoveFutureFile('source', 'target'));
+	}
+
+	public function testBeforeMoveFutureFileSkipNonExisting() {
+		$sourceNode = $this->createMock(FutureFile::class);
+
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->with('source')
+			->will($this->returnValue($sourceNode));
+		$this->tree->expects($this->any())
+			->method('nodeExists')
+			->with('target')
+			->will($this->returnValue(false));
+		$this->server->httpResponse->expects($this->never())
+			->method('setStatus');
+
+		$this->assertNull($this->plugin->beforeMoveFutureFile('source', 'target'));
+	}
+
+	public function testBeforeMoveFutureFileMoveIt() {
+		$sourceNode = $this->createMock(FutureFile::class);
+
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->with('source')
+			->will($this->returnValue($sourceNode));
+		$this->tree->expects($this->any())
+			->method('nodeExists')
+			->with('target')
+			->will($this->returnValue(true));
+		$this->tree->expects($this->once())
+			->method('move')
+			->with('source', 'target');
+
+		$this->server->httpResponse->expects($this->once())
+			->method('setHeader')
+			->with('Content-Length', '0');
+		$this->server->httpResponse->expects($this->once())
+			->method('setStatus')
+			->with(204);
+
+		$this->assertFalse($this->plugin->beforeMoveFutureFile('source', 'target'));
+	}
 }