From 55c4ef48319dc341492360926e9e83461d6fc552 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 13 Mar 2021 21:25:29 +0000 Subject: [PATCH 1/2] Robust handling of responses with invalid headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tim Düsterhus <209270+TimWolla@users.noreply.github.com> --- src/Handler/EasyHandle.php | 18 +++++-------- src/Handler/HeaderProcessor.php | 42 +++++++++++++++++++++++++++++ src/Handler/StreamHandler.php | 22 ++++++++++----- tests/Handler/CurlFactoryTest.php | 20 ++++++++++++++ tests/Handler/StreamHandlerTest.php | 30 +++++++++++++++++++++ tests/server.js | 13 ++++++++- 6 files changed, 126 insertions(+), 19 deletions(-) create mode 100644 src/Handler/HeaderProcessor.php diff --git a/src/Handler/EasyHandle.php b/src/Handler/EasyHandle.php index a68c62f08..224344d7c 100644 --- a/src/Handler/EasyHandle.php +++ b/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 ); } diff --git a/src/Handler/HeaderProcessor.php b/src/Handler/HeaderProcessor.php new file mode 100644 index 000000000..fb6fd2aae --- /dev/null +++ b/src/Handler/HeaderProcessor.php @@ -0,0 +1,42 @@ +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) { + $msg = 'An error was encountered while creating the response'; + $ex = new RequestException($msg, $request, null, $e); + return P\Create::rejectionFor($ex); + } + [$stream, $headers] = $this->checkDecode($options, $headers, $stream); $stream = Psr7\Utils::streamFor($stream); $sink = $stream; @@ -112,7 +116,13 @@ 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) { + $msg = 'An error was encountered while creating the response'; + $ex = new RequestException($msg, $request, null, $e); + return P\Create::rejectionFor($ex); + } if (isset($options['on_headers'])) { try { diff --git a/tests/Handler/CurlFactoryTest.php b/tests/Handler/CurlFactoryTest.php index 0f9c32374..254d7c188 100644 --- a/tests/Handler/CurlFactoryTest.php +++ b/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(); + } } diff --git a/tests/Handler/StreamHandlerTest.php b/tests/Handler/StreamHandlerTest.php index 5bab7f43a..c0c6c9f63 100644 --- a/tests/Handler/StreamHandlerTest.php +++ b/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(); + } } diff --git a/tests/server.js b/tests/server.js index f6c336a5a..f784ff661 100644 --- a/tests/server.js +++ b/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') { From 436f3c7d82452210f7fd905618f00d539aff4de8 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sun, 21 Mar 2021 17:06:49 +0000 Subject: [PATCH 2/2] Cleanup --- src/Handler/HeaderProcessor.php | 14 +++++++------- src/Handler/StreamHandler.php | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Handler/HeaderProcessor.php b/src/Handler/HeaderProcessor.php index fb6fd2aae..a0988845f 100644 --- a/src/Handler/HeaderProcessor.php +++ b/src/Handler/HeaderProcessor.php @@ -12,22 +12,22 @@ final class HeaderProcessor /** * Returns the HTTP version, status code, reason phrase, and headers. * - * @param string[] $hdrs + * @param string[] $headers * * @throws \RuntimeException * * @return array{0:string, 1:int, 2:?string, 3:array} */ - public static function parseHeaders(array $hdrs): array + public static function parseHeaders(array $headers): array { - if ($hdrs === []) { + if ($headers === []) { throw new \RuntimeException('Expected a non-empty array of header data'); } - $parts = \explode(' ', \array_shift($hdrs), 3); - $ver = \explode('/', $parts[0])[1] ?? null; + $parts = \explode(' ', \array_shift($headers), 3); + $version = \explode('/', $parts[0])[1] ?? null; - if ($ver === null) { + if ($version === null) { throw new \RuntimeException('HTTP version missing from header data'); } @@ -37,6 +37,6 @@ public static function parseHeaders(array $hdrs): array throw new \RuntimeException('HTTP status code missing from header data'); } - return [$ver, (int) $status, $parts[2] ?? null, Utils::headersFromLines($hdrs)]; + return [$version, (int) $status, $parts[2] ?? null, Utils::headersFromLines($headers)]; } } diff --git a/src/Handler/StreamHandler.php b/src/Handler/StreamHandler.php index 1473cac79..877befd38 100644 --- a/src/Handler/StreamHandler.php +++ b/src/Handler/StreamHandler.php @@ -103,9 +103,9 @@ private function createResponse(RequestInterface $request, array $options, $stre try { [$ver, $status, $reason, $headers] = HeaderProcessor::parseHeaders($hdrs); } catch (\Exception $e) { - $msg = 'An error was encountered while creating the response'; - $ex = new RequestException($msg, $request, null, $e); - return P\Create::rejectionFor($ex); + return P\Create::rejectionFor( + new RequestException('An error was encountered while creating the response', $request, null, $e) + ); } [$stream, $headers] = $this->checkDecode($options, $headers, $stream); @@ -119,18 +119,18 @@ private function createResponse(RequestInterface $request, array $options, $stre try { $response = new Psr7\Response($status, $headers, $sink, $ver, $reason); } catch (\Exception $e) { - $msg = 'An error was encountered while creating the response'; - $ex = new RequestException($msg, $request, null, $e); - return P\Create::rejectionFor($ex); + 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) + ); } }