Handle 404 in CardDAV multi get - refs #829

sabre-io · Nov 13, 2023 · e61f0a0 · e61f0a0
1 parent 9fb8b91
commit e61f0a0
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 17 deletions.
diff --git 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
@@ -4,6 +4,7 @@
 
 namespace Sabre\DAV;
 
+use Sabre\DAV\Xml\Response\MultiStatus;
 use Sabre\HTTP;
 use Sabre\Uri;
 
@@ -416,6 +417,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
@@ -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'] .= '/';
+                    }
                 }
             }
         }
@@ -1665,9 +1668,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
@@ -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
@@ -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)
         $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
@@ -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(
+            '<?xml version="1.0"?>
+<c:addressbook-multiget xmlns:d="DAV:" xmlns:c="urn:ietf:params:xml:ns:carddav">
+    <d:prop>
+      <d:getetag />
+      <c:address-data />
+    </d:prop>
+    <d:href>/addressbooks/user1/unknown/card1</d:href>
+    <d:href>/addressbooks/user1/book1/card1</d:href>
+    <d:href>/addressbooks/user1/book1/unknown-card</d:href>
+</c:addressbook-multiget>'
+        );
+
+        $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('<?xml version="1.0"?>
+<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:card="urn:ietf:params:xml:ns:carddav">
+ <d:response>
+  <d:href>/addressbooks/user1/unknown/card1</d:href>
+  <d:status>HTTP/1.1 404 Not Found</d:status>
+ </d:response>
+ <d:response>
+  <d:href>/addressbooks/user1/book1/card1</d:href>
+    <d:propstat>
+      <d:prop>
+        <d:getetag>"ffe3b42186ba156c84fc1581c273f01c"</d:getetag>
+        <card:address-data>BEGIN:VCARD
+VERSION:3.0
+UID:12345
+END:VCARD</card:address-data>
+      </d:prop>
+      <d:status>HTTP/1.1 200 OK</d:status>
+    </d:propstat>
+ </d:response>
+ <d:response>
+  <d:href>/addressbooks/user1/book1/unknown-card</d:href>
+  <d:status>HTTP/1.1 404 Not Found</d:status>
+ </d:response>
+</d:multistatus>', $response->body);
+    }
 }
diff --git 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