diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index ddf324ce85..9afa636768 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/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); diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 1b1f43df9e..929cd1b0be 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/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; + } + } diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 1c9ebdd09b..e0dea49c49 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/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 @@ -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')); + } }