Robust handling of responses with invalid headers (#2872)

* Robust handling of responses with invalid headers

Co-Authored-By: Tim Düsterhus <209270+TimWolla@users.noreply.github.com>

* Cleanup

Co-authored-by: Tim Düsterhus <209270+TimWolla@users.noreply.github.com>

src/Handler/EasyHandle.php

@@ -63,17 +63,13 @@ final class EasyHandle
     /**
      * Attach a response to the easy handle based on the received headers.
      *
-     * @throws \RuntimeException if no headers have been received.
+     * @throws \RuntimeException if no headers have been received or the first
+     *                           header line is invalid.
      */
     public function createResponse(): void
     {
-        if (empty($this->headers)) {
-            throw new \RuntimeException('No headers have been received');
-        }
+        [$ver, $status, $reason, $headers] = HeaderProcessor::parseHeaders($this->headers);
 
-        // HTTP-version SP status-code SP reason-phrase
-        $startLine = \explode(' ', \array_shift($this->headers), 3);
-        $headers = Utils::headersFromLines($this->headers);
         $normalizedKeys = Utils::normalizeHeaderKeys($headers);
 
         if (!empty($this->options['decode_content']) && isset($normalizedKeys['content-encoding'])) {
@@ -91,15 +87,13 @@ public function createResponse(): void
             }
         }
 
-        $statusCode = (int) $startLine[1];
-
         // Attach a response to the easy handle with the parsed headers.
         $this->response = new Response(
-            $statusCode,
+            $status,
             $headers,
             $this->sink,
-            \substr($startLine[0], 5),
-            isset($startLine[2]) ? (string) $startLine[2] : null
+            $ver,
+            $reason
         );
     }
 

src/Handler/HeaderProcessor.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace GuzzleHttp\Handler;
+
+use GuzzleHttp\Utils;
+
+/**
+ * @internal
+ */
+final class HeaderProcessor
+{
+    /**
+     * Returns the HTTP version, status code, reason phrase, and headers.
+     *
+     * @param string[] $headers
+     *
+     * @throws \RuntimeException
+     *
+     * @return array{0:string, 1:int, 2:?string, 3:array}
+     */
+    public static function parseHeaders(array $headers): array
+    {
+        if ($headers === []) {
+            throw new \RuntimeException('Expected a non-empty array of header data');
+        }
+
+        $parts = \explode(' ', \array_shift($headers), 3);
+        $version = \explode('/', $parts[0])[1] ?? null;
+
+        if ($version === null) {
+            throw new \RuntimeException('HTTP version missing from header data');
+        }
+
+        $status = $parts[1] ?? null;
+
+        if ($status === null) {
+            throw new \RuntimeException('HTTP status code missing from header data');
+        }
+
+        return [$version, (int) $status, $parts[2] ?? null, Utils::headersFromLines($headers)];
+    }
+}

src/Handler/StreamHandler.php

@@ -99,11 +99,15 @@ private function createResponse(RequestInterface $request, array $options, $stre
     {
         $hdrs = $this->lastHeaders;
         $this->lastHeaders = [];
-        $parts = \explode(' ', \array_shift($hdrs), 3);
-        $ver = \explode('/', $parts[0])[1];
-        $status = (int) $parts[1];
-        $reason = $parts[2] ?? null;
-        $headers = Utils::headersFromLines($hdrs);
+
+        try {
+            [$ver, $status, $reason, $headers] = HeaderProcessor::parseHeaders($hdrs);
+        } catch (\Exception $e) {
+            return P\Create::rejectionFor(
+                new RequestException('An error was encountered while creating the response', $request, null, $e)
+            );
+        }
+
         [$stream, $headers] = $this->checkDecode($options, $headers, $stream);
         $stream = Psr7\Utils::streamFor($stream);
         $sink = $stream;
@@ -112,15 +116,21 @@ private function createResponse(RequestInterface $request, array $options, $stre
             $sink = $this->createSink($stream, $options);
         }
 
-        $response = new Psr7\Response($status, $headers, $sink, $ver, $reason);
+        try {
+            $response = new Psr7\Response($status, $headers, $sink, $ver, $reason);
+        } catch (\Exception $e) {
+            return P\Create::rejectionFor(
+                new RequestException('An error was encountered while creating the response', $request, null, $e)
+            );
+        }
 
         if (isset($options['on_headers'])) {
             try {
                 $options['on_headers']($response);
             } catch (\Exception $e) {
-                $msg = 'An error was encountered during the on_headers event';
-                $ex = new RequestException($msg, $request, $response, $e);
-                return P\Create::rejectionFor($ex);
+                return P\Create::rejectionFor(
+                    new RequestException('An error was encountered during the on_headers event', $request, $response, $e)
+                );
             }
         }
 

tests/Handler/CurlFactoryTest.php

@@ -878,4 +878,24 @@ public function testBodyEofOnWindows()
         }
         self::assertSame($expectedLength, $actualLength);
     }
+
+    public function testHandlesGarbageHttpServerGracefully()
+    {
+        $a = new Handler\CurlMultiHandler();
+
+        $this->expectException(RequestException::class);
+        $this->expectExceptionMessage('cURL error 1: Received HTTP/0.9 when not allowed');
+
+        $a(new Psr7\Request('GET', Server::$url . 'guzzle-server/garbage'), [])->wait();
+    }
+
+    public function testHandlesInvalidStatusCodeGracefully()
+    {
+        $a = new Handler\CurlMultiHandler();
+
+        $this->expectException(RequestException::class);
+        $this->expectExceptionMessage('An error was encountered while creating the response');
+
+        $a(new Psr7\Request('GET', Server::$url . 'guzzle-server/bad-status'), [])->wait();
+    }
 }

tests/Handler/StreamHandlerTest.php

@@ -708,4 +708,34 @@ public function testHonorsReadTimeout()
         self::assertTrue(\stream_get_meta_data($body)['timed_out']);
         self::assertFalse(\feof($body));
     }
+
+    public function testHandlesGarbageHttpServerGracefully()
+    {
+        $handler = new StreamHandler();
+
+        $this->expectException(RequestException::class);
+        $this->expectExceptionMessage('An error was encountered while creating the response');
+
+        $handler(
+            new Request('GET', Server::$url . 'guzzle-server/garbage'),
+            [
+                RequestOptions::STREAM => true,
+            ]
+        )->wait();
+    }
+
+    public function testHandlesInvalidStatusCodeGracefully()
+    {
+        $handler = new StreamHandler();
+
+        $this->expectException(RequestException::class);
+        $this->expectExceptionMessage('An error was encountered while creating the response');
+
+        $handler(
+            new Request('GET', Server::$url . 'guzzle-server/bad-status'),
+            [
+                RequestOptions::STREAM => true,
+            ]
+        )->wait();
+    }
 }

tests/server.js

@@ -125,7 +125,18 @@ var GuzzleServer = function(port, log) {
   };
 
   var controlRequest = function(request, req, res) {
-    if (req.url == '/guzzle-server/perf') {
+    if (req.url == '/guzzle-server/garbage') {
+      if (that.log) {
+        console.log('returning garbage')
+      }
+      res.socket.end("220 example.com ESMTP\r\n200 This is garbage\r\n\r\n");
+    } else if (req.url == '/guzzle-server/bad-status') {
+      if (that.log) {
+        console.log('returning bad status code')
+      }
+      res.writeHead(700, 'BAD', {'Content-Length': 16});
+      res.end('Body of response');
+    } else if (req.url == '/guzzle-server/perf') {
       res.writeHead(200, 'OK', {'Content-Length': 16});
       res.end('Body of response');
     } else if (req.method == 'DELETE') {