Merge pull request #22128 from nextcloud/bugfix/noid/cleanup-chunks-o…

…n-failure Delete chunks if the move on an upload failed
nextcloud · Aug 13, 2020 · e9f5c7f · e9f5c7f
2 parents ed46115 + 6722246
commit e9f5c7f
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php
@@ -26,6 +26,7 @@
 namespace OCA\DAV\Upload;
 
 use OCA\DAV\Connector\Sabre\Directory;
+use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\INode;
@@ -87,13 +88,17 @@ public function beforeMove($sourcePath, $destination) {
 	 * @return bool|void false to stop handling, void to skip this handler
 	 */
 	public function performMove($path, $destination) {
-		if (!$this->server->tree->nodeExists($destination)) {
-			// skip and let the default handler do its work
-			return;
-		}
-
+		$fileExists = $this->server->tree->nodeExists($destination);
 		// do a move manually, skipping Sabre's default "delete" for existing nodes
-		$this->server->tree->move($path, $destination);
+		try {
+			$this->server->tree->move($path, $destination);
+		} catch (Forbidden $e) {
+			$sourceNode = $this->server->tree->getNodeForPath($path);
+			if ($sourceNode instanceof FutureFile) {
+				$sourceNode->delete();
+			}
+			throw $e;
+		}
 
 		// trigger all default events (copied from CorePlugin::move)
 		$this->server->emit('afterMove', [$path, $destination]);
@@ -102,7 +107,7 @@ public function performMove($path, $destination) {
 
 		$response = $this->server->httpResponse;
 		$response->setHeader('Content-Length', '0');
-		$response->setStatus(204);
+		$response->setStatus($fileExists ? 204 : 201);
 
 		return false;
 	}

diff --git a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
@@ -128,14 +128,18 @@ public function testBeforeMoveFutureFileSkipNonExisting() {
 			->method('nodeExists')
 			->with('target')
 			->willReturn(false);
-		$this->response->expects($this->never())
-			->method('setStatus');
+		$this->response->expects($this->once())
+			->method('setHeader')
+			->with('Content-Length', '0');
+		$this->response->expects($this->once())
+			->method('setStatus')
+			->with(201);
 		$this->request->expects($this->once())
 			->method('getHeader')
 			->with('OC-Total-Length')
 			->willReturn(4);
 
-		$this->assertNull($this->plugin->beforeMove('source', 'target'));
+		$this->assertFalse($this->plugin->beforeMove('source', 'target'));
 	}
 
 	public function testBeforeMoveFutureFileMoveIt() {