Merge pull request #336 from nextcloud/icap-encapsulated-header-fix

improve parsing of encapsulated icap headers

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.2</version>
+	<version>5.5.3</version>
 	<licence>agpl</licence>
 
 	<author>Manuel Delgado</author>

lib/ICAP/ICAPClient.php

@@ -67,6 +67,8 @@ protected function connect() {
 			$this->connectTimeout
 		);
 
+		socket_set_timeout($stream, 600);
+
 		if (!$stream) {
 			throw new RuntimeException(
 				"Cannot connect to \"tcp://{$this->host}:{$this->port}\": $errorMessage (code $errorCode)"

lib/ICAP/ResponseParser.php

@@ -66,58 +66,37 @@ private function readIcapStatusLine($stream): IcapResponseStatus {
 	}
 
 	private function parseResHdr($stream, string $headerValue): array {
-		$encapsulatedHeaders = [];
 		$encapsulatedParts = \explode(",", $headerValue);
 		foreach ($encapsulatedParts as $encapsulatedPart) {
 			$pieces = \explode("=", \trim($encapsulatedPart));
-			if ($pieces[1] === "0") {
-				continue;
+			if ($pieces[0] === "res-hdr") {
+				$offset = (int)$pieces[1];
+				if ($offset > 0) {
+					fseek($stream, $offset);
+				}
+				break;
 			}
-			$rawEncapsulatedHeaders = \fread($stream, (int)$pieces[1]);
-			$encapsulatedHeaders = $this->parseEncapsulatedHeaders($rawEncapsulatedHeaders);
-			// According to the spec we have a single res-hdr part and are not interested in res-body content
-			break;
 		}
+
+		$status = trim(\fgets($stream));
+		$encapsulatedHeaders = $this->readHeaders($stream);
+		$encapsulatedHeaders['HTTP_STATUS'] = $status;
+
 		return $encapsulatedHeaders;
 	}
 
 	private function readHeaders($stream): array {
 		$headers = [];
-		$prevString = "";
 		while (($headerString = \fgets($stream)) !== false) {
 			$trimmedHeaderString = \trim($headerString);
-			if ($prevString === "" && $trimmedHeaderString === "") {
+			if ($trimmedHeaderString === "") {
 				break;
 			}
 			[$headerName, $headerValue] = $this->parseHeader($trimmedHeaderString);
 			if ($headerName !== '') {
 				$headers[$headerName] = $headerValue;
-				if ($headerName == "Encapsulated") {
-					break;
-				}
-			}
-			$prevString = $trimmedHeaderString;
-		}
-		return $headers;
-	}
-
-	private function parseEncapsulatedHeaders(string $headerString): array {
-		$headers = [];
-		$split = \preg_split('/\r?\n/', \trim($headerString));
-		$statusLine = \array_shift($split);
-		if ($statusLine !== null) {
-			$headers['HTTP_STATUS'] = $statusLine;
-		}
-		foreach (\preg_split('/\r?\n/', $headerString) as $line) {
-			if ($line === '') {
-				continue;
-			}
-			[$name, $value] = $this->parseHeader($line);
-			if ($name !== '') {
-				$headers[$name] = $value;
 			}
 		}
-
 		return $headers;
 	}
 

tests/ICAP/ResponseParserTest.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2024 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * 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
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Files_Antivirus\Tests\ICAP;
+
+use OCA\Files_Antivirus\ICAP\ResponseParser;
+use Test\TestCase;
+
+class ResponseParserTest extends TestCase {
+	private function parseResponse(string $responsePath) {
+		return (new ResponseParser())->read_response(fopen($responsePath, 'r'));
+	}
+
+	public function testParse403() {
+		$response = $this->parseResponse(__DIR__ . '/../data/icap/403-response.txt');
+		$this->assertEquals('HTTP/1.1 403 Forbidden', $response->getResponseHeaders()['HTTP_STATUS']);
+	}
+}

tests/data/icap/403-response.txt

@@ -0,0 +1,13 @@
+ICAP/1.0 200 OK
+ISTag: "b9eb4f031598bb0b-1716440776"
+Encapsulated: res-hdr=0, res-body=70
+X-Infection-Found: Type=0; Resolution=0; Threat=Eicar;
+X-Virus-ID: Testdatei
+X-Response-Info: Blocked
+X-Response-Description: Durch Löschen gesäubert
+
+HTTP/1.1 403 Forbidden
+Content-Type: text/html
+Content-Length: 0
+
+0