Skip to content

Commit

Permalink
Merge pull request #31748 from owncloud/bugfix/exception-handling-in-…
Browse files Browse the repository at this point in the history
…preview-plugin

[stable10] Exceptions need to properly be handled in the preview plugin
  • Loading branch information
Vincent Petry authored Jun 14, 2018
2 parents 174234c + f33c953 commit dc2422b
Show file tree
Hide file tree
Showing 3 changed files with 214 additions and 32 deletions.
89 changes: 58 additions & 31 deletions apps/dav/lib/Files/PreviewPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

/**
Expand All @@ -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) {

Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
155 changes: 155 additions & 0 deletions apps/dav/tests/unit/Files/PreviewPluginTest.php
Original file line number Diff line number Diff line change
@@ -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));
}
}

0 comments on commit dc2422b

Please sign in to comment.