From f6e644b43fe3c33ba298ee34a73536c85cc92b4a Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 20 Jan 2015 19:45:32 +0100 Subject: [PATCH 1/3] 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(''); From 87ce64c24e4320c2dca9327010e3048141df3686 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 26 Jan 2015 15:51:31 +0100 Subject: [PATCH 2/3] Replace OC_Log::ERROR with OCP\Util::ERROR --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index fd2ba11d30..001f17cdb5 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1049,7 +1049,7 @@ class View { 'core', 'Exception while scanning storage "' . $subStorage->getId() . '": ' . get_class($e) . ': ' . $e->getMessage(), - \OC_Log::ERROR + \OCP\Util::ERROR ); continue; } From 5376b0b123873269a92d9aa45f6233c48c2720df Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 26 Jan 2015 16:07:28 +0100 Subject: [PATCH 3/3] Add back exit() --- apps/files_sharing/ajax/external.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_sharing/ajax/external.php b/apps/files_sharing/ajax/external.php index 3d6d589aff..2813faf607 100644 --- a/apps/files_sharing/ajax/external.php +++ b/apps/files_sharing/ajax/external.php @@ -77,7 +77,7 @@ if (substr($remote, 0, 5) === 'https' and !OC_Util::getUrlContent($remote)) { ); $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()); + exit(); } $result = $storage->file_exists(''); if ($result) {