From f6e644b43fe3c33ba298ee34a73536c85cc92b4a Mon Sep 17 00:00:00 2001
From: Vincent Petry <pvince81@owncloud.com>
Date: Tue, 20 Jan 2015 19:45:32 +0100
Subject: [PATCH] Catch storage exception in scanner for remote shares
Whenever an exception occurs during scan of a remote share, the share is
checked for availability. If the storage is gone, it will be removed
automatically.
Also, getDirectoryContent() will now skip unavailable storages.
---
apps/files_sharing/ajax/external.php | 58 +++++++++++++++++++--
apps/files_sharing/js/external.js | 2 +-
apps/files_sharing/lib/external/scanner.php | 53 ++++++++++++++++++-
apps/files_sharing/lib/external/storage.php | 56 +++++++++++++-------
lib/private/files/storage/dav.php | 41 ++++++++++++++-
lib/private/files/view.php | 17 +++++-
6 files changed, 200 insertions(+), 27 deletions(-)
diff --git a/apps/files_sharing/ajax/external.php b/apps/files_sharing/ajax/external.php
index 1a709eda07..3d6d589aff 100644
--- a/apps/files_sharing/ajax/external.php
+++ b/apps/files_sharing/ajax/external.php
@@ -42,20 +42,70 @@ $name = OCP\Files::buildNotExistingFileName('/', $name);
// check for ssl cert
if (substr($remote, 0, 5) === 'https' and !OC_Util::getUrlContent($remote)) {
- \OCP\JSON::error(array('data' => array('message' => $l->t("Invalid or untrusted SSL certificate"))));
+ \OCP\JSON::error(array('data' => array('message' => $l->t('Invalid or untrusted SSL certificate'))));
exit;
} else {
$mount = $externalManager->addShare($remote, $token, $password, $name, $owner, true);
+
/**
* @var \OCA\Files_Sharing\External\Storage $storage
*/
$storage = $mount->getStorage();
+ try {
+ // check if storage exists
+ $storage->checkStorageAvailability();
+ } catch (\OCP\Files\StorageInvalidException $e) {
+ // note: checkStorageAvailability will already remove the invalid share
+ \OCP\Util::writeLog(
+ 'files_sharing',
+ 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(),
+ \OCP\Util::DEBUG
+ );
+ \OCP\JSON::error(
+ array(
+ 'data' => array(
+ 'message' => $l->t('Could not authenticate to remote share, password might be wrong')
+ )
+ )
+ );
+ exit();
+ } catch (\Exception $e) {
+ \OCP\Util::writeLog(
+ 'files_sharing',
+ 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(),
+ \OCP\Util::DEBUG
+ );
+ $externalManager->removeShare($mount->getMountPoint());
+ \OCP\JSON::error(array('data' => array('message' => $l->t('Storage not valid'))));
+ throw new \OCP\Files\StorageNotAvailableException(get_class($e).': '.$e->getMessage());
+ }
$result = $storage->file_exists('');
if ($result) {
- $storage->getScanner()->scanAll();
- \OCP\JSON::success();
+ try {
+ $storage->getScanner()->scanAll();
+ \OCP\JSON::success();
+ } catch (\OCP\Files\StorageInvalidException $e) {
+ \OCP\Util::writeLog(
+ 'files_sharing',
+ 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(),
+ \OCP\Util::DEBUG
+ );
+ \OCP\JSON::error(array('data' => array('message' => $l->t('Storage not valid'))));
+ } catch (\Exception $e) {
+ \OCP\Util::writeLog(
+ 'files_sharing',
+ 'Invalid remote storage: ' . get_class($e) . ': ' . $e->getMessage(),
+ \OCP\Util::DEBUG
+ );
+ \OCP\JSON::error(array('data' => array('message' => $l->t('Couldn\'t add remote share'))));
+ }
} else {
$externalManager->removeShare($mount->getMountPoint());
- \OCP\JSON::error(array('data' => array('message' => $l->t("Couldn't add remote share"))));
+ \OCP\Util::writeLog(
+ 'files_sharing',
+ 'Couldn\'t add remote share',
+ \OCP\Util::DEBUG
+ );
+ \OCP\JSON::error(array('data' => array('message' => $l->t('Couldn\'t add remote share'))));
}
}
diff --git a/apps/files_sharing/js/external.js b/apps/files_sharing/js/external.js
index 31407f28ff..85a9159fa6 100644
--- a/apps/files_sharing/js/external.js
+++ b/apps/files_sharing/js/external.js
@@ -95,7 +95,7 @@
name: share.name,
password: password}, function(result) {
if (result.status === 'error') {
- OC.Notification.show(result.data.message);
+ OC.Notification.showTemporary(result.data.message);
} else {
fileList.reload();
}
diff --git a/apps/files_sharing/lib/external/scanner.php b/apps/files_sharing/lib/external/scanner.php
index 4e61e0c4cc..b45a8942e9 100644
--- a/apps/files_sharing/lib/external/scanner.php
+++ b/apps/files_sharing/lib/external/scanner.php
@@ -8,6 +8,11 @@
namespace OCA\Files_Sharing\External;
+use OC\ForbiddenException;
+use OCP\Files\NotFoundException;
+use OCP\Files\StorageInvalidException;
+use OCP\Files\StorageNotAvailableException;
+
class Scanner extends \OC\Files\Cache\Scanner {
/**
* @var \OCA\Files_Sharing\External\Storage
@@ -18,12 +23,56 @@ class Scanner extends \OC\Files\Cache\Scanner {
$this->scanAll();
}
+ /**
+ * Scan a single file and store it in the cache.
+ * If an exception happened while accessing the external storage,
+ * the storage will be checked for availability and removed
+ * if it is not available any more.
+ *
+ * @param string $file file to scan
+ * @param int $reuseExisting
+ * @return array an array of metadata of the scanned file
+ */
+ public function scanFile($file, $reuseExisting = 0) {
+ try {
+ return parent::scanFile($file, $reuseExisting);
+ } catch (ForbiddenException $e) {
+ $this->storage->checkStorageAvailability();
+ } catch (NotFoundException $e) {
+ // if the storage isn't found, the call to
+ // checkStorageAvailable() will verify it and remove it
+ // if appropriate
+ $this->storage->checkStorageAvailability();
+ } catch (StorageInvalidException $e) {
+ $this->storage->checkStorageAvailability();
+ } catch (StorageNotAvailableException $e) {
+ $this->storage->checkStorageAvailability();
+ }
+ }
+
+ /**
+ * Checks the remote share for changes.
+ * If changes are available, scan them and update
+ * the cache.
+ */
public function scanAll() {
- $data = $this->storage->getShareInfo();
+ try {
+ $data = $this->storage->getShareInfo();
+ } catch (\Exception $e) {
+ $this->storage->checkStorageAvailability();
+ throw new \Exception(
+ 'Error while scanning remote share: "' .
+ $this->storage->getRemote() . '" ' .
+ $e->getMessage()
+ );
+ }
if ($data['status'] === 'success') {
$this->addResult($data['data'], '');
} else {
- throw new \Exception('Error while scanning remote share');
+ throw new \Exception(
+ 'Error while scanning remote share: "' .
+ $this->storage->getRemote() . '"'
+ );
}
}
diff --git a/apps/files_sharing/lib/external/storage.php b/apps/files_sharing/lib/external/storage.php
index 0d41176e45..d5b1b1df44 100644
--- a/apps/files_sharing/lib/external/storage.php
+++ b/apps/files_sharing/lib/external/storage.php
@@ -142,27 +142,47 @@ class Storage extends DAV implements ISharedStorage {
$this->updateChecked = true;
try {
return parent::hasUpdated('', $time);
+ } catch (StorageInvalidException $e) {
+ // check if it needs to be removed
+ $this->checkStorageAvailability();
+ throw $e;
} catch (StorageNotAvailableException $e) {
- // see if we can find out why the share is unavailable\
- try {
- $this->getShareInfo();
- } catch (NotFoundException $shareException) {
- // a 404 can either mean that the share no longer exists or there is no ownCloud on the remote
- if ($this->testRemote()) {
- // valid ownCloud instance means that the public share no longer exists
- // since this is permanent (re-sharing the file will create a new token)
- // we remove the invalid storage
- $this->manager->removeShare($this->mountPoint);
- $this->manager->getMountManager()->removeMount($this->mountPoint);
- throw new StorageInvalidException();
- } else {
- // ownCloud instance is gone, likely to be a temporary server configuration error
- throw $e;
- }
- } catch (\Exception $shareException) {
- // todo, maybe handle 403 better and ask the user for a new password
+ // check if it needs to be removed or just temp unavailable
+ $this->checkStorageAvailability();
+ throw $e;
+ }
+ }
+
+ /**
+ * Check whether this storage is permanently or temporarily
+ * unavailable
+ *
+ * @throws \OCP\Files\StorageNotAvailableException
+ * @throws \OCP\Files\StorageInvalidException
+ */
+ public function checkStorageAvailability() {
+ // see if we can find out why the share is unavailable
+ try {
+ $this->getShareInfo();
+ } catch (NotFoundException $e) {
+ // a 404 can either mean that the share no longer exists or there is no ownCloud on the remote
+ if ($this->testRemote()) {
+ // valid ownCloud instance means that the public share no longer exists
+ // since this is permanent (re-sharing the file will create a new token)
+ // we remove the invalid storage
+ $this->manager->removeShare($this->mountPoint);
+ $this->manager->getMountManager()->removeMount($this->mountPoint);
+ throw new StorageInvalidException();
+ } else {
+ // ownCloud instance is gone, likely to be a temporary server configuration error
throw $e;
}
+ } catch (ForbiddenException $e) {
+ // auth error, remove share for now (provide a dialog in the future)
+ $this->manager->removeShare($this->mountPoint);
+ $this->manager->getMountManager()->removeMount($this->mountPoint);
+ throw new StorageInvalidException();
+ } catch (\Exception $e) {
throw $e;
}
}
diff --git a/lib/private/files/storage/dav.php b/lib/private/files/storage/dav.php
index 355148de37..2d4916df08 100644
--- a/lib/private/files/storage/dav.php
+++ b/lib/private/files/storage/dav.php
@@ -8,6 +8,7 @@
namespace OC\Files\Storage;
+use OCP\Files\StorageInvalidException;
use OCP\Files\StorageNotAvailableException;
use Sabre\DAV\Exception;
@@ -125,6 +126,8 @@ class DAV extends \OC\Files\Storage\Common {
return opendir('fakedir://' . $id);
} catch (Exception\NotFound $e) {
return false;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -144,6 +147,8 @@ class DAV extends \OC\Files\Storage\Common {
return (count($responseType) > 0 and $responseType[0] == "{DAV:}collection") ? 'dir' : 'file';
} catch (Exception\NotFound $e) {
return false;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -159,6 +164,8 @@ class DAV extends \OC\Files\Storage\Common {
return true; //no 404 exception
} catch (Exception\NotFound $e) {
return false;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -275,6 +282,8 @@ class DAV extends \OC\Files\Storage\Common {
$this->client->proppatch($this->encodePath($path), array('{DAV:}lastmodified' => $mtime));
} catch (Exception\NotImplemented $e) {
return false;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -326,6 +335,8 @@ class DAV extends \OC\Files\Storage\Common {
$this->removeCachedFile($path1);
$this->removeCachedFile($path2);
return true;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -341,6 +352,8 @@ class DAV extends \OC\Files\Storage\Common {
$this->client->request('COPY', $path1, null, array('Destination' => $path2));
$this->removeCachedFile($path2);
return true;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -359,6 +372,8 @@ class DAV extends \OC\Files\Storage\Common {
);
} catch (Exception\NotFound $e) {
return array();
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -383,6 +398,8 @@ class DAV extends \OC\Files\Storage\Common {
} else {
return false;
}
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -423,6 +440,8 @@ class DAV extends \OC\Files\Storage\Common {
try {
$response = $this->client->request($method, $this->encodePath($path), $body);
return $response['statusCode'] == $expected;
+ } catch (\Sabre\DAV\Exception $e) {
+ $this->convertSabreException($e);
} catch (\Exception $e) {
// TODO: log for now, but in the future need to wrap/rethrow exception
\OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
@@ -529,8 +548,28 @@ class DAV extends \OC\Files\Storage\Common {
} catch (Exception\NotFound $e) {
return false;
} catch (Exception $e) {
- throw new StorageNotAvailableException(get_class($e).": ".$e->getMessage());
+ $this->convertSabreException($e);
}
}
+
+ /**
+ * Convert sabre DAV exception to a storage exception,
+ * then throw it
+ *
+ * @param \Sabre\Dav\Exception $e sabre exception
+ * @throws StorageInvalidException if the storage is invalid, for example
+ * when the authentication expired or is invalid
+ * @throws StorageNotAvailableException if the storage is not available,
+ * which might be temporary
+ */
+ private function convertSabreException(\Sabre\Dav\Exception $e) {
+ \OCP\Util::writeLog('files_external', $e->getMessage(), \OCP\Util::ERROR);
+ if ($e instanceof \Sabre\DAV\Exception\NotAuthenticated) {
+ // either password was changed or was invalid all along
+ throw new StorageInvalidException(get_class($e).': '.$e->getMessage());
+ }
+
+ throw new StorageNotAvailableException(get_class($e).': '.$e->getMessage());
+ }
}
diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 76b7d34e75..fd2ba11d30 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -1037,7 +1037,22 @@ class View {
if ($subCache->getStatus('') === Cache\Cache::NOT_FOUND) {
$subScanner = $subStorage->getScanner('');
- $subScanner->scanFile('');
+ try {
+ $subScanner->scanFile('');
+ } catch (\OCP\Files\StorageNotAvailableException $e) {
+ continue;
+ } catch (\OCP\Files\StorageInvalidException $e) {
+ continue;
+ } catch (\Exception $e) {
+ // sometimes when the storage is not available it can be any exception
+ \OCP\Util::writeLog(
+ 'core',
+ 'Exception while scanning storage "' . $subStorage->getId() . '": ' .
+ get_class($e) . ': ' . $e->getMessage(),
+ \OC_Log::ERROR
+ );
+ continue;
+ }
}
$rootEntry = $subCache->get('');
--
GitLab