Skip to content

Commit

Permalink
Merge pull request #30771 from owncloud/suppress-exception-log
Browse files Browse the repository at this point in the history
A new exception to suppress exception logs
  • Loading branch information
sharidas authored Apr 3, 2018
2 parents ace89ed + f38b692 commit c59a9d1
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 5 deletions.
7 changes: 6 additions & 1 deletion apps/dav/lib/Connector/Sabre/Directory.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
use OCA\DAV\Upload\FutureFile;
use OCA\DAV\Upload\FutureFileZsync;
use OCP\Files\FileContentNotAllowedException;
use OCP\Files\ForbiddenException;
use OCP\Files\InvalidPathException;
use OCP\Files\StorageNotAvailableException;
Expand Down Expand Up @@ -175,7 +176,11 @@ public function createFile($name, $data = null) {
} catch (InvalidPathException $ex) {
throw new InvalidPath($ex->getMessage());
} catch (ForbiddenException $ex) {
throw new Forbidden($ex->getMessage(), $ex->getRetry());
if ($ex->getPrevious() instanceof FileContentNotAllowedException) {
throw new FileContentNotAllowedException($ex->getMessage(), $ex->getRetry(), $ex);
} else {
throw new Forbidden($ex->getMessage(), $ex->getRetry());
}
} catch (LockedException $e) {
throw new FileLocked($e->getMessage(), $e->getCode(), $e);
}
Expand Down
6 changes: 6 additions & 0 deletions apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

namespace OCA\DAV\Connector\Sabre;

use OCP\Files\FileContentNotAllowedException;
use OCP\ILogger;
use Sabre\DAV\Exception;
use Sabre\HTTP\Response;
Expand Down Expand Up @@ -87,6 +88,11 @@ public function initialize(\Sabre\DAV\Server $server) {
*
*/
public function logException(\Exception $ex) {
if ($ex->getPrevious() instanceof FileContentNotAllowedException) {
//Don't log because its already been logged may be by different
//app or so.
return null;
}
$exceptionClass = get_class($ex);
$level = \OCP\Util::FATAL;
if (isset($this->nonFatalExceptions[$exceptionClass])) {
Expand Down
5 changes: 5 additions & 0 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use OCP\Encryption\Exceptions\GenericEncryptionException;
use OCP\Events\EventEmitterTrait;
use OCP\Files\EntityTooLargeException;
use OCP\Files\FileContentNotAllowedException;
use OCP\Files\ForbiddenException;
use OCP\Files\InvalidContentException;
use OCP\Files\InvalidPathException;
Expand Down Expand Up @@ -600,6 +601,10 @@ private function needsPartFile($storage) {
* @throws \Sabre\DAV\Exception
*/
private function convertToSabreException(\Exception $e) {
if ($e instanceof FileContentNotAllowedException) {
// the file content is not permitted
throw new FileContentNotAllowedException($e->getMessage(), $e->getRetry(), $e);
}
if ($e instanceof \Sabre\DAV\Exception) {
throw $e;
}
Expand Down
17 changes: 17 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

use OC\Files\FileInfo;
use OCA\DAV\Connector\Sabre\Directory;
use OCP\Files\FileContentNotAllowedException;
use OCP\Files\ForbiddenException;

class TestDoubleFileView extends \OC\Files\View {
Expand Down Expand Up @@ -431,4 +432,20 @@ public function testFailingMove() {

$targetNode->moveInto(basename($destination), $source, $sourceNode);
}

/**
* A test to throw ExcludeForbiddenException
*
* @expectedException \OCP\Files\FileContentNotAllowedException
* @expectedExceptionMessage The message already logged
*/
public function testFailCreateFile() {
//$this->invokePrivate();
$previous = new FileContentNotAllowedException('The message already logged', false);
$this->view->expects($this->any())
->method('isCreatable')
->willThrowException(new FileContentNotAllowedException('The message already logged', false, $previous));
$dir = $this->getDir();
$dir->createFile('foobar.txt', 'hello foo bar');
}
}
16 changes: 12 additions & 4 deletions apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@

use OC\Log;
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType;
use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin as PluginToTest;
use OCP\Files\ExcludeForbiddenException;
use OCP\Files\FileContentNotAllowedException;
use PHPUnit_Framework_MockObject_MockObject;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\Server;
Expand Down Expand Up @@ -67,16 +70,21 @@ private function init() {
*/
public function testLogging($expectedLogLevel, $expectedMessage, $exception) {
$this->init();
$this->plugin->logException($exception);
$result = $this->plugin->logException($exception);

$this->assertEquals($expectedLogLevel, $this->logger->level);
$this->assertStringStartsWith('Exception: {"Message":"' . $expectedMessage, $this->logger->message);
if ($exception instanceof FileContentNotAllowedException) {
$this->assertNull($result);
} else {
$this->assertEquals($expectedLogLevel, $this->logger->level);
$this->assertStringStartsWith('Exception: {"Message":"' . $expectedMessage, $this->logger->message);
}
}

public function providesExceptions() {
return [
[0, 'HTTP\/1.1 404 Not Found', new NotFound()],
[4, 'HTTP\/1.1 400 This path leads to nowhere', new InvalidPath('This path leads to nowhere')]
[4, 'HTTP\/1.1 400 This path leads to nowhere', new InvalidPath('This path leads to nowhere')],
[0, '', new FileContentNotAllowedException("Testing", 0, new FileContentNotAllowedException("pervious exception", 0))],
];
}

Expand Down
30 changes: 30 additions & 0 deletions lib/public/Files/FileContentNotAllowedException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
/**
* @author Sujith Haridasan <sharidasan@owncloud.com>
*
* @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/>
*
*/

// use OCP namespace for all classes that are considered public.
// This means that they should be used by apps instead of the internal ownCloud classes
namespace OCP\Files;

/**
* FileContentNotAllowed to suppress logging.
* @since 10.0.8
*/
class FileContentNotAllowedException extends ForbiddenException {}

0 comments on commit c59a9d1

Please sign in to comment.