Fix DAV stat cache to properly cache 404

404 errors were not properly cached due to catching the wrong
exception. Now catching ClientHttpException and checking the error
code. In case of 404, adjust the stat cache accordingly.

diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php
index ea4bbba2748c03320749857182def651e5d200b3..2f821b218fc45bad494f3aa7d0f74ed7890cb12a 100644 (file)
--- a/lib/private/Files/Storage/DAV.php
+++ b/lib/private/Files/Storage/DAV.php
@@ -48,7 +48,6 @@ use OCP\Files\StorageInvalidException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\Util;
 use Sabre\DAV\Client;
-use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\Xml\Property\ResourceType;
 use Sabre\HTTP\ClientException;
 use Sabre\HTTP\ClientHttpException;
@@ -203,13 +202,15 @@ class DAV extends Common {
                $this->init();
                $path = $this->cleanPath($path);
                try {
-                       $response = $this->client->propfind(
+                       $response = $this->client->propFind(
                                $this->encodePath($path),
-                               array(),
+                               [],
                                1
                        );
-                       $id = md5('webdav' . $this->root . $path);
-                       $content = array();
+                       if ($response === false) {
+                               return false;
+                       }
+                       $content = [];
                        $files = array_keys($response);
                        array_shift($files); //the first entry is the current directory
 
@@ -226,13 +227,6 @@ class DAV extends Common {
                                $content[] = $file;
                        }
                        return IteratorDirectory::wrap($content);
-               } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404) {
-                               $this->statCache->clear($path . '/');
-                               $this->statCache->set($path, false);
-                               return false;
-                       }
-                       $this->convertException($e, $path);
                } catch (\Exception $e) {
                        $this->convertException($e, $path);
                }
