diff --git a/src/Io/AbstractMessage.php b/src/Io/AbstractMessage.php index ab023304..a0706bb1 100644 --- a/src/Io/AbstractMessage.php +++ b/src/Io/AbstractMessage.php @@ -19,7 +19,7 @@ abstract class AbstractMessage implements MessageInterface * @internal * @var string */ - const REGEX_HEADERS = '/^([^()<>@,;:\\\"\/\[\]?={}\x01-\x20\x7F]++):[\x20\x09]*+((?:[\x20\x09]*+[\x21-\x7E\x80-\xFF]++)*+)[\x20\x09]*+[\r]?+\n/m'; + const REGEX_HEADERS = '/^([^()<>@,;:\\\"\/\[\]?={}\x00-\x20\x7F]++):[\x20\x09]*+((?:[\x20\x09]*+[\x21-\x7E\x80-\xFF]++)*+)[\x20\x09]*+[\r]?+\n/m'; /** @var array */ private $headers = array(); diff --git a/src/Io/ClientRequestStream.php b/src/Io/ClientRequestStream.php index 12c15caf..ff9bf2d4 100644 --- a/src/Io/ClientRequestStream.php +++ b/src/Io/ClientRequestStream.php @@ -56,7 +56,25 @@ private function writeHead() { $this->state = self::STATE_WRITING_HEAD; - $request = $this->request; + $expected = 0; + $headers = "{$this->request->getMethod()} {$this->request->getRequestTarget()} HTTP/{$this->request->getProtocolVersion()}\r\n"; + foreach ($this->request->getHeaders() as $name => $values) { + if (\strpos($name, ':') !== false) { + $expected = -1; + break; + } + foreach ($values as $value) { + $headers .= "$name: $value\r\n"; + ++$expected; + } + } + + /** @var array $m legacy PHP 5.3 only */ + if (!\preg_match('#^\S+ \S+ HTTP/1\.[01]\r\n#m', $headers) || \substr_count($headers, "\n") !== ($expected + 1) || (\PHP_VERSION_ID >= 50400 ? \preg_match_all(AbstractMessage::REGEX_HEADERS, $headers) : \preg_match_all(AbstractMessage::REGEX_HEADERS, $headers, $m)) !== $expected) { + $this->closeError(new \InvalidArgumentException('Unable to send request with invalid request headers')); + return; + } + $connectionRef = &$this->connection; $stateRef = &$this->state; $pendingWrites = &$this->pendingWrites; @@ -64,7 +82,7 @@ private function writeHead() $promise = $this->connectionManager->connect($this->request->getUri()); $promise->then( - function (ConnectionInterface $connection) use ($request, &$connectionRef, &$stateRef, &$pendingWrites, $that) { + function (ConnectionInterface $connection) use ($headers, &$connectionRef, &$stateRef, &$pendingWrites, $that) { $connectionRef = $connection; assert($connectionRef instanceof ConnectionInterface); @@ -74,14 +92,6 @@ function (ConnectionInterface $connection) use ($request, &$connectionRef, &$sta $connection->on('error', array($that, 'handleError')); $connection->on('close', array($that, 'close')); - assert($request instanceof RequestInterface); - $headers = "{$request->getMethod()} {$request->getRequestTarget()} HTTP/{$request->getProtocolVersion()}\r\n"; - foreach ($request->getHeaders() as $name => $values) { - foreach ($values as $value) { - $headers .= "$name: $value\r\n"; - } - } - $more = $connection->write($headers . "\r\n" . $pendingWrites); assert($stateRef === ClientRequestStream::STATE_WRITING_HEAD); diff --git a/src/Io/StreamingServer.php b/src/Io/StreamingServer.php index 790c8cc1..143edaa8 100644 --- a/src/Io/StreamingServer.php +++ b/src/Io/StreamingServer.php @@ -333,13 +333,26 @@ public function handleResponse(ConnectionInterface $connection, ServerRequestInt } // build HTTP response header by appending status line and header fields + $expected = 0; $headers = "HTTP/" . $version . " " . $code . " " . $response->getReasonPhrase() . "\r\n"; foreach ($response->getHeaders() as $name => $values) { + if (\strpos($name, ':') !== false) { + $expected = -1; + break; + } foreach ($values as $value) { $headers .= $name . ": " . $value . "\r\n"; + ++$expected; } } + /** @var array $m legacy PHP 5.3 only */ + if ($code < 100 || $code > 999 || \substr_count($headers, "\n") !== ($expected + 1) || (\PHP_VERSION_ID >= 50400 ? \preg_match_all(AbstractMessage::REGEX_HEADERS, $headers) : \preg_match_all(AbstractMessage::REGEX_HEADERS, $headers, $m)) !== $expected) { + $this->emit('error', array(new \InvalidArgumentException('Unable to send response with invalid response headers'))); + $this->writeError($connection, Response::STATUS_INTERNAL_SERVER_ERROR, $request); + return; + } + // response to HEAD and 1xx, 204 and 304 responses MUST NOT include a body // exclude status 101 (Switching Protocols) here for Upgrade request handling above if ($method === 'HEAD' || ($code >= 100 && $code < 200 && $code !== Response::STATUS_SWITCHING_PROTOCOLS) || $code === Response::STATUS_NO_CONTENT || $code === Response::STATUS_NOT_MODIFIED) { diff --git a/tests/Io/ClientRequestStreamTest.php b/tests/Io/ClientRequestStreamTest.php index 181db173..9a5373a1 100644 --- a/tests/Io/ClientRequestStreamTest.php +++ b/tests/Io/ClientRequestStreamTest.php @@ -2,6 +2,7 @@ namespace React\Tests\Http\Io; +use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use React\Http\Io\ClientRequestStream; use React\Http\Message\Request; @@ -100,6 +101,67 @@ public function requestShouldEmitErrorIfConnectionEmitsError() $request->handleError(new \Exception('test')); } + public static function provideInvalidRequest() + { + $request = new Request('GET' , "http://localhost/"); + + return array( + array( + $request->withMethod("INVA\r\nLID", '') + ), + array( + $request->withRequestTarget('/inva lid') + ), + array( + $request->withHeader('Invalid', "Yes\r\n") + ), + array( + $request->withHeader('Invalid', "Yes\n") + ), + array( + $request->withHeader('Invalid', "Yes\r") + ), + array( + $request->withHeader("Inva\r\nlid", 'Yes') + ), + array( + $request->withHeader("Inva\nlid", 'Yes') + ), + array( + $request->withHeader("Inva\rlid", 'Yes') + ), + array( + $request->withHeader('Inva Lid', 'Yes') + ), + array( + $request->withHeader('Inva:Lid', 'Yes') + ), + array( + $request->withHeader('Invalid', "Val\0ue") + ), + array( + $request->withHeader("Inva\0lid", 'Yes') + ) + ); + } + + /** + * @dataProvider provideInvalidRequest + * @param RequestInterface $request + */ + public function testStreamShouldEmitErrorBeforeCreatingConnectionWhenRequestIsInvalid(RequestInterface $request) + { + $connectionManager = $this->getMockBuilder('React\Http\Io\ClientConnectionManager')->disableOriginalConstructor()->getMock(); + $connectionManager->expects($this->never())->method('connect'); + + $stream = new ClientRequestStream($connectionManager, $request); + + $stream->on('error', $this->expectCallableOnceWith($this->isInstanceOf('InvalidArgumentException'))); + $stream->on('close', $this->expectCallableOnce()); + + $stream->end(); + } + /** @test */ public function requestShouldEmitErrorIfRequestParserThrowsException() { diff --git a/tests/Io/StreamingServerTest.php b/tests/Io/StreamingServerTest.php index afab371e..b4e3f2f8 100644 --- a/tests/Io/StreamingServerTest.php +++ b/tests/Io/StreamingServerTest.php @@ -2,6 +2,7 @@ namespace React\Tests\Http\Io; +use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use React\EventLoop\Loop; use React\Http\Io\StreamingServer; @@ -2511,7 +2512,7 @@ function ($data) use (&$buffer) { public function testInvalidCallbackFunctionLeadsToException() { $this->setExpectedException('InvalidArgumentException'); - $server = new StreamingServer(Loop::get(), 'invalid'); + new StreamingServer(Loop::get(), 'invalid'); } public function testResponseBodyStreamWillStreamDataWithChunkedTransferEncoding() @@ -2926,6 +2927,91 @@ function ($data) use (&$buffer) { $this->assertInstanceOf('RuntimeException', $exception); } + public static function provideInvalidResponse() + { + $response = new Response(200, array(), '', '1.1', 'OK'); + + return array( + array( + $response->withStatus(99, 'OK') + ), + array( + $response->withStatus(1000, 'OK') + ), + array( + $response->withStatus(200, "Invald\r\nReason: Yes") + ), + array( + $response->withHeader('Invalid', "Yes\r\n") + ), + array( + $response->withHeader('Invalid', "Yes\n") + ), + array( + $response->withHeader('Invalid', "Yes\r") + ), + array( + $response->withHeader("Inva\r\nlid", 'Yes') + ), + array( + $response->withHeader("Inva\nlid", 'Yes') + ), + array( + $response->withHeader("Inva\rlid", 'Yes') + ), + array( + $response->withHeader('Inva Lid', 'Yes') + ), + array( + $response->withHeader('Inva:Lid', 'Yes') + ), + array( + $response->withHeader('Invalid', "Val\0ue") + ), + array( + $response->withHeader("Inva\0lid", 'Yes') + ) + ); + } + + /** + * @dataProvider provideInvalidResponse + * @param ResponseInterface $response + */ + public function testInvalidResponseObjectWillResultInErrorMessage(ResponseInterface $response) + { + $server = new StreamingServer(Loop::get(), function (ServerRequestInterface $request) use ($response) { + return $response; + }); + + $exception = null; + $server->on('error', function (\Exception $ex) use (&$exception) { + $exception = $ex; + }); + + $buffer = ''; + $this->connection + ->expects($this->any()) + ->method('write') + ->will( + $this->returnCallback( + function ($data) use (&$buffer) { + $buffer .= $data; + } + ) + ); + + $server->listen($this->socket); + $this->socket->emit('connection', array($this->connection)); + + $data = $this->createGetRequest(); + + $this->connection->emit('data', array($data)); + + $this->assertContainsString("HTTP/1.1 500 Internal Server Error\r\n", $buffer); + $this->assertInstanceOf('InvalidArgumentException', $exception); + } + public function testRequestServerRequestParams() { $requestValidation = null;