From dd04ef1be5e6b8f4589d79e62bb55bf4db8140e5 Mon Sep 17 00:00:00 2001 From: Viktar Dubiniuk Date: Tue, 25 May 2021 18:01:02 +0300 Subject: [PATCH] Stop reading the response after headers are read --- lib/Scanner/ICAPClient.php | 162 ++++++++++++------------- lib/Scanner/ICAPScanner.php | 6 +- tests/unit/Scanner/IcapScannerTest.php | 1 + 3 files changed, 82 insertions(+), 87 deletions(-) diff --git a/lib/Scanner/ICAPClient.php b/lib/Scanner/ICAPClient.php index b848857a..e39f67a9 100644 --- a/lib/Scanner/ICAPClient.php +++ b/lib/Scanner/ICAPClient.php @@ -95,28 +95,13 @@ public function getRequest(string $method, string $service, array $body = [], ar return $request; } - public function options(string $service): array { - $request = $this->getRequest('OPTIONS', $service); - $response = $this->send($request); - - return $this->parseResponse($response); - } - - public function respmod(string $service, array $body = [], array $headers = []): array { - $request = $this->getRequest('RESPMOD', $service, $body, $headers); - $response = $this->send($request); - - return $this->parseResponse($response); - } - public function reqmod(string $service, array $body = [], array $headers = []): array { $request = $this->getRequest('REQMOD', $service, $body, $headers); $response = $this->send($request); - - return $this->parseResponse($response); + return $response; } - private function send(string $request): string { + private function send(string $request): array { $this->connect(); // Shut stupid uncontrolled messaging up - we handle errors on our own if (@\fwrite($this->writeHandle, $request) === false) { @@ -125,90 +110,99 @@ private function send(string $request): string { ); } - # McAfee seems to not properly close the socket once all response bytes are sent to the client - # we use a 10 sec time out on receiving data - \stream_set_timeout($this->writeHandle, 10, 0); - $response = ''; - while ($buffer = \fread($this->writeHandle, 2048)) { - $response .= $buffer; + // read header + $protocol = $this->readIcapStatusLine(); + $headers = $this->readHeaders(); + $resHdr = []; + if (isset($headers['Encapsulated'])) { + $resHdr = $this->parseResHdr($headers['Encapsulated']); } $this->disconnect(); - return $response; + return [ + 'protocol' => $protocol, + 'headers' => $headers, + 'body' => ['res-hdr' => $resHdr] + ]; } - private function parseResponse(string $response): array { - $responseArray = [ - 'protocol' => [], - 'headers' => [], - 'body' => [], - 'rawBody' => '' + private function readIcapStatusLine(): array { + $icapHeader = \trim(\fgets($this->writeHandle)); + $numValues = \sscanf($icapHeader, "ICAP/%d.%d %d %s", $v1, $v2, $code, $status); + if ($numValues !== 4) { + throw new RuntimeException("Unknown ICAP response: \"$icapHeader\""); + } + return [ + 'protocolVersion' => "$v1.$v2", + 'code' => $code, + 'status' => $status, ]; + } - foreach (\preg_split('/\r?\n/', $response) as $line) { - if ($responseArray['protocol'] === []) { - if (\strpos($line, 'ICAP/') !== 0) { - throw new RuntimeException("Unknown ICAP response: \"$response\""); - } - - $parts = \preg_split('/\ +/', $line, 3); - - $responseArray['protocol'] = [ - 'icap' => $parts[0] ?? '', - 'code' => $parts[1] ?? '', - 'message' => $parts[2] ?? '', - ]; - + private function parseResHdr(string $headerValue): array { + $encapsulatedHeaders = []; + $encapsulatedParts = \explode(",", $headerValue); + foreach ($encapsulatedParts as $encapsulatedPart) { + $pieces = \explode("=", \trim($encapsulatedPart)); + if ($pieces[1] === "0") { continue; } + $rawEncapsulatedHeaders = \fread($this->writeHandle, $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; + } + return $encapsulatedHeaders; + } - if ($line === '') { + private function readHeaders(): array { + $headers = []; + $prevString = ""; + while ($headerString = \fgets($this->writeHandle)) { + $trimmedHeaderString = \trim($headerString); + if ($prevString === "" && $trimmedHeaderString === "") { break; } - - $parts = \preg_split('/:\ /', $line, 2); - if (isset($parts[0])) { - $responseArray['headers'][$parts[0]] = $parts[1] ?? ''; + list($headerName, $headerValue) = $this->parseHeader($trimmedHeaderString); + if ($headerName !== '') { + $headers[$headerName] = $headerValue; + if ($headerName == "Encapsulated") { + break; + } } + $prevString = $trimmedHeaderString; } + return $headers; + } - $body = \preg_split('/\r?\n\r?\n/', $response, 2); - if (isset($body[1])) { - $responseArray['rawBody'] = $body[1]; - - if (\array_key_exists('Encapsulated', $responseArray['headers'])) { - $encapsulated = []; - $params = \explode(", ", $responseArray['headers']['Encapsulated']); - - foreach ($params as $param) { - $parts = \explode("=", $param); - if (\count($parts) !== 2) { - continue; - } - - $encapsulated[$parts[0]] = $parts[1]; - } - - foreach ($encapsulated as $section => $offset) { - $data = \substr($body[1], (int)$offset); - switch ($section) { - case 'req-hdr': - case 'res-hdr': - $responseArray['body'][$section] = \preg_split('/\r?\n\r?\n/', $data, 2)[0]; - break; - - case 'req-body': - case 'res-body': - $parts = \preg_split('/\r?\n/', $data, 2); - if (\count($parts) === 2) { - $responseArray['body'][$section] = \substr($parts[1], 0, \hexdec($parts[0])); - } - break; - } - } + 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; + } + list($name, $value) = $this->parseHeader($line); + if ($name !== '') { + $headers[$name] = $value; } } - return $responseArray; + return $headers; + } + + private function parseHeader(string $headerString): array { + $name = ''; + $value = ''; + $parts = \preg_split('/:\ /', $headerString, 2); + if (isset($parts[0])) { + $name = $parts[0]; + $value = $parts[1] ?? ''; + } + return [$name, $value]; } } diff --git a/lib/Scanner/ICAPScanner.php b/lib/Scanner/ICAPScanner.php index b52e17ba..b110e41f 100644 --- a/lib/Scanner/ICAPScanner.php +++ b/lib/Scanner/ICAPScanner.php @@ -53,8 +53,8 @@ public function completeAsyncScan() { ], [ 'Allow' => 204 ]); - $code = $response['protocol']['code'] ?? '500'; - if ($code === '200' || $code === '204') { + $code = $response['protocol']['code'] ?? 500; + if ($code === 200 || $code === 204) { // c-icap/clamav reports this header $virus = $response['headers'][$this->virusHeader] ?? false; if ($virus) { @@ -62,7 +62,7 @@ public function completeAsyncScan() { } // kaspersky(pre 2020 product editions) and McAfee handling - $respHeader = $response['body']['res-hdr'] ?? ''; + $respHeader = $response['body']['res-hdr']['HTTP_STATUS'] ?? ''; if (\strpos($respHeader, '403 Forbidden') || \strpos($respHeader, '403 VirusFound')) { $message = $this->l10n->t('A malware or virus was detected, your upload was deleted. In doubt or for details please contact your system administrator'); return Status::create(Status::SCANRESULT_INFECTED, $message); diff --git a/tests/unit/Scanner/IcapScannerTest.php b/tests/unit/Scanner/IcapScannerTest.php index 66fd6695..ad8a6b07 100644 --- a/tests/unit/Scanner/IcapScannerTest.php +++ b/tests/unit/Scanner/IcapScannerTest.php @@ -33,6 +33,7 @@ protected function setUp(): void { $logger = $this->createMock(ILogger::class); $l10n = $this->createMock(IL10N::class); + $l10n->method('t')->will($this->returnArgument(0)); # for local testing replace 'icap' with the ip of the clamav instance $config->method('getAvHost')->willReturn('icap');