From 069c16d3399aa4b473d1257c1004f4436fad9f75 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 25 Oct 2023 23:57:46 +0200 Subject: [PATCH] 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;