Skip to content

Commit

Permalink
Merge pull request #523 from clue-labs/http-validate
Browse files Browse the repository at this point in the history 
Validate outgoing HTTP message headers and reject invalid messages
  • Loading branch information
@WyriHaximus
WyriHaximus committed Mar 27, 2024
2 parents 6212f3e + 27d2e74 commit 04cb936
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/Io/AbstractMessage.php
View file
Expand Up @@ -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<string,string[]> */
private $headers = array();
Expand Down
30 changes: 20 additions & 10 deletions src/Io/ClientRequestStream.php
View file
Expand Up @@ -56,15 +56,33 @@ 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;
$that = $this;

$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);

Expand All @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions src/Io/StreamingServer.php
View file
Expand Up @@ -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) {
Expand Down
62 changes: 62 additions & 0 deletions tests/Io/ClientRequestStreamTest.php
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down
88 changes: 87 additions & 1 deletion tests/Io/StreamingServerTest.php
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 04cb936

Please sign in to comment.