Skip to content

Commit 8e5e77a

Browse files
clueclue
committed
Refactor to validate message boundaries while parsing request headers
1 parent 6e7dd66 commit 8e5e77a

File tree

4 files changed

+102
-309
lines changed

4 files changed

+102
-309
lines changed

‎src/Io/RequestHeaderParser.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,26 @@ public function parseRequest($headers, $remoteSocketUri, $localSocketUri)
199199
}
200200
}
201201

202+
// ensure message boundaries are valid according to Content-Length and Transfer-Encoding request headers
203+
if ($request->hasHeader('Transfer-Encoding')) {
204+
if (\strtolower($request->getHeaderLine('Transfer-Encoding')) !== 'chunked') {
205+
throw new \InvalidArgumentException('Only chunked-encoding is allowed for Transfer-Encoding', 501);
206+
}
207+
208+
// Transfer-Encoding: chunked and Content-Length header MUST NOT be used at the same time
209+
// as per https://tools.ietf.org/html/rfc7230#section-3.3.3
210+
if ($request->hasHeader('Content-Length')) {
211+
throw new \InvalidArgumentException('Using both `Transfer-Encoding: chunked` and `Content-Length` is not allowed', 400);
212+
}
213+
} elseif ($request->hasHeader('Content-Length')) {
214+
$string = $request->getHeaderLine('Content-Length');
215+
216+
if ((string)(int)$string !== $string) {
217+
// Content-Length value is not an integer or not a single integer
218+
throw new \InvalidArgumentException('The value of `Content-Length` is not valid', 400);
219+
}
220+
}
221+
202222
// set URI components from socket address if not already filled via Host header
203223
if ($request->getUri()->getHost() === '') {
204224
$parts = \parse_url($localSocketUri);

‎src/StreamingServer.php‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -185,30 +185,10 @@ public function handleRequest(ConnectionInterface $conn, ServerRequestInterface
185185
$contentLength = 0;
186186
$stream = new CloseProtectionStream($conn);
187187
if ($request->hasHeader('Transfer-Encoding')) {
188-
if (\strtolower($request->getHeaderLine('Transfer-Encoding')) !== 'chunked') {
189-
$this->emit('error', array(new \InvalidArgumentException('Only chunked-encoding is allowed for Transfer-Encoding')));
190-
return $this->writeError($conn, 501, $request);
191-
}
192-
193-
// Transfer-Encoding: chunked and Content-Length header MUST NOT be used at the same time
194-
// as per https://tools.ietf.org/html/rfc7230#section-3.3.3
195-
if ($request->hasHeader('Content-Length')) {
196-
$this->emit('error', array(new \InvalidArgumentException('Using both `Transfer-Encoding: chunked` and `Content-Length` is not allowed')));
197-
return $this->writeError($conn, 400, $request);
198-
}
199-
200-
$stream = new ChunkedDecoder($stream);
201188
$contentLength = null;
189+
$stream = new ChunkedDecoder($stream);
202190
} elseif ($request->hasHeader('Content-Length')) {
203-
$string = $request->getHeaderLine('Content-Length');
204-
205-
$contentLength = (int)$string;
206-
if ((string)$contentLength !== $string) {
207-
// Content-Length value is not an integer or not a single integer
208-
$this->emit('error', array(new \InvalidArgumentException('The value of `Content-Length` is not valid')));
209-
return $this->writeError($conn, 400, $request);
210-
}
211-
191+
$contentLength = (int)$request->getHeaderLine('Content-Length');
212192
$stream = new LengthLimitedStream($stream, $contentLength);
213193
}
214194

‎tests/Io/RequestHeaderParserTest.php‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,86 @@ public function testInvalidHttpVersion()
423423
$this->assertSame('Received request with invalid protocol version', $error->getMessage());
424424
}
425425

426+
public function testInvalidContentLengthRequestHeaderWillEmitError()
427+
{
428+
$error = null;
429+
430+
$parser = new RequestHeaderParser();
431+
$parser->on('headers', $this->expectCallableNever());
432+
$parser->on('error', function ($message) use (&$error) {
433+
$error = $message;
434+
});
435+
436+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
437+
$parser->handle($connection);
438+
439+
$connection->emit('data', array("GET / HTTP/1.1\r\nContent-Length: foo\r\n\r\n"));
440+
441+
$this->assertInstanceOf('InvalidArgumentException', $error);
442+
$this->assertSame(400, $error->getCode());
443+
$this->assertSame('The value of `Content-Length` is not valid', $error->getMessage());
444+
}
445+
446+
public function testInvalidRequestWithMultipleContentLengthRequestHeadersWillEmitError()
447+
{
448+
$error = null;
449+
450+
$parser = new RequestHeaderParser();
451+
$parser->on('headers', $this->expectCallableNever());
452+
$parser->on('error', function ($message) use (&$error) {
453+
$error = $message;
454+
});
455+
456+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
457+
$parser->handle($connection);
458+
459+
$connection->emit('data', array("GET / HTTP/1.1\r\nContent-Length: 4\r\nContent-Length: 5\r\n\r\n"));
460+
461+
$this->assertInstanceOf('InvalidArgumentException', $error);
462+
$this->assertSame(400, $error->getCode());
463+
$this->assertSame('The value of `Content-Length` is not valid', $error->getMessage());
464+
}
465+
466+
public function testInvalidTransferEncodingRequestHeaderWillEmitError()
467+
{
468+
$error = null;
469+
470+
$parser = new RequestHeaderParser();
471+
$parser->on('headers', $this->expectCallableNever());
472+
$parser->on('error', function ($message) use (&$error) {
473+
$error = $message;
474+
});
475+
476+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
477+
$parser->handle($connection);
478+
479+
$connection->emit('data', array("GET / HTTP/1.1\r\nTransfer-Encoding: foo\r\n\r\n"));
480+
481+
$this->assertInstanceOf('InvalidArgumentException', $error);
482+
$this->assertSame(501, $error->getCode());
483+
$this->assertSame('Only chunked-encoding is allowed for Transfer-Encoding', $error->getMessage());
484+
}
485+
486+
public function testInvalidRequestWithBothTransferEncodingAndContentLengthWillEmitError()
487+
{
488+
$error = null;
489+
490+
$parser = new RequestHeaderParser();
491+
$parser->on('headers', $this->expectCallableNever());
492+
$parser->on('error', function ($message) use (&$error) {
493+
$error = $message;
494+
});
495+
496+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(null)->getMock();
497+
$parser->handle($connection);
498+
499+
$connection->emit('data', array("GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\nContent-Length: 0\r\n\r\n"));
500+
501+
$this->assertInstanceOf('InvalidArgumentException', $error);
502+
$this->assertSame(400, $error->getCode());
503+
$this->assertSame('Using both `Transfer-Encoding: chunked` and `Content-Length` is not allowed', $error->getMessage());
504+
}
505+
426506
public function testServerParamsWillBeSetOnHttpsRequest()
427507
{
428508
$request = null;

0 commit comments

Comments
 (0)