Skip to content

Commit

Permalink
fix(CorePlugin): Handle Depth header for COPY requests
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@susnux
susnux committed Sep 5, 2024
1 parent 9dfa4be commit 069c16d
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 20 deletions.
6 changes: 3 additions & 3 deletions lib/DAV/CorePlugin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion lib/DAV/ICopyTarget.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
37 changes: 26 additions & 11 deletions lib/DAV/Tree.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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 [];
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion tests/Sabre/DAV/FSExt/ServerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
48 changes: 45 additions & 3 deletions tests/Sabre/DAV/HttpCopyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,29 @@ 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,
]);
}

public function testCopyFile()
{
$request = new HTTP\Request('COPY', '/file1', [
'Destination' => '/file5',
'Depth' => 'infinity',
]);
$response = $this->request($request);
self::assertEquals(201, $response->getStatus());
Expand All @@ -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());
Expand All @@ -64,6 +74,7 @@ public function testCopyFileToExistingOverwriteT()
{
$request = new HTTP\Request('COPY', '/file1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'T',
]);
$response = $this->request($request);
Expand All @@ -75,6 +86,7 @@ public function testCopyFileToExistingOverwriteBadValue()
{
$request = new HTTP\Request('COPY', '/file1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'B',
]);
$response = $this->request($request);
Expand All @@ -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());
Expand All @@ -94,6 +107,7 @@ public function testCopyFileToExistingOverwriteF()
{
$request = new HTTP\Request('COPY', '/file1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'F',
]);
$response = $this->request($request);
Expand All @@ -110,6 +124,7 @@ public function testCopyFileToExistinBlockedCreateDestination()
});
$request = new HTTP\Request('COPY', '/file1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'T',
]);
$response = $this->request($request);
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -151,6 +190,7 @@ public function testCopyCollToExistingOverwriteT()
{
$request = new HTTP\Request('COPY', '/coll1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'T',
]);
$response = $this->request($request);
Expand All @@ -162,6 +202,7 @@ public function testCopyCollToExistingOverwriteF()
{
$request = new HTTP\Request('COPY', '/coll1', [
'Destination' => '/file2',
'Depth' => 'infinity',
'Overwrite' => 'F',
]);
$response = $this->request($request);
Expand All @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions tests/Sabre/DAV/Locks/PluginTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion tests/Sabre/DAV/Mock/Collection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public function createDirectory($name)
*/
public function getChildren()
{
return $this->children;
return $this->children ?? [];
}

/**
Expand Down
22 changes: 22 additions & 0 deletions tests/Sabre/DAV/Mock/PropertiesCollection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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])) {
Expand All @@ -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;
}
}
1 change: 1 addition & 0 deletions tests/Sabre/DAV/ServerEventsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 069c16d

Please sign in to comment.