diff --git a/lib/DAV/CorePlugin.php b/lib/DAV/CorePlugin.php index ac9292801b..ce1e01ba94 100644 --- a/lib/DAV/CorePlugin.php +++ b/lib/DAV/CorePlugin.php @@ -589,6 +589,11 @@ public function httpMove(RequestInterface $request, ResponseInterface $response) $moveInfo = $this->server->getCopyAndMoveInfo($request); + // MOVE does only allow "infinity" every other header value is considered invalid + if (Server::DEPTH_INFINITY !== $moveInfo['depth']) { + throw new BadRequest('The HTTP Depth header must only contain "infinity" for MOVE'); + } + if ($moveInfo['destinationExists']) { if (!$this->server->emit('beforeUnbind', [$moveInfo['destination']])) { return false; @@ -645,7 +650,7 @@ public function httpCopy(RequestInterface $request, ResponseInterface $response) if (!$this->server->emit('beforeBind', [$copyInfo['destination']])) { return false; } - if (!$this->server->emit('beforeCopy', [$path, $copyInfo['destination']])) { + if (!$this->server->emit('beforeCopy', [$path, $copyInfo['destination'], $copyInfo['depth']])) { return false; } @@ -656,8 +661,8 @@ public function httpCopy(RequestInterface $request, ResponseInterface $response) $this->server->tree->delete($copyInfo['destination']); } - $this->server->tree->copy($path, $copyInfo['destination']); - $this->server->emit('afterCopy', [$path, $copyInfo['destination']]); + $this->server->tree->copy($path, $copyInfo['destination'], $copyInfo['depth']); + $this->server->emit('afterCopy', [$path, $copyInfo['destination'], $copyInfo['depth']]); $this->server->emit('afterBind', [$copyInfo['destination']]); // If a resource was overwritten we should send a 204, otherwise a 201 diff --git a/lib/DAV/ICopyTarget.php b/lib/DAV/ICopyTarget.php index 47227138a2..57d79a0386 100644 --- a/lib/DAV/ICopyTarget.php +++ b/lib/DAV/ICopyTarget.php @@ -31,8 +31,11 @@ interface ICopyTarget extends ICollection * @param string $targetName new local file/collection name * @param string $sourcePath Full path to source node * @param INode $sourceNode Source node itself + * @param int $depth How many level of children to copy. + * The value can be 'infinity' (Sabre\DAV\Server::DEPTH_INFINITY) or a positive number including zero. + * Zero means to only copy a shallow collection with props but without children. * * @return bool */ - public function copyInto($targetName, $sourcePath, INode $sourceNode); + public function copyInto($targetName, $sourcePath, INode $sourceNode, int $depth); } diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php index 5d37dbff9f..f0acf5abdd 100644 --- a/lib/DAV/Server.php +++ b/lib/DAV/Server.php @@ -580,9 +580,10 @@ public function calculateUri($uri) /** * Returns the HTTP depth header. * - * This method returns the contents of the HTTP depth request header. If the depth header was 'infinity' it will return the Sabre\DAV\Server::DEPTH_INFINITY object + * This method returns the contents of the HTTP depth request header. If the depth header was 'infinity' it will return the Sabre\DAV\Server::DEPTH_INFINITY constant. * It is possible to supply a default depth value, which is used when the depth header has invalid content, or is completely non-existent * + * @param mixed $default default value to use if no header is set or has invalid value * @param mixed $default * * @return int @@ -725,10 +726,18 @@ public function getCopyAndMoveInfo(RequestInterface $request) throw new Exception\BadRequest('The destination header was not supplied'); } $destination = $this->calculateUri($request->getHeader('Destination')); - $overwrite = $request->getHeader('Overwrite'); - if (!$overwrite) { - $overwrite = 'T'; + + // Depth of infinity is valid for MOVE and COPY. If it is not set the RFC requires to act like it was 'infinity'. + $depth = $request->getHeader('Depth') ?? 'infinity'; + if ('infinity' === strtolower($depth)) { + $depth = self::DEPTH_INFINITY; + } elseif (!ctype_digit($depth) || ((int) $depth) < 0) { + throw new Exception\BadRequest('The HTTP Depth header may only be "infinity", 0 or a positive integer'); + } else { + $depth = (int) $depth; } + + $overwrite = $request->getHeader('Overwrite') ?? 'T'; if ('T' == strtoupper($overwrite)) { $overwrite = true; } elseif ('F' == strtoupper($overwrite)) { @@ -773,6 +782,7 @@ public function getCopyAndMoveInfo(RequestInterface $request) // These are the three relevant properties we need to return return [ + 'depth' => $depth, 'destination' => $destination, 'destinationExists' => (bool) $destinationNode, 'destinationNode' => $destinationNode, diff --git a/lib/DAV/Tree.php b/lib/DAV/Tree.php index 1483e1bc51..6a35feb654 100644 --- a/lib/DAV/Tree.php +++ b/lib/DAV/Tree.php @@ -137,8 +137,11 @@ public function nodeExists($path) * * @param string $sourcePath The source location * @param string $destinationPath The full destination path + * @param int $depth How many levels of children to copy. + * The value can be 'infinity' (\Sabre\DAV\Server::DEPTH_INFINITY) or a positive integer, including zero. + * Zero means only copy the collection without children but with its properties. */ - public function copy($sourcePath, $destinationPath) + public function copy($sourcePath, $destinationPath, int $depth = Server::DEPTH_INFINITY) { $sourceNode = $this->getNodeForPath($sourcePath); @@ -147,8 +150,8 @@ public function copy($sourcePath, $destinationPath) $destinationParent = $this->getNodeForPath($destinationDir); // Check if the target can handle the copy itself. If not, we do it ourselves. - if (!$destinationParent instanceof ICopyTarget || !$destinationParent->copyInto($destinationName, $sourcePath, $sourceNode)) { - $this->copyNode($sourceNode, $destinationParent, $destinationName); + if (!$destinationParent instanceof ICopyTarget || !$destinationParent->copyInto($destinationName, $sourcePath, $sourceNode, $depth)) { + $this->copyNode($sourceNode, $destinationParent, $destinationName, $depth); } $this->markDirty($destinationDir); @@ -178,7 +181,8 @@ public function move($sourcePath, $destinationPath) $moveSuccess = $newParentNode->moveInto($destinationName, $sourcePath, $sourceNode); } if (!$moveSuccess) { - $this->copy($sourcePath, $destinationPath); + // Move is a copy with depth = infinity and deleting the source afterwards + $this->copy($sourcePath, $destinationPath, Server::DEPTH_INFINITY); $this->getNodeForPath($sourcePath)->delete(); } } @@ -215,9 +219,13 @@ public function getChildren($path) $basePath .= '/'; } - foreach ($node->getChildren() as $child) { - $this->cache[$basePath.$child->getName()] = $child; - yield $child; + if ($node instanceof ICollection) { + foreach ($node->getChildren() as $child) { + $this->cache[$basePath.$child->getName()] = $child; + yield $child; + } + } else { + yield from []; } } @@ -303,8 +311,9 @@ public function getMultipleNodes($paths) * copyNode. * * @param string $destinationName + * @param int $depth How many children of the node to copy */ - protected function copyNode(INode $source, ICollection $destinationParent, $destinationName = null) + protected function copyNode(INode $source, ICollection $destinationParent, ?string $destinationName = null, int $depth = Server::DEPTH_INFINITY) { if ('' === (string) $destinationName) { $destinationName = $source->getName(); @@ -326,10 +335,16 @@ protected function copyNode(INode $source, ICollection $destinationParent, $dest $destination = $destinationParent->getChild($destinationName); } elseif ($source instanceof ICollection) { $destinationParent->createDirectory($destinationName); - $destination = $destinationParent->getChild($destinationName); - foreach ($source->getChildren() as $child) { - $this->copyNode($child, $destination); + + // Copy children if depth is not zero + if (0 !== $depth) { + // Adjust next depth for children (keep 'infinity' or decrease) + $depth = Server::DEPTH_INFINITY === $depth ? Server::DEPTH_INFINITY : $depth - 1; + $destination = $destinationParent->getChild($destinationName); + foreach ($source->getChildren() as $child) { + $this->copyNode($child, $destination, null, $depth); + } } } if ($source instanceof IProperties && $destination instanceof IProperties) { diff --git a/tests/Sabre/DAV/CorePluginTest.php b/tests/Sabre/DAV/CorePluginTest.php index 152a50ff55..3ab6812726 100644 --- a/tests/Sabre/DAV/CorePluginTest.php +++ b/tests/Sabre/DAV/CorePluginTest.php @@ -4,6 +4,10 @@ namespace Sabre\DAV; +use PHPUnit\Framework\MockObject\MockObject; +use Sabre\DAV\Exception\BadRequest; +use Sabre\HTTP; + class CorePluginTest extends \PHPUnit\Framework\TestCase { public function testGetInfo() @@ -11,4 +15,54 @@ public function testGetInfo() $corePlugin = new CorePlugin(); self::assertEquals('core', $corePlugin->getPluginInfo()['name']); } + + public function moveInvalidDepthHeaderProvider() + { + return [ + [0], + [1], + ]; + } + + /** + * MOVE does only allow "infinity" every other header value is considered invalid. + * + * @dataProvider moveInvalidDepthHeaderProvider + */ + public function testMoveWithInvalidDepth($depthHeader) + { + $request = new HTTP\Request('MOVE', '/path/'); + $response = new HTTP\Response(); + + /** @var Server|MockObject */ + $server = $this->getMockBuilder(Server::class)->getMock(); + $corePlugin = new CorePlugin(); + $corePlugin->initialize($server); + + $server->expects($this->once()) + ->method('getCopyAndMoveInfo') + ->willReturn(['depth' => $depthHeader]); + + $this->expectException(BadRequest::class); + $corePlugin->httpMove($request, $response); + } + + /** + * MOVE does only allow "infinity" every other header value is considered invalid. + */ + public function testMoveSupportsDepth() + { + $request = new HTTP\Request('MOVE', '/path/'); + $response = new HTTP\Response(); + + /** @var Server|MockObject */ + $server = $this->getMockBuilder(Server::class)->getMock(); + $corePlugin = new CorePlugin(); + $corePlugin->initialize($server); + + $server->expects($this->once()) + ->method('getCopyAndMoveInfo') + ->willReturn(['depth' => Server::DEPTH_INFINITY, 'destinationExists' => true, 'destination' => 'dst']); + $corePlugin->httpMove($request, $response); + } } diff --git a/tests/Sabre/DAV/FSExt/ServerTest.php b/tests/Sabre/DAV/FSExt/ServerTest.php index 3f2277400e..c884ae6bd0 100644 --- a/tests/Sabre/DAV/FSExt/ServerTest.php +++ b/tests/Sabre/DAV/FSExt/ServerTest.php @@ -234,7 +234,10 @@ public function testCopy() { mkdir($this->tempDir.'/testcol'); - $request = new HTTP\Request('COPY', '/test.txt', ['Destination' => '/testcol/test2.txt']); + $request = new HTTP\Request('COPY', '/test.txt', [ + 'Destination' => '/testcol/test2.txt', + 'Depth' => 'infinity', + ]); $this->server->httpRequest = ($request); $this->server->exec(); diff --git a/tests/Sabre/DAV/HttpCopyTest.php b/tests/Sabre/DAV/HttpCopyTest.php index 1089713000..0ce084df20 100644 --- a/tests/Sabre/DAV/HttpCopyTest.php +++ b/tests/Sabre/DAV/HttpCopyTest.php @@ -21,13 +21,21 @@ class HttpCopyTest extends AbstractDAVServerTestCase */ public function setUpTree() { - $this->tree = new Mock\Collection('root', [ + $propsCollection = new Mock\PropertiesCollection('propscoll', [ + 'file3' => 'content3', + 'file4' => 'content4', + ], [ + 'my-prop' => 'my-value', + ]); + $propsCollection->failMode = 'updatepropstrue'; + $this->tree = new Mock\PropertiesCollection('root', [ 'file1' => 'content1', 'file2' => 'content2', - 'coll1' => [ + 'coll1' => new Mock\Collection('coll1', [ 'file3' => 'content3', 'file4' => 'content4', - ], + ]), + 'propscoll' => $propsCollection, ]); } @@ -35,6 +43,7 @@ public function testCopyFile() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file5', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(201, $response->getStatus()); @@ -54,6 +63,7 @@ public function testCopyFileToExisting() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(204, $response->getStatus()); @@ -64,6 +74,7 @@ public function testCopyFileToExistingOverwriteT() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'T', ]); $response = $this->request($request); @@ -75,6 +86,7 @@ public function testCopyFileToExistingOverwriteBadValue() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'B', ]); $response = $this->request($request); @@ -85,6 +97,7 @@ public function testCopyFileNonExistantParent() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/notfound/file2', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(409, $response->getStatus()); @@ -94,6 +107,7 @@ public function testCopyFileToExistingOverwriteF() { $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'F', ]); $response = $this->request($request); @@ -110,6 +124,7 @@ public function testCopyFileToExistinBlockedCreateDestination() }); $request = new HTTP\Request('COPY', '/file1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'T', ]); $response = $this->request($request); @@ -122,16 +137,39 @@ public function testCopyColl() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/coll2', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(201, $response->getStatus()); self::assertEquals('content3', $this->tree->getChild('coll2')->getChild('file3')->get()); } + public function testShallowCopyColl() + { + // Ensure proppatches are applied + $this->tree->failMode = 'updatepropstrue'; + $request = new HTTP\Request('COPY', '/propscoll', [ + 'Destination' => '/shallow-coll', + 'Depth' => '0', + ]); + $response = $this->request($request); + // reset + $this->tree->failMode = false; + + self::assertEquals(201, $response->getStatus()); + // The copied collection exists + self::assertEquals(true, $this->tree->childExists('shallow-coll')); + // But it does not contain children + self::assertEquals([], $this->tree->getChild('shallow-coll')->getChildren()); + // But the properties are preserved + self::assertEquals(['my-prop' => 'my-value'], $this->tree->getChild('shallow-coll')->getProperties([])); + } + public function testCopyCollToSelf() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/coll1', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(403, $response->getStatus()); @@ -141,6 +179,7 @@ public function testCopyCollToExisting() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(204, $response->getStatus()); @@ -151,6 +190,7 @@ public function testCopyCollToExistingOverwriteT() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'T', ]); $response = $this->request($request); @@ -162,6 +202,7 @@ public function testCopyCollToExistingOverwriteF() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/file2', + 'Depth' => 'infinity', 'Overwrite' => 'F', ]); $response = $this->request($request); @@ -173,6 +214,7 @@ public function testCopyCollIntoSubtree() { $request = new HTTP\Request('COPY', '/coll1', [ 'Destination' => '/coll1/subcol', + 'Depth' => 'infinity', ]); $response = $this->request($request); self::assertEquals(409, $response->getStatus()); diff --git a/tests/Sabre/DAV/Locks/PluginTest.php b/tests/Sabre/DAV/Locks/PluginTest.php index 465bbc1609..c82033c063 100644 --- a/tests/Sabre/DAV/Locks/PluginTest.php +++ b/tests/Sabre/DAV/Locks/PluginTest.php @@ -585,6 +585,7 @@ public function testLockCopyLockSource() $request = new HTTP\Request('COPY', '/dir/child.txt', [ 'Destination' => '/dir/child2.txt', + 'Depth' => 'infinity', ]); $this->server->httpRequest = $request; @@ -619,6 +620,7 @@ public function testLockCopyLockDestination() $request = new HTTP\Request('COPY', '/dir/child.txt', [ 'Destination' => '/dir/child2.txt', + 'Depth' => 'infinity', ]); $this->server->httpRequest = $request; $this->server->exec(); diff --git a/tests/Sabre/DAV/Mock/Collection.php b/tests/Sabre/DAV/Mock/Collection.php index 7baa6f6216..f8b00c7990 100644 --- a/tests/Sabre/DAV/Mock/Collection.php +++ b/tests/Sabre/DAV/Mock/Collection.php @@ -118,7 +118,7 @@ public function createDirectory($name) */ public function getChildren() { - return $this->children; + return $this->children ?? []; } /** diff --git a/tests/Sabre/DAV/Mock/PropertiesCollection.php b/tests/Sabre/DAV/Mock/PropertiesCollection.php index 42468f995d..b0a1695899 100644 --- a/tests/Sabre/DAV/Mock/PropertiesCollection.php +++ b/tests/Sabre/DAV/Mock/PropertiesCollection.php @@ -45,6 +45,12 @@ public function propPatch(PropPatch $proppatch) $proppatch->handleRemaining(function ($updateProperties) { switch ($this->failMode) { case 'updatepropsfalse': return false; + case 'updatepropstrue': + foreach ($updateProperties as $k => $v) { + $this->properties[$k] = $v; + } + + return true; case 'updatepropsarray': $r = []; foreach ($updateProperties as $k => $v) { @@ -76,6 +82,10 @@ public function propPatch(PropPatch $proppatch) */ public function getProperties($requestedProperties) { + if (0 === count($requestedProperties)) { + return $this->properties; + } + $returnedProperties = []; foreach ($requestedProperties as $requestedProperty) { if (isset($this->properties[$requestedProperty])) { @@ -86,4 +96,17 @@ public function getProperties($requestedProperties) return $returnedProperties; } + + /** + * Creates a new subdirectory. (Override to ensure props are preserved). + * + * @param string $name + */ + public function createDirectory($name) + { + $child = new self($name, []); + // keep same setting + $child->failMode = $this->failMode; + $this->children[] = $child; + } } diff --git a/tests/Sabre/DAV/ServerCopyMoveTest.php b/tests/Sabre/DAV/ServerCopyMoveTest.php new file mode 100644 index 0000000000..47350f2805 --- /dev/null +++ b/tests/Sabre/DAV/ServerCopyMoveTest.php @@ -0,0 +1,91 @@ + 'infinity']); + + $this->expectException(BadRequest::class); + $this->expectExceptionMessage('The destination header was not supplied'); + $this->server->getCopyAndMoveInfo($request); + } + + public function testMissingDepthHeader() + { + $request = new HTTP\Request('COPY', '/', ['Destination' => '/destination']); + + $this->assertEquals(Server::DEPTH_INFINITY, $this->server->getCopyAndMoveInfo($request)['depth']); + } + + /** + * Only 'infinity' and positive (incl. 0) numbers are allowed. + * + * @dataProvider dataInvalidDepthHeader + */ + public function testInvalidDepthHeader(string $headerValue) + { + $request = new HTTP\Request('COPY', '/', ['Destination' => '/destination', 'Depth' => $headerValue]); + + $this->expectException(BadRequest::class); + $this->expectExceptionMessage('The HTTP Depth header may only be "infinity", 0 or a positive integer'); + $this->server->getCopyAndMoveInfo($request); + } + + public function dataInvalidDepthHeader() + { + return [ + ['-1'], + ['0.5'], + ['2f'], + ['inf'], + ]; + } + + /** + * Only 'infinity' and positive (incl. 0) numbers are allowed. + * + * @dataProvider dataDepthHeader + * + * @param string|int $expectedDepth + */ + public function testValidDepthHeader(array $depthHeader, $expectedDepth) + { + $request = new HTTP\Request('COPY', '/', array_merge(['Destination' => '/dst'], $depthHeader)); + + $this->assertEquals($expectedDepth, $this->server->getCopyAndMoveInfo($request)['depth']); + } + + public function dataDepthHeader() + { + return [ + [ + [], + Server::DEPTH_INFINITY, + ], + [ + ['Depth' => 'infinity'], + Server::DEPTH_INFINITY, + ], + [ + ['Depth' => 'INFINITY'], + Server::DEPTH_INFINITY, + ], + [ + ['Depth' => '0'], + 0, + ], + [ + ['Depth' => '10'], + 10, + ], + ]; + } +} diff --git a/tests/Sabre/DAV/ServerEventsTest.php b/tests/Sabre/DAV/ServerEventsTest.php index 753e390c35..5a9e5fb726 100644 --- a/tests/Sabre/DAV/ServerEventsTest.php +++ b/tests/Sabre/DAV/ServerEventsTest.php @@ -69,6 +69,7 @@ public function testAfterCopy() $this->server->createFile($oldPath, 'body'); $request = new HTTP\Request('COPY', $oldPath, [ 'Destination' => $newPath, + 'Depth' => 'infinity', ]); $this->server->httpRequest = $request;