Merge pull request #22241 from nextcloud/backport/22128/stable17

[stable17] Delete chunks if the move on an upload failed

2 files changed, 19 insertions, 10 deletions

diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php
index 421a2dd519a..e2244a57826 100644
--- a/apps/dav/lib/Upload/ChunkingPlugin.php
+++ b/apps/dav/lib/Upload/ChunkingPlugin.php
@@ -22,6 +22,7 @@
 
 namespace OCA\DAV\Upload;
 
+use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
@@ -68,13 +69,17 @@ class ChunkingPlugin extends ServerPlugin {
 	 * @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]);
@@ -83,7 +88,7 @@ class ChunkingPlugin extends ServerPlugin {
 
 		$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
index 3951d1f1795..3dfc5d08314 100644
--- a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
+++ b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
@@ -100,14 +100,18 @@ class ChunkingPluginTest extends TestCase {
 			->method('nodeExists')
 			->with('target')
 			->will($this->returnValue(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() {