From 9dfa4beab9a13b0138362b1630335ad4cf730620 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 25 Oct 2023 19:50:47 +0200 Subject: [PATCH 1/3] fix(CorePlugin): Handle `Depth` header in HTTP MOVE Only `Depth: infinity` is allowed, ref: rfc4918#section-9.9.2 Signed-off-by: Ferdinand Thiessen --- lib/DAV/CorePlugin.php | 5 ++ lib/DAV/Server.php | 14 ++++-- tests/Sabre/DAV/CorePluginTest.php | 50 +++++++++++++++++++ tests/Sabre/DAV/ServerCopyMoveTest.php | 68 ++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 tests/Sabre/DAV/ServerCopyMoveTest.php diff --git a/lib/DAV/CorePlugin.php b/lib/DAV/CorePlugin.php index dbd8976b17..429832cdb8 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 ($moveInfo['depth'] !== 'infinity') { + 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; diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php index 3133e54ad3..19b1cda97c 100644 --- a/lib/DAV/Server.php +++ b/lib/DAV/Server.php @@ -725,10 +725,17 @@ 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 inifinty is valid for MOVE and COPY. If it is not set the RFC requires to act like it was 'infinity'. + $depth = strtolower($request->getHeader('Depth') ?? 'infinity'); + if ($depth !== 'infinity' && is_numeric($depth)) { + $depth = (int)$depth; + if ($depth < 0) { + throw new Exception\BadRequest('The HTTP Depth header may only be "infinity", 0 or a positiv number'); + } } + + $overwrite = $request->getHeader('Overwrite') ?? 'T'; if ('T' == strtoupper($overwrite)) { $overwrite = true; } elseif ('F' == strtoupper($overwrite)) { @@ -773,6 +780,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/tests/Sabre/DAV/CorePluginTest.php b/tests/Sabre/DAV/CorePluginTest.php index 152a50ff55..0749e56d08 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,50 @@ 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' => 'infinity', 'destinationExists' => true, 'destination' => 'dst']); + $corePlugin->httpMove($request, $response); + } } diff --git a/tests/Sabre/DAV/ServerCopyMoveTest.php b/tests/Sabre/DAV/ServerCopyMoveTest.php new file mode 100644 index 0000000000..df4f279694 --- /dev/null +++ b/tests/Sabre/DAV/ServerCopyMoveTest.php @@ -0,0 +1,68 @@ + $headerValue] : []); + + $this->expectException(BadRequest::class); + $this->server->getCopyAndMoveInfo($request); + } + + public function dataInvalidDepthHeader() { + return [ + ['-1'], + ['0.5'], + ['2f'], + ['inf'], + ]; + } + + /** + * Only 'infinity' and positiv (incl. 0) numbers are allowed + * @dataProvider dataDepthHeader + */ + public function testValidDepthHeader(array $depthHeader, string|int $expectedDepth) + { + $request = new HTTP\Request('COPY', '/', array_merge(['Destination' => '/dst'], $depthHeader)); + + $this->assertEquals($expectedDepth, $this->server->getCopyAndMoveInfo($request)['depth']); + } + + public function dataDepthHeader() { + return [ + [ + [], + 'infinity', + ], + [ + ['Depth' => 'infinity'], + 'infinity', + ], + [ + ['Depth' => 'INFINITY'], + 'infinity', + ], + [ + ['Depth' => '0'], + 0, + ], + [ + ['Depth' => '10'], + 10, + ], + ]; + } +} From 069c16d3399aa4b473d1257c1004f4436fad9f75 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 25 Oct 2023 23:57:46 +0200 Subject: [PATCH 2/3] fix(CorePlugin): Handle `Depth` header for `COPY` requests 1. According to the RFC[1] servers **must** support `Depth` 'infinity' and 0. 2. And COPY method on a collection without a Depth header MUST act as if a Depth header with value "infinity" was included. [1] rfc4918#section-9.8.3 Signed-off-by: Ferdinand Thiessen --- lib/DAV/CorePlugin.php | 6 +-- lib/DAV/ICopyTarget.php | 5 +- lib/DAV/Tree.php | 37 +++++++++----- tests/Sabre/DAV/FSExt/ServerTest.php | 5 +- tests/Sabre/DAV/HttpCopyTest.php | 48 +++++++++++++++++-- tests/Sabre/DAV/Locks/PluginTest.php | 2 + tests/Sabre/DAV/Mock/Collection.php | 2 +- tests/Sabre/DAV/Mock/PropertiesCollection.php | 22 +++++++++ tests/Sabre/DAV/ServerEventsTest.php | 1 + 9 files changed, 108 insertions(+), 20 deletions(-) diff --git a/lib/DAV/CorePlugin.php b/lib/DAV/CorePlugin.php index 429832cdb8..7ca7296a75 100644 --- a/lib/DAV/CorePlugin.php +++ b/lib/DAV/CorePlugin.php @@ -650,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; } @@ -661,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..c887368dda 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 string|int $depth How many level of children to copy. + * The value can be 'infinity' or a positiv 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, $depth); } diff --git a/lib/DAV/Tree.php b/lib/DAV/Tree.php index 1483e1bc51..ffb2e63e13 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|string $depth How much levle of children to copy. + * The value can be 'infinity' or a positiv 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, $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, '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|string $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, $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 ($depth !== 0) { + // Adjust next depth for children (keep 'infinity' or decrease) + $depth = $depth === 'infinity' ? '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/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 66f16abd81..345b5b9ce6 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..746b30e5a1 100644 --- a/tests/Sabre/DAV/Mock/PropertiesCollection.php +++ b/tests/Sabre/DAV/Mock/PropertiesCollection.php @@ -45,6 +45,11 @@ 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 +81,10 @@ public function propPatch(PropPatch $proppatch) */ public function getProperties($requestedProperties) { + if (count($requestedProperties) === 0) { + return $this->properties; + } + $returnedProperties = []; foreach ($requestedProperties as $requestedProperty) { if (isset($this->properties[$requestedProperty])) { @@ -86,4 +95,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/ServerEventsTest.php b/tests/Sabre/DAV/ServerEventsTest.php index cee10e6552..d02b24472e 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; From 3e722c392e5501bdeaf9b2a1503c6ce90ffa4cc3 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 5 Sep 2024 15:40:22 +0200 Subject: [PATCH 3/3] chore: Fix code style Signed-off-by: Ferdinand Thiessen --- lib/DAV/CorePlugin.php | 2 +- lib/DAV/ICopyTarget.php | 12 +++++------ lib/DAV/Server.php | 4 ++-- lib/DAV/Tree.php | 18 ++++++++--------- tests/Sabre/DAV/CorePluginTest.php | 14 ++++++++----- tests/Sabre/DAV/Mock/PropertiesCollection.php | 5 +++-- tests/Sabre/DAV/ServerCopyMoveTest.php | 20 ++++++++++++------- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/lib/DAV/CorePlugin.php b/lib/DAV/CorePlugin.php index 7ca7296a75..c0725ed145 100644 --- a/lib/DAV/CorePlugin.php +++ b/lib/DAV/CorePlugin.php @@ -590,7 +590,7 @@ 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 ($moveInfo['depth'] !== 'infinity') { + if ('infinity' !== $moveInfo['depth']) { throw new BadRequest('The HTTP Depth header must only contain "infinity" for MOVE'); } diff --git a/lib/DAV/ICopyTarget.php b/lib/DAV/ICopyTarget.php index c887368dda..26ffef038c 100644 --- a/lib/DAV/ICopyTarget.php +++ b/lib/DAV/ICopyTarget.php @@ -28,12 +28,12 @@ interface ICopyTarget extends ICollection * is that the copy was successful. * If you return false, sabre/dav will handle the copy itself. * - * @param string $targetName new local file/collection name - * @param string $sourcePath Full path to source node - * @param INode $sourceNode Source node itself - * @param string|int $depth How many level of children to copy. - * The value can be 'infinity' or a positiv number including zero. - * Zero means to only copy a shallow collection with props but without children. + * @param string $targetName new local file/collection name + * @param string $sourcePath Full path to source node + * @param INode $sourceNode Source node itself + * @param string|int $depth How many level of children to copy. + * The value can be 'infinity' or a positiv number including zero. + * Zero means to only copy a shallow collection with props but without children. * * @return bool */ diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php index 19b1cda97c..6da5bc7872 100644 --- a/lib/DAV/Server.php +++ b/lib/DAV/Server.php @@ -728,8 +728,8 @@ public function getCopyAndMoveInfo(RequestInterface $request) // Depth of inifinty is valid for MOVE and COPY. If it is not set the RFC requires to act like it was 'infinity'. $depth = strtolower($request->getHeader('Depth') ?? 'infinity'); - if ($depth !== 'infinity' && is_numeric($depth)) { - $depth = (int)$depth; + if ('infinity' !== $depth && is_numeric($depth)) { + $depth = (int) $depth; if ($depth < 0) { throw new Exception\BadRequest('The HTTP Depth header may only be "infinity", 0 or a positiv number'); } diff --git a/lib/DAV/Tree.php b/lib/DAV/Tree.php index ffb2e63e13..01308d50fe 100644 --- a/lib/DAV/Tree.php +++ b/lib/DAV/Tree.php @@ -135,11 +135,11 @@ public function nodeExists($path) /** * Copies a file from path to another. * - * @param string $sourcePath The source location - * @param string $destinationPath The full destination path - * @param int|string $depth How much levle of children to copy. - * The value can be 'infinity' or a positiv integer, including zero. - * Zero means only copy the collection without children but with its properties. + * @param string $sourcePath The source location + * @param string $destinationPath The full destination path + * @param int|string $depth How much levle of children to copy. + * The value can be 'infinity' or a positiv integer, including zero. + * Zero means only copy the collection without children but with its properties. */ public function copy($sourcePath, $destinationPath, $depth = 'infinity') { @@ -310,8 +310,8 @@ public function getMultipleNodes($paths) /** * copyNode. * - * @param string $destinationName - * @param int|string $depth How many children of the node to copy + * @param string $destinationName + * @param int|string $depth How many children of the node to copy */ protected function copyNode(INode $source, ICollection $destinationParent, ?string $destinationName = null, $depth = 'infinity') { @@ -338,9 +338,9 @@ protected function copyNode(INode $source, ICollection $destinationParent, ?stri $destination = $destinationParent->getChild($destinationName); // Copy children if depth is not zero - if ($depth !== 0) { + if (0 !== $depth) { // Adjust next depth for children (keep 'infinity' or decrease) - $depth = $depth === 'infinity' ? 'infinity' : $depth - 1; + $depth = 'infinity' === $depth ? 'infinity' : $depth - 1; $destination = $destinationParent->getChild($destinationName); foreach ($source->getChildren() as $child) { $this->copyNode($child, $destination, null, $depth); diff --git a/tests/Sabre/DAV/CorePluginTest.php b/tests/Sabre/DAV/CorePluginTest.php index 0749e56d08..8e61e0886d 100644 --- a/tests/Sabre/DAV/CorePluginTest.php +++ b/tests/Sabre/DAV/CorePluginTest.php @@ -16,7 +16,8 @@ public function testGetInfo() self::assertEquals('core', $corePlugin->getPluginInfo()['name']); } - public function moveInvalidDepthHeaderProvider() { + public function moveInvalidDepthHeaderProvider() + { return [ [0], [1], @@ -24,10 +25,12 @@ public function moveInvalidDepthHeaderProvider() { } /** - * MOVE does only allow "infinity" every other header value is considered invalid + * MOVE does only allow "infinity" every other header value is considered invalid. + * * @dataProvider moveInvalidDepthHeaderProvider */ - public function testMoveWithInvalidDepth($depthHeader) { + public function testMoveWithInvalidDepth($depthHeader) + { $request = new HTTP\Request('MOVE', '/path/'); $response = new HTTP\Response(); @@ -45,9 +48,10 @@ public function testMoveWithInvalidDepth($depthHeader) { } /** - * MOVE does only allow "infinity" every other header value is considered invalid + * MOVE does only allow "infinity" every other header value is considered invalid. */ - public function testMoveSupportsDepth() { + public function testMoveSupportsDepth() + { $request = new HTTP\Request('MOVE', '/path/'); $response = new HTTP\Response(); diff --git a/tests/Sabre/DAV/Mock/PropertiesCollection.php b/tests/Sabre/DAV/Mock/PropertiesCollection.php index 746b30e5a1..b0a1695899 100644 --- a/tests/Sabre/DAV/Mock/PropertiesCollection.php +++ b/tests/Sabre/DAV/Mock/PropertiesCollection.php @@ -49,6 +49,7 @@ public function propPatch(PropPatch $proppatch) foreach ($updateProperties as $k => $v) { $this->properties[$k] = $v; } + return true; case 'updatepropsarray': $r = []; @@ -81,7 +82,7 @@ public function propPatch(PropPatch $proppatch) */ public function getProperties($requestedProperties) { - if (count($requestedProperties) === 0) { + if (0 === count($requestedProperties)) { return $this->properties; } @@ -97,7 +98,7 @@ public function getProperties($requestedProperties) } /** - * Creates a new subdirectory. (Override to ensure props are preserved) + * Creates a new subdirectory. (Override to ensure props are preserved). * * @param string $name */ diff --git a/tests/Sabre/DAV/ServerCopyMoveTest.php b/tests/Sabre/DAV/ServerCopyMoveTest.php index df4f279694..f7c07abc12 100644 --- a/tests/Sabre/DAV/ServerCopyMoveTest.php +++ b/tests/Sabre/DAV/ServerCopyMoveTest.php @@ -7,21 +7,23 @@ use Sabre\DAV\Exception\BadRequest; use Sabre\HTTP; -class ServerCopyMoveTest extends AbstractServer +class ServerCopyMoveTest extends AbstractServerTestCase { /** - * Only 'infinity' and positiv (incl. 0) numbers are allowed + * Only 'infinity' and positive (incl. 0) numbers are allowed. + * * @dataProvider dataInvalidDepthHeader */ public function testInvalidDepthHeader(?string $headerValue) { - $request = new HTTP\Request('COPY', '/', $headerValue !== null ? ['Depth' => $headerValue] : []); + $request = new HTTP\Request('COPY', '/', null !== $headerValue ? ['Depth' => $headerValue] : []); $this->expectException(BadRequest::class); $this->server->getCopyAndMoveInfo($request); } - public function dataInvalidDepthHeader() { + public function dataInvalidDepthHeader() + { return [ ['-1'], ['0.5'], @@ -31,17 +33,21 @@ public function dataInvalidDepthHeader() { } /** - * Only 'infinity' and positiv (incl. 0) numbers are allowed + * Only 'infinity' and positive (incl. 0) numbers are allowed. + * * @dataProvider dataDepthHeader + * + * @param string|int $expectedDepth */ - public function testValidDepthHeader(array $depthHeader, 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() { + public function dataDepthHeader() + { return [ [ [],