@@ -247,22 +241,18 @@ class DAV extends Common {
         *
         * @param string $path path to propfind
         *
-        * @return array propfind response
+        * @return array|boolean propfind response or false if the entry was not found
         *
-        * @throws NotFound
+        * @throws ClientHttpException
         */
        protected function propfind($path) {
                $path = $this->cleanPath($path);
                $cachedResponse = $this->statCache->get($path);
-               if ($cachedResponse === false) {
-                       // we know it didn't exist
-                       throw new NotFound();
-               }
                // we either don't know it, or we know it exists but need more details
                if (is_null($cachedResponse) || $cachedResponse === true) {
                        $this->init();
                        try {
-                               $response = $this->client->propfind(
+                               $response = $this->client->propFind(
                                        $this->encodePath($path),
                                        array(
                                                '{DAV:}getlastmodified',
@@ -275,11 +265,15 @@ class DAV extends Common {
                                        )
                                );
                                $this->statCache->set($path, $response);
-                       } catch (NotFound $e) {
-                               // remember that this path did not exist
-                               $this->statCache->clear($path . '/');
-                               $this->statCache->set($path, false);
-                               throw $e;
+                       } catch (ClientHttpException $e) {
+                               if ($e->getHttpStatus() === 404) {
+                                       $this->statCache->clear($path . '/');
+                                       $this->statCache->set($path, false);
+                                       return false;
+                               }
+                               $this->convertException($e, $path);
+                       } catch (\Exception $e) {
+                               $this->convertException($e, $path);
                        }
                } else {
                        $response = $cachedResponse;
@@ -291,17 +285,15 @@ class DAV extends Common {
        public function filetype($path) {
                try {
                        $response = $this->propfind($path);
-                       $responseType = array();
+                       if ($response === false) {
+                               return false;
+                       }
+                       $responseType = [];
                        if (isset($response["{DAV:}resourcetype"])) {
                                /** @var ResourceType[] $response */
                                $responseType = $response["{DAV:}resourcetype"]->getValue();
                        }
                        return (count($responseType) > 0 and $responseType[0] == "{DAV:}collection") ? 'dir' : 'file';
-               } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404) {
-                               return false;
-                       }
-                       $this->convertException($e, $path);
                } catch (\Exception $e) {
                        $this->convertException($e, $path);
                }
@@ -320,13 +312,7 @@ class DAV extends Common {
                                return true;
                        }
                        // need to get from server
-                       $this->propfind($path);
-                       return true; //no 404 exception
-               } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404) {
-                               return false;
-                       }
-                       $this->convertException($e, $path);
+                       return ($this->propfind($path) !== false);
                } catch (\Exception $e) {
                        $this->convertException($e, $path);
                }
@@ -431,7 +417,10 @@ class DAV extends Common {
                $path = $this->cleanPath($path);
                try {
                        // TODO: cacheable ?
-                       $response = $this->client->propfind($this->encodePath($path), array('{DAV:}quota-available-bytes'));
+                       $response = $this->client->propfind($this->encodePath($path), ['{DAV:}quota-available-bytes']);
+                       if ($response === false) {
+                               return FileInfo::SPACE_UNKNOWN;
+                       }
                        if (isset($response['{DAV:}quota-available-bytes'])) {
                                return (int)$response['{DAV:}quota-available-bytes'];
                        } else {
@@ -457,6 +446,9 @@ class DAV extends Common {
                                $this->client->proppatch($this->encodePath($path), ['{DAV:}lastmodified' => $mtime]);
                                // non-owncloud clients might not have accepted the property, need to recheck it
                                $response = $this->client->propfind($this->encodePath($path), ['{DAV:}getlastmodified'], 0);
+                               if ($response === false) {
+                                       return false;
+                               }
                                if (isset($response['{DAV:}getlastmodified'])) {
                                        $remoteMtime = strtotime($response['{DAV:}getlastmodified']);
                                        if ($remoteMtime !== $mtime) {
@@ -579,15 +571,13 @@ class DAV extends Common {
        public function stat($path) {
                try {
                        $response = $this->propfind($path);
-                       return array(
+                       if ($response === false) {
+                               return [];
+                       }
+                       return [
                                'mtime' => strtotime($response['{DAV:}getlastmodified']),
                                'size' => (int)isset($response['{DAV:}getcontentlength']) ? $response['{DAV:}getcontentlength'] : 0,
-                       );
-               } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404) {
-                               return array();
-                       }
-                       $this->convertException($e, $path);
+                       ];
                } catch (\Exception $e) {
                        $this->convertException($e, $path);
                }
@@ -598,7 +588,10 @@ class DAV extends Common {
        public function getMimeType($path) {
                try {
                        $response = $this->propfind($path);
-                       $responseType = array();
+                       if ($response === false) {
+                               return false;
+                       }
+                       $responseType = [];
                        if (isset($response["{DAV:}resourcetype"])) {
                                /** @var ResourceType[] $response */
                                $responseType = $response["{DAV:}resourcetype"]->getValue();
@@ -611,11 +604,6 @@ class DAV extends Common {
                        } else {
                                return false;
                        }
-               } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404) {
-                               return false;
-                       }
-                       $this->convertException($e, $path);
                } catch (\Exception $e) {
                        $this->convertException($e, $path);
                }
@@ -706,6 +694,9 @@ class DAV extends Common {
                $this->init();
                $path = $this->cleanPath($path);
                $response = $this->propfind($path);
+               if ($response === false) {
+                       return 0;
+               }
                if (isset($response['{http://owncloud.org/ns}permissions'])) {
                        return $this->parsePermissions($response['{http://owncloud.org/ns}permissions']);
                } else if ($this->is_dir($path)) {
@@ -722,6 +713,9 @@ class DAV extends Common {
                $this->init();
                $path = $this->cleanPath($path);
                $response = $this->propfind($path);
+               if ($response === false) {
+                       return null;
+               }
                if (isset($response['{DAV:}getetag'])) {
                        return trim($response['{DAV:}getetag'], '"');
                }
@@ -765,6 +759,13 @@ class DAV extends Common {
                        // force refresh for $path
                        $this->statCache->remove($path);
                        $response = $this->propfind($path);
+                       if ($response === false) {
+                               if ($path === '') {
+                                       // if root is gone it means the storage is not available
+                                       throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage());
+                               }
+                               return false;
+                       }
                        if (isset($response['{DAV:}getetag'])) {
                                $cachedData = $this->getCache()->get($path);
                                $etag = null;
@@ -787,7 +788,7 @@ class DAV extends Common {
                                return $remoteMtime > $time;
                        }
                } catch (ClientHttpException $e) {
-                       if ($e->getHttpStatus() === 404 || $e->getHttpStatus() === 405) {
+                       if ($e->getHttpStatus() === 405) {
                                if ($path === '') {
                                        // if root is gone it means the storage is not available
                                        throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage());