[stable10] Exceptions need to properly be handled in the preview plugin

apps/dav/lib/Files/PreviewPlugin.php

@@ -21,9 +21,17 @@
 
 namespace OCA\DAV\Files;
 
+use OCA\DAV\Connector\Sabre\Exception\FileLocked;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Encryption\Exceptions\GenericEncryptionException;
+use OCP\Files\ForbiddenException;
 use OCP\Files\IPreviewNode;
+use OCP\Files\StorageNotAvailableException;
 use OCP\ILogger;
+use OCP\Lock\LockedException;
+use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\Exception\ServiceUnavailable;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
@@ -33,11 +41,16 @@ class PreviewPlugin extends ServerPlugin {
 
 	/** @var Server */
 	protected $server;
-	/** @var ILogger */
-	private $logger;
+	/** @var ITimeFactory */
+	private $timeFactory;
 
-	public function __construct(ILogger $logger) {
-		$this->logger = $logger;
+	/**
+	 * PreviewPlugin constructor.
+	 *
+	 * @param ITimeFactory $timeFactory
+	 */
+	public function __construct(ITimeFactory $timeFactory) {
+		$this->timeFactory = $timeFactory;
 	}
 
 	/**
@@ -62,6 +75,9 @@ function initialize(Server $server) {
 	 * @throws NotFound
 	 * @throws \Sabre\DAVACL\Exception\NeedPrivileges
 	 * @throws \Sabre\DAV\Exception\NotAuthenticated
+	 * @throws Forbidden
+	 * @throws FileLocked
+	 * @throws ServiceUnavailable
 	 */
 	function httpGet(RequestInterface $request, ResponseInterface $response) {
 
@@ -87,34 +103,45 @@ function httpGet(RequestInterface $request, ResponseInterface $response) {
 			$aclPlugin->checkPrivileges($path, '{DAV:}read');
 		}
 
-		if ($image = $fileNode->getThumbnail($queryParams)) {
-			if ($image === null || !$image->valid()) {
-				throw new NotFound();
+		try {
+			if ($image = $fileNode->getThumbnail($queryParams)) {
+				if ($image === null || !$image->valid()) {
+					throw new NotFound();
+				}
+				$type = $image->mimeType();
+				if (!in_array($type, ['image/png', 'image/jpeg', 'image/gif'])) {
+					$type = 'application/octet-stream';
+				}
+
+				// Enable output buffering
+				ob_start();
+				// Capture the output
+				$image->show();
+				$imageData = ob_get_contents();
+				// Clear the output buffer
+				ob_end_clean();
+
+				$response->setHeader('Content-Type', $type);
+				$response->setHeader('Content-Disposition', 'attachment');
+				// cache 24h
+				$response->setHeader('Cache-Control', 'max-age=86400, must-revalidate');
+				$response->setHeader('Expires', gmdate("D, d M Y H:i:s", $this->timeFactory->getTime() + 86400) . " GMT");
+
+				$response->setStatus(200);
+				$response->setBody($imageData);
+
+				// Returning false to break the event chain
+				return false;
 			}
-			$type = $image->mimeType();
-			if (!in_array($type, ['image/png', 'image/jpeg', 'image/gif'])) {
-				$type = 'application/octet-stream';
-			}
-
-			// Enable output buffering
-			ob_start();
-			// Capture the output
-			$image->show();
-			$imageData = ob_get_contents();
-			// Clear the output buffer
-			ob_end_clean();
-
-			$response->setHeader('Content-Type', $type);
-			$response->setHeader('Content-Disposition', 'attachment');
-			// cache 24h
-			$response->setHeader('Cache-Control', 'max-age=86400, must-revalidate');
-			$response->setHeader('Expires', gmdate ("D, d M Y H:i:s", time() + 86400) . " GMT");
-
-			$response->setStatus(200);
-			$response->setBody($imageData);
-
-			// Returning false to break the event chain
-			return false;
+		} catch (GenericEncryptionException $e) {
+			// returning 403 because some apps stops syncing if 503 is returned.
+			throw new Forbidden('Encryption not ready: ' . $e->getMessage());
+		} catch (StorageNotAvailableException $e) {
+			throw new ServiceUnavailable('Failed to open file: ' . $e->getMessage());
+		} catch (ForbiddenException $ex) {
+			throw new Forbidden($ex->getMessage(), $ex->getRetry());
+		} catch (LockedException $e) {
+			throw new FileLocked($e->getMessage(), $e->getCode(), $e);
 		}
 		// TODO: add forceIcon handling .... if still needed
 		throw new NotFound();

apps/dav/lib/Server.php

@@ -186,7 +186,7 @@ public function __construct(IRequest $request, $baseUri) {
 			$this->server->addPlugin(new BrowserErrorPagePlugin());
 		}
 
-		$this->server->addPlugin(new PreviewPlugin($logger));
+		$this->server->addPlugin(new PreviewPlugin(\OC::$server->getTimeFactory()));
 		// wait with registering these until auth is handled and the filesystem is setup
 		$this->server->on('beforeMethod', function () use ($root) {
 			// custom properties plugin must be the last one

apps/dav/tests/unit/Files/PreviewPluginTest.php

@@ -0,0 +1,155 @@
+<?php
+/**
+ * @author Thomas Müller <[email protected]>
+ * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ */
+
+
+namespace OCA\DAV\Tests\Unit\Files;
+
+
+use OCA\DAV\Connector\Sabre\Exception\FileLocked;
+use OCA\DAV\Files\IFileNode;
+use OCA\DAV\Files\PreviewPlugin;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Encryption\Exceptions\GenericEncryptionException;
+use OCP\Files\ForbiddenException;
+use OCP\Files\IPreviewNode;
+use OCP\Files\StorageNotAvailableException;
+use OCP\IImage;
+use OCP\ILogger;
+use OCP\Lock\LockedException;
+use Sabre\DAV\Exception\Forbidden;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\Exception\ServiceUnavailable;
+use Sabre\DAV\Server;
+use Sabre\DAV\Tree;
+use Sabre\HTTP\RequestInterface;
+use Sabre\HTTP\ResponseInterface;
+use Test\TestCase;
+
+class PreviewPluginTest extends TestCase {
+
+	/** @var RequestInterface | \PHPUnit_Framework_MockObject_MockObject */
+	private $request;
+	/** @var IPreviewNode | \PHPUnit_Framework_MockObject_MockObject */
+	private $previewNode;
+	/** @var PreviewPlugin */
+	private $plugin;
+	/** @var ResponseInterface| \PHPUnit_Framework_MockObject_MockObject */
+	private $response;
+
+	public function setUp() {
+		parent::setUp();
+
+		/** @var ITimeFactory | \PHPUnit_Framework_MockObject_MockObject $timeFactory */
+		$timeFactory = $this->createMock(ITimeFactory::class);
+		$timeFactory->method('getTime')->willReturn(1234567);
+		$this->plugin = new PreviewPlugin($timeFactory);
+
+		$this->previewNode = $this->createMock(IPreviewNode::class);
+
+		/** @var IFileNode | \PHPUnit_Framework_MockObject_MockObject $node */
+		$node = $this->createMock(IFileNode::class);
+		$node->method('getNode')->willReturn($this->previewNode);
+
+		/** @var Tree | \PHPUnit_Framework_MockObject_MockObject $tree */
+		$tree = $this->createMock(Tree::class);
+		$tree->method('getNodeForPath')->willReturn($node);
+
+		/** @var Server | \PHPUnit_Framework_MockObject_MockObject $server */
+		$server = $this->createMock(Server::class);
+		$server->tree = $tree;
+
+		$this->request = $this->createMock(RequestInterface::class);
+		/** @var ResponseInterface | \PHPUnit_Framework_MockObject_MockObject $response */
+		$this->response = $this->createMock(ResponseInterface::class);
+		$this->plugin->initialize($server);
+	}
+
+	public function testPreviewParamNotSet() {
+		$this->request
+			->expects($this->once())
+			->method('getQueryParameters')
+			->will($this->returnValue([]));
+		$this->assertTrue($this->plugin->httpGet($this->request, $this->response));
+	}
+
+	/**
+	 * @param $expectedExceptionClass
+	 * @param $exception
+	 * @throws Forbidden
+	 * @throws \OCA\DAV\Connector\Sabre\Exception\FileLocked
+	 * @throws \Sabre\DAV\Exception\NotAuthenticated
+	 * @throws \Sabre\DAV\Exception\NotFound
+	 * @throws \Sabre\DAV\Exception\ServiceUnavailable
+	 * @dataProvider providesExceptions
+	 */
+	public function testPreviewWithExceptions($expectedExceptionClass, $exception) {
+
+		$this->previewNode->method('getThumbnail')->willThrowException($exception);
+
+		$this->request->method('getQueryParameters')->willReturn([
+			'preview' => '1'
+		]);
+
+		$this->expectException($expectedExceptionClass);
+		$this->plugin->httpGet($this->request, $this->response);
+	}
+
+	public function providesExceptions() {
+		return [
+			[Forbidden::class, new GenericEncryptionException()],
+			[ServiceUnavailable::class, new StorageNotAvailableException()],
+			[Forbidden::class, new ForbiddenException('', false)],
+			[FileLocked::class, new LockedException('')]
+		];
+	}
+
+	public function testPreviewNoImage() {
+
+		$this->previewNode->method('getThumbnail')->willReturn(null);
+
+		$this->request->method('getQueryParameters')->willReturn([
+			'preview' => '1'
+		]);
+
+		$this->expectException(NotFound::class);
+		$this->plugin->httpGet($this->request, $this->response);
+	}
+
+	public function testPreviewCreatesImage() {
+		$image = $this->createMock(IImage::class);
+		$image->method('valid')->willReturn(true);
+		$this->previewNode->method('getThumbnail')->willReturn($image);
+
+		$this->request->method('getQueryParameters')->willReturn([
+			'preview' => '1'
+		]);
+
+		$this->response->method('setStatus')->with(200);
+		$this->response->method('setBody')->with('');
+		$this->response->expects($this->exactly(4))
+			->method('setHeader')->withConsecutive(
+			['Content-Type', 'application/octet-stream'],
+			['Content-Disposition', 'attachment'],
+			['Cache-Control', 'max-age=86400, must-revalidate'],
+			['Expires', gmdate("D, d M Y H:i:s", 1234567 + 86400) . " GMT"]
+		);
+
+		$this->assertFalse($this->plugin->httpGet($this->request, $this->response));
+	}
+}