Merge pull request #518 from owncloud/fix/user-message-av-broken

fix: in case of missconfiguration a non-tech message is presented to …
owncloud · Nov 18, 2022 · 871ad29 · 871ad29
2 parents f04d39f + 3041de6
commit 871ad29
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 39 deletions.
diff --git a/lib/AvirWrapper.php b/lib/AvirWrapper.php
@@ -23,7 +23,6 @@
 use Icewind\Streams\CallbackWrapper;
 
 class AvirWrapper extends Wrapper {
-	public const AV_EXCEPTION_MESSAGE = 'Either the ownCloud antivirus app is misconfigured or the external antivirus service is not accessible. %s';
 
 	/**
 	 * Modes that are used for writing
@@ -89,9 +88,8 @@ public function file_put_contents($path, $data) {
 
 			return parent::file_put_contents($path, $data);
 		} catch (InitException $e) {
-			$message = \sprintf(self::AV_EXCEPTION_MESSAGE, $e->getMessage());
-			$this->logger->warning($message, ['app' => 'files_antivirus']);
-			throw new ForbiddenException($message, true, $e);
+			$this->logger->logException($e, ['app' => 'files_antivirus']);
+			throw new ForbiddenException(L10n::getEnduserNotification($this->l10n), true, $e);
 		} catch (FileContentNotAllowedException $e) {
 			throw new ForbiddenException($e->getMessage(), false, $e);
 		} catch (\Exception $e) {
@@ -131,9 +129,8 @@ function () use ($scanner, $path) {
 					}
 				);
 			} catch (InitException $e) {
-				$message = \sprintf(self::AV_EXCEPTION_MESSAGE, $e->getMessage());
-				$this->logger->warning($message, ['app' => 'files_antivirus']);
-				throw new ForbiddenException($message, false, $e);
+				$this->logger->logException($e, ['app' => 'files_antivirus']);
+				throw new ForbiddenException(L10n::getEnduserNotification($this->l10n), true, $e);
 			} catch (\Exception $e) {
 				$message = 	\implode(' ', [ __CLASS__, __METHOD__, $e->getMessage()]);
 				$this->logger->warning($message, ['app' => 'files_antivirus']);

diff --git a/lib/L10n.php b/lib/L10n.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace OCA\Files_Antivirus;
+
+use OCP\IL10N;
+
+class L10n {
+	public static function getEnduserNotification(IL10N $n) {
+		return $n->t('Either the ownCloud antivirus app is misconfigured or the external antivirus service is not accessible. Please reach out to your system administrator!');
+	}
+}
diff --git a/lib/Scanner/ICAPScanner.php b/lib/Scanner/ICAPScanner.php
@@ -4,6 +4,7 @@
 
 use OCA\Files_Antivirus\AppConfig;
 use OCA\Files_Antivirus\IScannable;
+use OCA\Files_Antivirus\L10n;
 use OCA\Files_Antivirus\Status;
 use OCP\IL10N;
 use OCP\ILogger;
@@ -18,6 +19,7 @@ class ICAPScanner implements IScanner {
 	private string $virusHeader;
 	private int $sizeLimit;
 	private string $filename;
+	private ILogger $logger;
 
 	public function __construct(AppConfig $config, ILogger $logger, IL10N $l10n) {
 		$this->host = $config->getAvHost();
@@ -26,6 +28,7 @@ public function __construct(AppConfig $config, ILogger $logger, IL10N $l10n) {
 		$this->virusHeader = $config->getAvResponseHeader();
 		$this->sizeLimit = $config->getAvMaxFileSize();
 		$this->l10n = $l10n;
+		$this->logger = $logger;
 	}
 
 	public function initScanner(string $fileName): void {
@@ -50,48 +53,50 @@ public function onAsyncData($data): void {
 		}
 	}
 
-	/**
-	 * @throws InitException
-	 */
 	public function completeAsyncScan(): Status {
 		if ($this->data === '') {
 			return Status::create(Status::SCANRESULT_CLEAN);
 		}
 
 		$icapHeaders = $this->getICAPHeaders();
 
-		$c = new ICAPClient($this->host, $this->port);
-		if ($this->usesReqMod()) {
-			$response = $c->reqmod(
-				$this->reqService,
-				$this->buildReqModBody(),
-				$icapHeaders
-			);
-		} else {
-			$response = $c->respmod(
-				$this->reqService,
-				$this->buildRespModBody(),
-				$icapHeaders
-			);
-		}
-		$code = $response['protocol']['code'] ?? 500;
-		if ($code === 100 || $code === 200 || $code === 204) {
-			// c-icap/clamav reports this header
-			$virus = $response['headers'][$this->virusHeader] ?? false;
-			if ($virus) {
-				return Status::create(Status::SCANRESULT_INFECTED, $virus);
+		try {
+			$c = new ICAPClient($this->host, $this->port);
+			if ($this->usesReqMod()) {
+				$response = $c->reqmod(
+					$this->reqService,
+					$this->buildReqModBody(),
+					$icapHeaders
+				);
+			} else {
+				$response = $c->respmod(
+					$this->reqService,
+					$this->buildRespModBody(),
+					$icapHeaders
+				);
 			}
-
-			// kaspersky(pre-2020 product editions) and McAfee handling
-			$respHeader = $response['body']['res-hdr']['HTTP_STATUS'] ?? '';
-			if (\strpos($respHeader, '403 Forbidden') !== false || \strpos($respHeader, '403 VirusFound') !== false) {
-				$message = $this->l10n->t('A malware or virus was detected, your upload was denied. In doubt or for details please contact your system administrator.');
-				return Status::create(Status::SCANRESULT_INFECTED, $message);
+			$code = $response['protocol']['code'] ?? 500;
+			if ($code === 100 || $code === 200 || $code === 204) {
+				// c-icap/clamav reports this header
+				$virus = $response['headers'][$this->virusHeader] ?? false;
+				if ($virus) {
+					return Status::create(Status::SCANRESULT_INFECTED, $virus);
+				}
+
+				// kaspersky(pre-2020 product editions) and McAfee handling
+				$respHeader = $response['body']['res-hdr']['HTTP_STATUS'] ?? '';
+				if (\strpos($respHeader, '403 Forbidden') !== false || \strpos($respHeader, '403 VirusFound') !== false) {
+					$message = $this->l10n->t('A malware or virus was detected, your upload was denied. In doubt or for details please contact your system administrator.');
+					return Status::create(Status::SCANRESULT_INFECTED, $message);
+				}
+				return Status::create(Status::SCANRESULT_CLEAN);
 			}
-		} else {
-			throw new \RuntimeException('AV failed!');
+			$respJson = json_encode($response, JSON_THROW_ON_ERROR);
+			$this->logger->error("ICAP response unusable: $respJson");
+		} catch (InitException $e) {
+			$this->logger->logException($e);
 		}
-		return Status::create(Status::SCANRESULT_CLEAN);
+		throw new \RuntimeException(L10n::getEnduserNotification($this->l10n));
 	}
 
 	/**