From 6d4abdab8f0a25917ecb0eac0851c50ad086641a Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 10 Oct 2016 12:29:06 +0200 Subject: [PATCH] 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. --- lib/private/Files/Storage/DAV.php | 105 +++++++++++++++--------------- 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index ea4bbba274..2f821b218f 100644 --- 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());