A new exception to suppress exception logs

There are scenarios where an app would have
done the job and the logger would have logged.
But the main task would throw an exception.
Since its logged gracefully we need not throw
exception in the log. This change would help
for the same.

Signed-off-by: Sujith H <sharidasan@owncloud.com>

apps/dav/lib/Connector/Sabre/Directory.php

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

apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php

@@ -26,6 +26,7 @@
 
 namespace OCA\DAV\Connector\Sabre;
 
+use OCP\Files\FileContentNotAllowedException;
 use OCP\ILogger;
 use Sabre\DAV\Exception;
 use Sabre\HTTP\Response;
@@ -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])) {

apps/dav/lib/Connector/Sabre/File.php

@@ -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;
@@ -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;
  }

apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php

@@ -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 {
@@ -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');
+ }
 }

apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php

@@ -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;
@@ -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))],
  ];
  }
 

lib/public/Files/FileContentNotAllowedException.php

@@ -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 {}