Merge pull request #13525 from owncloud/s2s-fixscanfileforbrokenstorage

Catch storage exception in scanner for remote shares

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'))));
+		exit();
+	}
 	$result = $storage->file_exists('');
 	if ($result) {
-		$storage->getScanner()->scanAll();
+		try {
-		\OCP\JSON::success();
+			$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'))));
 	}
 }

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();
 							}

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() . '"'
+			);
 		}
 	}
 

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\
+			// check if it needs to be removed or just temp unavailable
-			try {
+			$this->checkStorageAvailability();
-				$this->getShareInfo();
+			throw $e;
-			} 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)
+	 * Check whether this storage is permanently or temporarily
-					// we remove the invalid storage
+	 * unavailable
-					$this->manager->removeShare($this->mountPoint);
+	 *
-					$this->manager->getMountManager()->removeMount($this->mountPoint);
+	 * @throws \OCP\Files\StorageNotAvailableException
-					throw new StorageInvalidException();
+	 * @throws \OCP\Files\StorageInvalidException
-				} else {
+	 */
-					// ownCloud instance is gone, likely to be a temporary server configuration error
+	public function checkStorageAvailability() {
-					throw $e;
+		// see if we can find out why the share is unavailable
-				}
+		try {
-			} catch (\Exception $shareException) {
+			$this->getShareInfo();
-				// todo, maybe handle 403 better and ask the user for a new password
+		} 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;
 		}
 	}

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());
+	}
 }
 

lib/private/files/view.php

@@ -1044,7 +1044,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(),
+								\OCP\Util::ERROR
+							);
+							continue;
+						}
 					}
 
 					$rootEntry = $subCache->get('');