From 380233044c32c28c014977ebb2b20106a77f5f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 6 Aug 2020 11:50:31 +0200 Subject: [PATCH 1/3] Delete chunks if the move on an upload failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Upload/ChunkingPlugin.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php index 421a2dd519ac9..f3847d1ee1b43 100644 --- a/apps/dav/lib/Upload/ChunkingPlugin.php +++ b/apps/dav/lib/Upload/ChunkingPlugin.php @@ -68,13 +68,16 @@ 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; - } - // 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]); From e9bd87af96bc2b85f7495e164f68da8ccf02b629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 12 Aug 2020 08:18:35 +0200 Subject: [PATCH 2/3] Adjust chunking test for non-existing target node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/tests/unit/Upload/ChunkingPluginTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php index 3951d1f1795e8..102eb727c3db8 100644 --- a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php +++ b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php @@ -107,7 +107,7 @@ public function testBeforeMoveFutureFileSkipNonExisting() { ->with('OC-Total-Length') ->willReturn(4); - $this->assertNull($this->plugin->beforeMove('source', 'target')); + $this->assertFalse($this->plugin->beforeMove('source', 'target')); } public function testBeforeMoveFutureFileMoveIt() { From 4bc8601eb27992b0727428a9e8739ecd0a1abb3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 13 Aug 2020 15:26:42 +0200 Subject: [PATCH 3/3] Return proper status when file didn't exist before MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/lib/Upload/ChunkingPlugin.php | 4 +++- apps/dav/tests/unit/Upload/ChunkingPluginTest.php | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php index f3847d1ee1b43..e2244a5782682 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,6 +69,7 @@ function beforeMove($sourcePath, $destination) { * @return bool|void false to stop handling, void to skip this handler */ public function performMove($path, $destination) { + $fileExists = $this->server->tree->nodeExists($destination); // do a move manually, skipping Sabre's default "delete" for existing nodes try { $this->server->tree->move($path, $destination); @@ -86,7 +88,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 index 102eb727c3db8..3dfc5d08314c1 100644 --- a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php +++ b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php @@ -100,8 +100,12 @@ public function testBeforeMoveFutureFileSkipNonExisting() { ->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')