From 3c392758dfd0e5261eeeb585c4995555fa28cd03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 9 Nov 2023 13:06:16 +0100 Subject: [PATCH] fix: handle 404 in CardDAV multi get - refs #829 --- lib/CardDAV/AddressBook.php | 6 ++- lib/DAV/Client.php | 2 + lib/DAV/Server.php | 27 ++++++++------ lib/DAV/Tree.php | 23 +++++++++++- lib/DAV/Xml/Element/Response.php | 4 +- tests/Sabre/CardDAV/MultiGetTest.php | 55 ++++++++++++++++++++++++++++ tests/Sabre/DAV/Sync/PluginTest.php | 1 + 7 files changed, 101 insertions(+), 17 deletions(-) diff --git a/lib/CardDAV/AddressBook.php b/lib/CardDAV/AddressBook.php index f5744f644b..c4c9dcff55 100644 --- a/lib/CardDAV/AddressBook.php +++ b/lib/CardDAV/AddressBook.php @@ -102,8 +102,10 @@ public function getMultipleChildren(array $paths) $objs = $this->carddavBackend->getMultipleCards($this->addressBookInfo['id'], $paths); $children = []; foreach ($objs as $obj) { - $obj['acl'] = $this->getChildACL(); - $children[] = new Card($this->carddavBackend, $this->addressBookInfo, $obj); + if (is_array($obj)) { + $obj['acl'] = $this->getChildACL(); + $children[] = new Card($this->carddavBackend, $this->addressBookInfo, $obj); + } } return $children; diff --git a/lib/DAV/Client.php b/lib/DAV/Client.php index 187b29ba37..ac94a4d718 100644 --- a/lib/DAV/Client.php +++ b/lib/DAV/Client.php @@ -4,6 +4,7 @@ namespace Sabre\DAV; +use Sabre\DAV\Xml\Response\MultiStatus; use Sabre\HTTP; use Sabre\Uri; @@ -472,6 +473,7 @@ public function getAbsoluteUrl($url) */ public function parseMultiStatus($body) { + /** @var MultiStatus $multistatus */ $multistatus = $this->xml->expect('{DAV:}multistatus', $body); $result = []; diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php index 3133e54ad3..afb6ecdba6 100644 --- a/lib/DAV/Server.php +++ b/lib/DAV/Server.php @@ -1015,19 +1015,22 @@ public function getPropertiesForMultiplePaths(array $paths, array $propertyNames { $result = [ ]; - $nodes = $this->tree->getMultipleNodes($paths); - foreach ($nodes as $path => $node) { - $propFind = new PropFind($path, $propertyNames); - $r = $this->getPropertiesByNode($propFind, $node); - if ($r) { - $result[$path] = $propFind->getResultForMultiStatus(); + if (null === $node) { + $result[$path] = []; $result[$path]['href'] = $path; - - $resourceType = $this->getResourceTypeForNode($node); - if (in_array('{DAV:}collection', $resourceType) || in_array('{DAV:}principal', $resourceType)) { - $result[$path]['href'] .= '/'; + $result[$path]['status'] = 404; + } else { + $propFind = new PropFind($path, $propertyNames); + $r = $this->getPropertiesByNode($propFind, $node); + if ($r) { + $result[$path] = $propFind->getResultForMultiStatus(); + $result[$path]['href'] = $path; + $resourceType = $this->getResourceTypeForNode($node); + if (in_array('{DAV:}collection', $resourceType) || in_array('{DAV:}principal', $resourceType)) { + $result[$path]['href'] .= '/'; + } } } } @@ -1667,9 +1670,11 @@ private function writeMultiStatus(Writer $w, $fileProperties, bool $strip404s) if ($strip404s) { unset($entry[404]); } + $status = isset($entry['status']) ? $entry['status'] : null; $response = new Xml\Element\Response( ltrim($href, '/'), - $entry + $entry, + $status ); $w->write([ 'name' => '{DAV:}response', diff --git a/lib/DAV/Tree.php b/lib/DAV/Tree.php index 65b4583ceb..8f68045af9 100644 --- a/lib/DAV/Tree.php +++ b/lib/DAV/Tree.php @@ -4,6 +4,7 @@ namespace Sabre\DAV; +use Sabre\DAV\Exception\NotFound; use Sabre\Uri; /** @@ -269,17 +270,35 @@ public function getMultipleNodes($paths) $result = []; foreach ($parents as $parent => $children) { - $parentNode = $this->getNodeForPath($parent); + try { + $parentNode = $this->getNodeForPath($parent); + } catch (NotFound $ex) { + foreach ($children as $child) { + $fullPath = $parent.'/'.$child; + $result[$fullPath] = null; + } + continue; + } if ($parentNode instanceof IMultiGet) { foreach ($parentNode->getMultipleChildren($children) as $childNode) { $fullPath = $parent.'/'.$childNode->getName(); $result[$fullPath] = $childNode; $this->cache[$fullPath] = $childNode; } + foreach ($children as $child) { + $fullPath = $parent.'/'.$child; + if (!isset($result[$fullPath])) { + $result[$fullPath] = null; + } + } } else { foreach ($children as $child) { $fullPath = $parent.'/'.$child; - $result[$fullPath] = $this->getNodeForPath($fullPath); + try { + $result[$fullPath] = $this->getNodeForPath($fullPath); + } catch (NotFound $ex) { + $result[$fullPath] = null; + } } } } diff --git a/lib/DAV/Xml/Element/Response.php b/lib/DAV/Xml/Element/Response.php index 86f2d33882..d91e4e0265 100644 --- a/lib/DAV/Xml/Element/Response.php +++ b/lib/DAV/Xml/Element/Response.php @@ -29,7 +29,7 @@ class Response implements Element protected $href; /** - * Propertylist, ordered by HTTP status code. + * Property list, ordered by HTTP status code. * * @var array */ @@ -124,7 +124,7 @@ public function xmlSerialize(Writer $writer): void $writer->writeElement('{DAV:}href', $writer->contextUri.\Sabre\HTTP\encodePath($this->getHref())); $empty = true; - $httpStatus = $this->getHTTPStatus(); + $httpStatus = $this->getHttpStatus(); // Add propstat elements foreach ($this->getResponseProperties() as $status => $properties) { diff --git a/tests/Sabre/CardDAV/MultiGetTest.php b/tests/Sabre/CardDAV/MultiGetTest.php index 3557ddaeaa..ac2b90faf6 100644 --- a/tests/Sabre/CardDAV/MultiGetTest.php +++ b/tests/Sabre/CardDAV/MultiGetTest.php @@ -96,4 +96,59 @@ public function testMultiGetVCard4() ], ], $result); } + + public function testMultiGet404() + { + $request = HTTP\Sapi::createFromServerArray([ + 'REQUEST_METHOD' => 'REPORT', + 'REQUEST_URI' => '/addressbooks/user1/book1', + ]); + + $request->setBody( + ' + + + + + + /addressbooks/user1/unknown/card1 + /addressbooks/user1/book1/card1 + /addressbooks/user1/book1/unknown-card +' + ); + + $response = new HTTP\ResponseMock(); + + $this->server->httpRequest = $request; + $this->server->httpResponse = $response; + + $this->server->exec(); + + $this->assertEquals(207, $response->status, 'Incorrect status code. Full response body:'.$response->body); + + $this->assertXmlStringEqualsXmlString(' + + + /addressbooks/user1/unknown/card1 + HTTP/1.1 404 Not Found + + + /addressbooks/user1/book1/card1 + + + "ffe3b42186ba156c84fc1581c273f01c" + BEGIN:VCARD +VERSION:3.0 +UID:12345 +END:VCARD + + HTTP/1.1 200 OK + + + + /addressbooks/user1/book1/unknown-card + HTTP/1.1 404 Not Found + +', $response->body); + } } diff --git a/tests/Sabre/DAV/Sync/PluginTest.php b/tests/Sabre/DAV/Sync/PluginTest.php index bfe0d157a6..02687d42e4 100644 --- a/tests/Sabre/DAV/Sync/PluginTest.php +++ b/tests/Sabre/DAV/Sync/PluginTest.php @@ -92,6 +92,7 @@ public function testSyncInitialSyncCollection() self::assertEquals(207, $response->status, 'Full response body:'.$response->getBodyAsString()); + /** @var DAV\Xml\Response\MultiStatus $multiStatus */ $multiStatus = $this->server->xml->parse($response->getBodyAsString()); // Checking the sync-token