From 91ab8118244e2baa0135db4f8c40f8af0bc3dd6b Mon Sep 17 00:00:00 2001
From: Daniel Kesselberg <mail@danielkesselberg.de>
Date: Tue, 14 Apr 2020 11:48:43 +0200
Subject: [PATCH] Verify that destination is not a directory.
Otherwise file_put_contents will fail later.
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
---
apps/dav/lib/Upload/ChunkingPlugin.php | 16 +++++++
.../tests/unit/Upload/ChunkingPluginTest.php | 42 +++++++++++++++++--
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php
index 35487f61429..5a8d762de80 100644
--- a/apps/dav/lib/Upload/ChunkingPlugin.php
+++ b/apps/dav/lib/Upload/ChunkingPlugin.php
@@ -23,7 +23,10 @@
namespace OCA\DAV\Upload;
+use OCA\DAV\Connector\Sabre\Directory;
use Sabre\DAV\Exception\BadRequest;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\INode;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
@@ -45,6 +48,9 @@ class ChunkingPlugin extends ServerPlugin {
/**
* @param string $sourcePath source path
* @param string $destination destination path
+ * @return bool|void
+ * @throws BadRequest
+ * @throws NotFound
*/
public function beforeMove($sourcePath, $destination) {
$this->sourceNode = $this->server->tree->getNodeForPath($sourcePath);
@@ -53,6 +59,16 @@ class ChunkingPlugin extends ServerPlugin {
return;
}
+ try {
+ /** @var INode $destinationNode */
+ $destinationNode = $this->server->tree->getNodeForPath($destination);
+ if ($destinationNode instanceof Directory) {
+ throw new BadRequest("The given destination $destination is a directory.");
+ }
+ } catch (NotFound $e) {
+ // If the destination does not exist yet it's not a directory either ;)
+ }
+
$this->verifySize();
return $this->performMove($sourcePath, $destination);
}
diff --git a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
index abbded089db..3a3fcec66d6 100644
--- a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
+++ b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php
@@ -27,6 +27,7 @@ namespace OCA\DAV\Tests\unit\Upload;
use OCA\DAV\Connector\Sabre\Directory;
use OCA\DAV\Upload\ChunkingPlugin;
use OCA\DAV\Upload\FutureFile;
+use Sabre\DAV\Exception\NotFound;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Test\TestCase;
@@ -87,16 +88,41 @@ class ChunkingPluginTest extends TestCase {
$this->assertNull($this->plugin->beforeMove('source', 'target'));
}
+ public function testBeforeMoveDestinationIsDirectory() {
+ $this->expectException(\Sabre\DAV\Exception\BadRequest::class);
+ $this->expectExceptionMessage('The given destination target is a directory.');
+
+ $sourceNode = $this->createMock(FutureFile::class);
+ $targetNode = $this->createMock(Directory::class);
+
+ $this->tree->expects($this->at(0))
+ ->method('getNodeForPath')
+ ->with('source')
+ ->willReturn($sourceNode);
+ $this->tree->expects($this->at(1))
+ ->method('getNodeForPath')
+ ->with('target')
+ ->willReturn($targetNode);
+ $this->response->expects($this->never())
+ ->method('setStatus');
+
+ $this->assertNull($this->plugin->beforeMove('source', 'target'));
+ }
+
public function testBeforeMoveFutureFileSkipNonExisting() {
$sourceNode = $this->createMock(FutureFile::class);
$sourceNode->expects($this->once())
->method('getSize')
->willReturn(4);
- $this->tree->expects($this->any())
+ $this->tree->expects($this->at(0))
->method('getNodeForPath')
->with('source')
->willReturn($sourceNode);
+ $this->tree->expects($this->at(1))
+ ->method('getNodeForPath')
+ ->with('target')
+ ->willThrowException(new NotFound());
$this->tree->expects($this->any())
->method('nodeExists')
->with('target')
@@ -117,10 +143,14 @@ class ChunkingPluginTest extends TestCase {
->method('getSize')
->willReturn(4);
- $this->tree->expects($this->any())
+ $this->tree->expects($this->at(0))
->method('getNodeForPath')
->with('source')
->willReturn($sourceNode);
+ $this->tree->expects($this->at(1))
+ ->method('getNodeForPath')
+ ->with('target')
+ ->willThrowException(new NotFound());
$this->tree->expects($this->any())
->method('nodeExists')
->with('target')
@@ -143,7 +173,7 @@ class ChunkingPluginTest extends TestCase {
$this->assertFalse($this->plugin->beforeMove('source', 'target'));
}
-
+
public function testBeforeMoveSizeIsWrong() {
$this->expectException(\Sabre\DAV\Exception\BadRequest::class);
$this->expectExceptionMessage('Chunks on server do not sum up to 4 but to 3 bytes');
@@ -153,10 +183,14 @@ class ChunkingPluginTest extends TestCase {
->method('getSize')
->willReturn(3);
- $this->tree->expects($this->any())
+ $this->tree->expects($this->at(0))
->method('getNodeForPath')
->with('source')
->willReturn($sourceNode);
+ $this->tree->expects($this->at(1))
+ ->method('getNodeForPath')
+ ->with('target')
+ ->willThrowException(new NotFound());
$this->request->expects($this->once())
->method('getHeader')
->with('OC-Total-Length')
--
GitLab