Merge pull request #360 from nextcloud/unscanned-query

fix: improve query for finding unscanned files

appinfo/info.xml

@@ -14,7 +14,7 @@
 This application inspects files that are uploaded to Nextcloud for viruses before they are written to the Nextcloud storage. If a file is identified as a virus, it is either logged or not uploaded to the server. The application relies on the underlying ClamAV virus scanning engine, which the admin points Nextcloud to when configuring the application. Alternatively, a Kaspersky Scan Engine can be configured, which has to run on a separate server.
 For this app to be effective, the ClamAV virus definitions should be kept up to date. Also note that enabling this app will impact system performance as additional processing is required for every upload. More information is available in the Antivirus documentation.
 	]]></description>
-	<version>5.5.6</version>
+	<version>5.5.7</version>
 	<licence>agpl</licence>
 
 	<author>Manuel Delgado</author>
@@ -44,7 +44,7 @@ For this app to be effective, the ClamAV virus definitions should be kept up to
 	<screenshot>https://raw.githubusercontent.com/nextcloud/files_antivirus/master/screenshots/1.png</screenshot>
 
 	<dependencies>
-		<nextcloud min-version="26" max-version="29" />
+		<nextcloud min-version="24" max-version="29" />
 	</dependencies>
 
 	<background-jobs>

lib/BackgroundJob/BackgroundScanner.php

@@ -20,6 +20,7 @@
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Config\IUserMountCache;
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\Files\IMimeTypeLoader;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
@@ -196,19 +197,25 @@ protected function getSizeLimitExpression(IQueryBuilder $qb) {
 	}
 
 	/**
+	 * Find files in the filecache that have never been scanned
+	 *
 	 * @return \Iterator<int>
 	 * @throws \OCP\DB\Exception
 	 */
 	public function getUnscannedFiles() {
-		$dirMimeTypeId = $this->mimeTypeLoader->getId('httpd/unix-directory');
+		$dirMimeTypeId = $this->mimeTypeLoader->getId(FileInfo::MIMETYPE_FOLDER);
 
 		$query = $this->db->getQueryBuilder();
 		$query->select('fc.fileid')
 			->from('filecache', 'fc')
+			->leftJoin('fc', 'storages', 's', $query->expr()->eq('fc.storage', 's.numeric_id'))
 			->leftJoin('fc', 'files_antivirus', 'fa', $query->expr()->eq('fc.fileid', 'fa.fileid'))
 			->where($query->expr()->isNull('fa.fileid'))
-			->andWhere($query->expr()->neq('mimetype', $query->expr()->literal($dirMimeTypeId)))
-			->andWhere($query->expr()->like('path', $query->expr()->literal('files/%')))
+			->andWhere($query->expr()->neq('mimetype', $query->createNamedParameter($dirMimeTypeId)))
+			->andWhere($query->expr()->orX(
+				$query->expr()->like('path', $query->createNamedParameter('files/%')),
+				$query->expr()->notLike('s.id', $query->createNamedParameter("home::%"))
+			))
 			->andWhere($this->getSizeLimitExpression($query))
 			->setMaxResults($this->getBatchSize() * 10);
 
@@ -220,6 +227,8 @@ public function getUnscannedFiles() {
 
 
 	/**
+	 * Find files that have been updated since they got last scanned
+	 *
 	 * @return \Iterator<int>
 	 * @throws \OCP\DB\Exception
 	 */
@@ -240,6 +249,8 @@ public function getToRescanFiles() {
 
 
 	/**
+	 * Find files that have been last scanned more than 28 days ago
+	 *
 	 * @return \Iterator<int>
 	 * @throws \OCP\DB\Exception
 	 */

lib/Scanner/ICAP.php

@@ -76,7 +76,7 @@ public function initScanner() {
 		if (str_contains($path, '.ocTransferId') && str_ends_with($path, '.part')) {
 			[$path] = explode('.ocTransferId', $path, 2);
 		}
-		$remote = $this->request?->getRemoteAddress();
+		$remote = $this->request ? $this->request->getRemoteAddress() : null;
 		$encodedPath = implode("/", array_map("rawurlencode", explode("/", $path)));
 		if ($this->mode === ICAPClient::MODE_REQ_MOD) {
 			$this->icapRequest = $this->icapClient->reqmod($this->service, [

lib/Scanner/ScannerFactory.php

@@ -13,11 +13,18 @@
 use Psr\Container\ContainerInterface;
 
 class ScannerFactory {
+	protected AppConfig $appConfig;
+	protected ContainerInterface $serverContainer;
+	protected IRequest $request;
+
 	public function __construct(
-		protected AppConfig $appConfig,
-		private ContainerInterface $serverContainer,
-		private IRequest $request,
+		AppConfig $appConfig,
+		ContainerInterface $serverContainer,
+		IRequest $request
 	) {
+		$this->appConfig = $appConfig;
+		$this->serverContainer = $serverContainer;
+		$this->request = $request;
 	}
 
 	/**

tests/BackgroundScannerTest.php

@@ -35,22 +35,28 @@ class BackgroundScannerTest extends TestBase {
 	use UserTrait;
 	use MountProviderTrait;
 
-	/** @var Folder */
-	private $homeDirectory;
+	private Folder $homeDirectory;
+	private Folder $externalDirectory;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->createUser("av", "av");
-		$storage = new Temporary();
+		$storage = new TemporaryHome();
 		$storage->mkdir('files');
 		$storage->getScanner()->scan('');
+
+		$external = new Temporary();
+		$external->getScanner()->scan('');
+
 		$this->registerMount("av", $storage, "av");
+		$this->registerMount("av", $external, "av/files/external");
 
 		$this->loginAsUser("av");
 		/** @var IRootFolder $root */
 		$root = \OC::$server->get(IRootFolder::class);
 		$this->homeDirectory = $root->getUserFolder("av");
+		$this->externalDirectory = $this->homeDirectory->get('external');
 	}
 
 	private function markAllScanned() {
@@ -120,6 +126,17 @@ public function testGetUnscannedFiles() {
 
 		$scanner = $this->getBackgroundScanner();
 		$newFileId = $this->homeDirectory->newFile("foo", "bar")->getId();
+		$this->homeDirectory->getParent()->newFile("outside", "bar")->getId();
+
+		$outdated = iterator_to_array($scanner->getUnscannedFiles());
+		$this->assertEquals([$newFileId], $outdated);
+	}
+
+	public function testGetUnscannedFilesExternal() {
+		$this->markAllScanned();
+
+		$scanner = $this->getBackgroundScanner();
+		$newFileId = $this->homeDirectory->newFile("external/foo2", "bar2")->getId();
 
 		$outdated = iterator_to_array($scanner->getUnscannedFiles());
 		$this->assertEquals([$newFileId], $outdated);

tests/TemporaryHome.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Robin Appelman <[email protected]>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Files_Antivirus\Tests;
+
+class TemporaryHome extends \OC\Files\Storage\Temporary {
+	private string $id;
+
+	public function __construct($arguments = null) {
+		parent::__construct($arguments);
+		$this->id = uniqid();
+	}
+
+	public function getId() {
+		return "home::" . $this->id;
+	}
+
+}