From 81e997dfe72afa70614a057d865558fee1de4afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 18 Feb 2017 00:21:25 +0100 Subject: [PATCH 1/4] Response uses same HTTP protocol version as corresponding request --- README.md | 3 +++ src/Response.php | 13 +++++++--- src/Server.php | 2 +- tests/ResponseTest.php | 21 ++++++++++++++- tests/ServerTest.php | 58 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 88549055..be1d0dce 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,9 @@ It implements the `WritableStreamInterface`. The constructor is internal, you SHOULD NOT call this yourself. The `Server` is responsible for emitting `Request` and `Response` objects. +The `Response` will automatically use the same HTTP protocol version as the +corresponding `Request`. + See the above usage example and the class outline for details. #### writeContinue() diff --git a/src/Response.php b/src/Response.php index 91ae4358..541ff92d 100644 --- a/src/Response.php +++ b/src/Response.php @@ -14,6 +14,9 @@ * The constructor is internal, you SHOULD NOT call this yourself. * The `Server` is responsible for emitting `Request` and `Response` objects. * + * The `Response` will automatically use the same HTTP protocol version as the + * corresponding `Request`. + * * See the usage examples and the class outline for details. * * @see WritableStreamInterface @@ -21,9 +24,11 @@ */ class Response extends EventEmitter implements WritableStreamInterface { + private $conn; + private $protocolVersion; + private $closed = false; private $writable = true; - private $conn; private $headWritten = false; private $chunkedEncoding = true; @@ -36,9 +41,11 @@ class Response extends EventEmitter implements WritableStreamInterface * * @internal */ - public function __construct(ConnectionInterface $conn) + public function __construct(ConnectionInterface $conn, $protocolVersion = '1.1') { $this->conn = $conn; + $this->protocolVersion = $protocolVersion; + $that = $this; $this->conn->on('end', function () use ($that) { $that->close(); @@ -201,7 +208,7 @@ private function formatHead($status, array $headers) { $status = (int) $status; $text = isset(ResponseCodes::$statusTexts[$status]) ? ResponseCodes::$statusTexts[$status] : ''; - $data = "HTTP/1.1 $status $text\r\n"; + $data = "HTTP/$this->protocolVersion $status $text\r\n"; foreach ($headers as $name => $value) { $name = str_replace(array("\r", "\n"), '', $name); diff --git a/src/Server.php b/src/Server.php index 7ff523c1..93aaefdf 100644 --- a/src/Server.php +++ b/src/Server.php @@ -107,7 +107,7 @@ public function handleConnection(ConnectionInterface $conn) /** @internal */ public function handleRequest(ConnectionInterface $conn, Request $request) { - $response = new Response($conn); + $response = new Response($conn, $request->getProtocolVersion()); $response->on('close', array($request, 'close')); if (!$this->listeners('request')) { diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 27948504..470d8bed 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -26,6 +26,26 @@ public function testResponseShouldBeChunkedByDefault() $response->writeHead(); } + public function testResponseShouldUseGivenProtocolVersion() + { + $expected = ''; + $expected .= "HTTP/1.0 200 OK\r\n"; + $expected .= "X-Powered-By: React/alpha\r\n"; + $expected .= "Transfer-Encoding: chunked\r\n"; + $expected .= "\r\n"; + + $conn = $this + ->getMockBuilder('React\Socket\ConnectionInterface') + ->getMock(); + $conn + ->expects($this->once()) + ->method('write') + ->with($expected); + + $response = new Response($conn, '1.0'); + $response->writeHead(); + } + public function testResponseShouldBeChunkedEvenWithOtherTransferEncoding() { $expected = ''; @@ -46,7 +66,6 @@ public function testResponseShouldBeChunkedEvenWithOtherTransferEncoding() $response->writeHead(200, array('transfer-encoding' => 'custom')); } - public function testResponseShouldNotBeChunkedWithContentLength() { $expected = ''; diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 305bb31a..8b2d9c3d 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -202,6 +202,64 @@ function ($data) use (&$buffer) { $this->assertContains("\r\nX-Powered-By: React/alpha\r\n", $buffer); } + public function testResponseContainsSameRequestProtocolVersionForHttp11() + { + $server = new Server($this->socket); + $server->on('request', function (Request $request, Response $response) { + $response->writeHead(); + $response->end(); + }); + + $buffer = ''; + + $this->connection + ->expects($this->any()) + ->method('write') + ->will( + $this->returnCallback( + function ($data) use (&$buffer) { + $buffer .= $data; + } + ) + ); + + $this->socket->emit('connection', array($this->connection)); + + $data = "GET / HTTP/1.1\r\n\r\n"; + $this->connection->emit('data', array($data)); + + $this->assertContains("HTTP/1.1 200 OK\r\n", $buffer); + } + + public function testResponseContainsSameRequestProtocolVersionForHttp10() + { + $server = new Server($this->socket); + $server->on('request', function (Request $request, Response $response) { + $response->writeHead(); + $response->end(); + }); + + $buffer = ''; + + $this->connection + ->expects($this->any()) + ->method('write') + ->will( + $this->returnCallback( + function ($data) use (&$buffer) { + $buffer .= $data; + } + ) + ); + + $this->socket->emit('connection', array($this->connection)); + + $data = "GET / HTTP/1.0\r\n\r\n"; + $this->connection->emit('data', array($data)); + + $this->assertContains("HTTP/1.0 200 OK\r\n", $buffer); + } + public function testParserErrorEmitted() { $error = null; From d6192853c2d9987819b002f5c030f5d11e41895d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 18 Feb 2017 00:41:38 +0100 Subject: [PATCH 2/4] Apply chunked transfer encoding only for HTTP/1.1 responses by default --- README.md | 6 +++++- src/Response.php | 18 +++++++++--------- tests/ResponseTest.php | 3 +-- tests/ServerTest.php | 10 ++++++---- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index be1d0dce..ce27e302 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,10 @@ The `Server` is responsible for emitting `Request` and `Response` objects. The `Response` will automatically use the same HTTP protocol version as the corresponding `Request`. +HTTP/1.1 responses will automatically apply chunked transfer encoding if +no `Content-Length` header has been set. +See [`writeHead()`](#writehead) for more details. + See the above usage example and the class outline for details. #### writeContinue() @@ -237,7 +241,7 @@ $response->end('Hello World!'); Calling this method more than once will result in an `Exception`. -Unless you specify a `Content-Length` header yourself, the response message +Unless you specify a `Content-Length` header yourself, HTTP/1.1 responses will automatically use chunked transfer encoding and send the respective header (`Transfer-Encoding: chunked`) automatically. If you know the length of your body, you MAY specify it like this instead: diff --git a/src/Response.php b/src/Response.php index 541ff92d..e05cec2c 100644 --- a/src/Response.php +++ b/src/Response.php @@ -17,6 +17,10 @@ * The `Response` will automatically use the same HTTP protocol version as the * corresponding `Request`. * + * HTTP/1.1 responses will automatically apply chunked transfer encoding if + * no `Content-Length` header has been set. + * See `writeHead()` for more details. + * * See the usage examples and the class outline for details. * * @see WritableStreamInterface @@ -30,7 +34,7 @@ class Response extends EventEmitter implements WritableStreamInterface private $closed = false; private $writable = true; private $headWritten = false; - private $chunkedEncoding = true; + private $chunkedEncoding = false; /** * The constructor is internal, you SHOULD NOT call this yourself. @@ -129,7 +133,7 @@ public function writeContinue() * * Calling this method more than once will result in an `Exception`. * - * Unless you specify a `Content-Length` header yourself, the response message + * Unless you specify a `Content-Length` header yourself, HTTP/1.1 responses * will automatically use chunked transfer encoding and send the respective header * (`Transfer-Encoding: chunked`) automatically. If you know the length of your * body, you MAY specify it like this instead: @@ -174,11 +178,6 @@ public function writeHead($status = 200, array $headers = array()) $lower = array_change_key_case($headers); - // disable chunked encoding if content-length is given - if (isset($lower['content-length'])) { - $this->chunkedEncoding = false; - } - // assign default "X-Powered-By" header as first for history reasons if (!isset($lower['x-powered-by'])) { $headers = array_merge( @@ -187,8 +186,8 @@ public function writeHead($status = 200, array $headers = array()) ); } - // assign chunked transfer-encoding if chunked encoding is used - if ($this->chunkedEncoding) { + // assign chunked transfer-encoding if no 'content-length' is given for HTTP/1.1 responses + if (!isset($lower['content-length']) && $this->protocolVersion === '1.1') { foreach($headers as $name => $value) { if (strtolower($name) === 'transfer-encoding') { unset($headers[$name]); @@ -196,6 +195,7 @@ public function writeHead($status = 200, array $headers = array()) } $headers['Transfer-Encoding'] = 'chunked'; + $this->chunkedEncoding = true; } $data = $this->formatHead($status, $headers); diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 470d8bed..69a16001 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -26,12 +26,11 @@ public function testResponseShouldBeChunkedByDefault() $response->writeHead(); } - public function testResponseShouldUseGivenProtocolVersion() + public function testResponseShouldNotBeChunkedWhenProtocolVersionIsNot11() { $expected = ''; $expected .= "HTTP/1.0 200 OK\r\n"; $expected .= "X-Powered-By: React/alpha\r\n"; - $expected .= "Transfer-Encoding: chunked\r\n"; $expected .= "\r\n"; $conn = $this diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 8b2d9c3d..f72bfe97 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -202,12 +202,12 @@ function ($data) use (&$buffer) { $this->assertContains("\r\nX-Powered-By: React/alpha\r\n", $buffer); } - public function testResponseContainsSameRequestProtocolVersionForHttp11() + public function testResponseContainsSameRequestProtocolVersionAndChunkedBodyForHttp11() { $server = new Server($this->socket); $server->on('request', function (Request $request, Response $response) { $response->writeHead(); - $response->end(); + $response->end('bye'); }); $buffer = ''; @@ -229,14 +229,15 @@ function ($data) use (&$buffer) { $this->connection->emit('data', array($data)); $this->assertContains("HTTP/1.1 200 OK\r\n", $buffer); + $this->assertContains("\r\n\r\n3\r\nbye\r\n0\r\n\r\n", $buffer); } - public function testResponseContainsSameRequestProtocolVersionForHttp10() + public function testResponseContainsSameRequestProtocolVersionAndRawBodyForHttp10() { $server = new Server($this->socket); $server->on('request', function (Request $request, Response $response) { $response->writeHead(); - $response->end(); + $response->end('bye'); }); $buffer = ''; @@ -258,6 +259,7 @@ function ($data) use (&$buffer) { $this->connection->emit('data', array($data)); $this->assertContains("HTTP/1.0 200 OK\r\n", $buffer); + $this->assertContains("\r\n\r\nbye", $buffer); } public function testParserErrorEmitted() From 6ac0acbc1f44fce1c3bde4295879d3c752b08ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 18 Feb 2017 00:51:10 +0100 Subject: [PATCH 3/4] Ensure writeContinue() only works for HTTP/1.1 messages --- README.md | 15 +++++++++------ src/Response.php | 14 ++++++++++---- tests/ResponseTest.php | 14 ++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index ce27e302..7e53f7ae 100644 --- a/README.md +++ b/README.md @@ -217,12 +217,15 @@ $http->on('request', function (Request $request, Response $response) { }); ``` -Note that calling this method is strictly optional. -If you do not use it, then the client MUST continue sending the request body -after waiting some time. - -This method MUST NOT be invoked after calling `writeHead()`. -Calling this method after sending the headers will result in an `Exception`. +Note that calling this method is strictly optional for HTTP/1.1 responses. +If you do not use it, then a HTTP/1.1 client MUST continue sending the +request body after waiting some time. + +This method MUST NOT be invoked after calling [`writeHead()`](#writehead). +This method MUST NOT be invoked if this is not a HTTP/1.1 response +(please check [`expectsContinue()`](#expectscontinue) as above). +Calling this method after sending the headers or if this is not a HTTP/1.1 +response is an error that will result in an `Exception`. #### writeHead() diff --git a/src/Response.php b/src/Response.php index e05cec2c..952ae0d6 100644 --- a/src/Response.php +++ b/src/Response.php @@ -98,12 +98,15 @@ public function isWritable() * }); * ``` * - * Note that calling this method is strictly optional. - * If you do not use it, then the client MUST continue sending the request body - * after waiting some time. + * Note that calling this method is strictly optional for HTTP/1.1 responses. + * If you do not use it, then a HTTP/1.1 client MUST continue sending the + * request body after waiting some time. * * This method MUST NOT be invoked after calling `writeHead()`. - * Calling this method after sending the headers will result in an `Exception`. + * This method MUST NOT be invoked if this is not a HTTP/1.1 response + * (please check [`expectsContinue()`] as above). + * Calling this method after sending the headers or if this is not a HTTP/1.1 + * response is an error that will result in an `Exception`. * * @return void * @throws \Exception @@ -111,6 +114,9 @@ public function isWritable() */ public function writeContinue() { + if ($this->protocolVersion !== '1.1') { + throw new \Exception('Continue requires a HTTP/1.1 message'); + } if ($this->headWritten) { throw new \Exception('Response head has already been written.'); } diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 69a16001..6200934b 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -239,6 +239,20 @@ public function writeContinueShouldSendContinueLineBeforeRealHeaders() $response->writeHead(); } + /** + * @test + * @expectedException Exception + */ + public function writeContinueShouldThrowForHttp10() + { + $conn = $this + ->getMockBuilder('React\Socket\ConnectionInterface') + ->getMock(); + + $response = new Response($conn, '1.0'); + $response->writeContinue(); + } + /** @test */ public function shouldForwardEndDrainAndErrorEvents() { From 531c466c3dfaa1c16899214713eb229d4b6f827f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 17 Feb 2017 23:05:46 +0100 Subject: [PATCH 4/4] Only support HTTP/1.1 and HTTP/1.0 requests --- README.md | 6 ++++-- src/Server.php | 12 ++++++++++-- tests/ServerTest.php | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 7e53f7ae..26e37536 100644 --- a/README.md +++ b/README.md @@ -88,8 +88,10 @@ $http->on('request', function (Request $request, Response $response) { See also [`Request`](#request) and [`Response`](#response) for more details. -If a client sends an invalid request message, it will emit an `error` event, -send an HTTP error response to the client and close the connection: +The `Server` supports both HTTP/1.1 and HTTP/1.0 request messages. +If a client sends an invalid request message or uses an invalid HTTP protocol +version, it will emit an `error` event, send an HTTP error response to the +client and close the connection: ```php $http->on('error', function (Exception $e) { diff --git a/src/Server.php b/src/Server.php index 93aaefdf..cdc017f7 100644 --- a/src/Server.php +++ b/src/Server.php @@ -28,8 +28,10 @@ * * See also [`Request`](#request) and [`Response`](#response) for more details. * - * If a client sends an invalid request message, it will emit an `error` event, - * send an HTTP error response to the client and close the connection: + * The `Server` supports both HTTP/1.1 and HTTP/1.0 request messages. + * If a client sends an invalid request message or uses an invalid HTTP protocol + * version, it will emit an `error` event, send an HTTP error response to the + * client and close the connection: * * ```php * $http->on('error', function (Exception $e) { @@ -107,6 +109,12 @@ public function handleConnection(ConnectionInterface $conn) /** @internal */ public function handleRequest(ConnectionInterface $conn, Request $request) { + // only support HTTP/1.1 and HTTP/1.0 requests + if ($request->getProtocolVersion() !== '1.1' && $request->getProtocolVersion() !== '1.0') { + $this->emit('error', array(new \InvalidArgumentException('Received request with invalid protocol version'))); + return $this->writeError($conn, 505); + } + $response = new Response($conn, $request->getProtocolVersion()); $response->on('close', array($request, 'close')); diff --git a/tests/ServerTest.php b/tests/ServerTest.php index f72bfe97..bd0fdcf7 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -262,6 +262,38 @@ function ($data) use (&$buffer) { $this->assertContains("\r\n\r\nbye", $buffer); } + public function testRequestInvalidHttpProtocolVersionWillEmitErrorAndSendErrorResponse() + { + $error = null; + $server = new Server($this->socket); + $server->on('error', function ($message) use (&$error) { + $error = $message; + }); + + $buffer = ''; + + $this->connection + ->expects($this->any()) + ->method('write') + ->will( + $this->returnCallback( + function ($data) use (&$buffer) { + $buffer .= $data; + } + ) + ); + + $this->socket->emit('connection', array($this->connection)); + + $data = "GET / HTTP/1.2\r\nHost: localhost\r\n\r\n"; + $this->connection->emit('data', array($data)); + + $this->assertInstanceOf('InvalidArgumentException', $error); + + $this->assertContains("HTTP/1.1 505 HTTP Version Not Supported\r\n", $buffer); + $this->assertContains("\r\n\r\nError 505: HTTP Version Not Supported", $buffer); + } + public function testParserErrorEmitted() { $error = null;