From 45054d91c290de494549511e470ef5853f726a50 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Sat, 21 May 2016 00:19:24 +0200 Subject: [PATCH] better implementation of headers handling - fixes an edge case and is more logical - remove trimming of headers which is not according to psr-7 - fix '0' problem for response reason - improve tests --- src/MessageTrait.php | 97 +++++++++--------- src/Request.php | 23 +++-- src/Response.php | 18 ++-- tests/FunctionsTest.php | 4 +- tests/RequestTest.php | 3 +- tests/ResponseTest.php | 222 +++++++++++++++++++++++++++------------- 6 files changed, 226 insertions(+), 141 deletions(-) diff --git a/src/MessageTrait.php b/src/MessageTrait.php index 123205cf..f24f3fbd 100644 --- a/src/MessageTrait.php +++ b/src/MessageTrait.php @@ -8,11 +8,11 @@ */ trait MessageTrait { - /** @var array Cached HTTP header collection with lowercase key to values */ + /** @var array Map of all registered headers, as original name => array of values */ private $headers = []; - /** @var array Actual key to list of values per header. */ - private $headerLines = []; + /** @var array Map of lowercase header name => original name at registration */ + private $headerNames = []; /** @var string */ private $protocol = '1.1'; @@ -38,18 +38,25 @@ public function withProtocolVersion($version) public function getHeaders() { - return $this->headerLines; + return $this->headers; } public function hasHeader($header) { - return isset($this->headers[strtolower($header)]); + return isset($this->headerNames[strtolower($header)]); } public function getHeader($header) { - $name = strtolower($header); - return isset($this->headers[$name]) ? $this->headers[$name] : []; + $header = strtolower($header); + + if (!isset($this->headerNames[$header])) { + return []; + } + + $header = $this->headerNames[$header]; + + return $this->headers[$header]; } public function getHeaderLine($header) @@ -59,59 +66,54 @@ public function getHeaderLine($header) public function withHeader($header, $value) { - $new = clone $this; - $header = trim($header); - $name = strtolower($header); - if (!is_array($value)) { - $new->headers[$name] = [trim($value)]; - } else { - $new->headers[$name] = $value; - foreach ($new->headers[$name] as &$v) { - $v = trim($v); - } + $value = [$value]; } - // Remove the header lines. - foreach (array_keys($new->headerLines) as $key) { - if (strtolower($key) === $name) { - unset($new->headerLines[$key]); - } - } + $normalized = strtolower($header); - // Add the header line. - $new->headerLines[$header] = $new->headers[$name]; + $new = clone $this; + if (isset($new->headerNames[$normalized])) { + unset($new->headers[$new->headerNames[$normalized]]); + } + $new->headerNames[$normalized] = $header; + $new->headers[$header] = $value; return $new; } public function withAddedHeader($header, $value) { - if (!$this->hasHeader($header)) { + $normalized = strtolower($header); + + if (!isset($this->headerNames[$normalized])) { return $this->withHeader($header, $value); } + if (!is_array($value)) { + $value = [$value]; + } + + $header = $this->headerNames[$normalized]; + $new = clone $this; - $new->headers[strtolower($header)][] = $value; - $new->headerLines[$header][] = $value; + $new->headers[$header] = array_merge($this->headers[$header], $value); + return $new; } public function withoutHeader($header) { - if (!$this->hasHeader($header)) { + $normalized = strtolower($header); + + if (!isset($this->headerNames[$normalized])) { return $this; } - $new = clone $this; - $name = strtolower($header); - unset($new->headers[$name]); + $header = $this->headerNames[$normalized]; - foreach (array_keys($new->headerLines) as $key) { - if (strtolower($key) === $name) { - unset($new->headerLines[$key]); - } - } + $new = clone $this; + unset($new->headers[$header], $new->headerNames[$normalized]); return $new; } @@ -138,20 +140,19 @@ public function withBody(StreamInterface $body) private function setHeaders(array $headers) { - $this->headerLines = $this->headers = []; + $this->headerNames = $this->headers = []; foreach ($headers as $header => $value) { - $header = trim($header); - $name = strtolower($header); if (!is_array($value)) { - $value = trim($value); - $this->headers[$name][] = $value; - $this->headerLines[$header][] = $value; + $value = [$value]; + } + + $normalized = strtolower($header); + if (isset($this->headerNames[$normalized])) { + $header = $this->headerNames[$normalized]; + $this->headers[$header] = array_merge($this->headers[$header], $value); } else { - foreach ($value as $v) { - $v = trim($v); - $this->headers[$name][] = $v; - $this->headerLines[$header][] = $v; - } + $this->headerNames[$normalized] = $header; + $this->headers[$header] = $value; } } } diff --git a/src/Request.php b/src/Request.php index aa9b99da..a0fbe1c5 100644 --- a/src/Request.php +++ b/src/Request.php @@ -25,18 +25,18 @@ class Request implements RequestInterface private $uri; /** - * @param string $method HTTP method for the request. - * @param string|UriInterface $uri URI for the request. - * @param array $headers Headers for the message. - * @param string|null|resource|StreamInterface $body Message body. - * @param string $protocolVersion HTTP protocol version. + * @param string $method HTTP method. + * @param string|UriInterface $uri URI. + * @param array $headers Request headers. + * @param string|null|resource|StreamInterface $body Request body. + * @param string $version Protocol version. */ public function __construct( $method, $uri, array $headers = [], $body = null, - $protocolVersion = '1.1' + $version = '1.1' ) { if (!($uri instanceof UriInterface)) { $uri = new Uri($uri); @@ -45,7 +45,7 @@ public function __construct( $this->method = strtoupper($method); $this->uri = $uri; $this->setHeaders($headers); - $this->protocol = $protocolVersion; + $this->protocol = $version; if (!$this->hasHeader('Host')) { $this->updateHostFromUri(); @@ -138,9 +138,14 @@ private function updateHostFromUri() $host .= ':' . $port; } + if (isset($this->headerNames['host'])) { + $header = $this->headerNames['host']; + } else { + $header = 'Host'; + $this->headerNames['host'] = 'Host'; + } // Ensure Host is the first header. // See: http://tools.ietf.org/html/rfc7230#section-5.4 - $this->headerLines = ['Host' => [$host]] + $this->headerLines; - $this->headers = ['host' => [$host]] + $this->headers; + $this->headers = [$header => [$host]] + $this->headers; } } diff --git a/src/Response.php b/src/Response.php index 58c4c6a8..6e7115df 100644 --- a/src/Response.php +++ b/src/Response.php @@ -72,18 +72,18 @@ class Response implements ResponseInterface 511 => 'Network Authentication Required', ]; - /** @var null|string */ + /** @var string */ private $reasonPhrase = ''; /** @var int */ private $statusCode = 200; /** - * @param int $status Status code for the response, if any. - * @param array $headers Headers for the response, if any. - * @param mixed $body Stream body. - * @param string $version Protocol version. - * @param string $reason Reason phrase (a default will be used if possible). + * @param int $status Status code. + * @param array $headers Response headers. + * @param string|null|resource|StreamInterface $body Response body. + * @param string $version Protocol version. + * @param string|null $reason Reason phrase (when empty a default will be used based on the status code). */ public function __construct( $status = 200, @@ -94,12 +94,12 @@ public function __construct( ) { $this->statusCode = (int) $status; - if ($body !== null) { + if ($body != '') { $this->stream = stream_for($body); } $this->setHeaders($headers); - if (!$reason && isset(self::$phrases[$this->statusCode])) { + if ($reason == '' && isset(self::$phrases[$this->statusCode])) { $this->reasonPhrase = self::$phrases[$status]; } else { $this->reasonPhrase = (string) $reason; @@ -122,7 +122,7 @@ public function withStatus($code, $reasonPhrase = '') { $new = clone $this; $new->statusCode = (int) $code; - if (!$reasonPhrase && isset(self::$phrases[$new->statusCode])) { + if ($reasonPhrase == '' && isset(self::$phrases[$new->statusCode])) { $reasonPhrase = self::$phrases[$new->statusCode]; } $new->reasonPhrase = $reasonPhrase; diff --git a/tests/FunctionsTest.php b/tests/FunctionsTest.php index d428040f..664f5e88 100644 --- a/tests/FunctionsTest.php +++ b/tests/FunctionsTest.php @@ -439,7 +439,7 @@ public function testConvertsRequestsToStrings() { $request = new Psr7\Request('PUT', 'http://foo.com/hi?123', [ 'Baz' => 'bar', - 'Qux' => ' ipsum' + 'Qux' => 'ipsum' ], 'hello', '1.0'); $this->assertEquals( "PUT /hi?123 HTTP/1.0\r\nHost: foo.com\r\nBaz: bar\r\nQux: ipsum\r\n\r\nhello", @@ -451,7 +451,7 @@ public function testConvertsResponsesToStrings() { $response = new Psr7\Response(200, [ 'Baz' => 'bar', - 'Qux' => ' ipsum' + 'Qux' => 'ipsum' ], 'hello', '1.0', 'FOO'); $this->assertEquals( "HTTP/1.0 200 FOO\r\nBaz: bar\r\nQux: ipsum\r\n\r\nhello", diff --git a/tests/RequestTest.php b/tests/RequestTest.php index 0d2f8445..0144ab97 100644 --- a/tests/RequestTest.php +++ b/tests/RequestTest.php @@ -156,10 +156,11 @@ public function testOverridesHostWithUri() public function testAggregatesHeaders() { - $r = new Request('GET', 'http://foo.com', [ + $r = new Request('GET', '', [ 'ZOO' => 'zoobar', 'zoo' => ['foobar', 'zoobar'] ]); + $this->assertEquals(['ZOO' => ['zoobar', 'foobar', 'zoobar']], $r->getHeaders()); $this->assertEquals('zoobar, foobar, zoobar', $r->getHeaderLine('zoo')); } diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php index 0ce3e21e..c65175a8 100644 --- a/tests/ResponseTest.php +++ b/tests/ResponseTest.php @@ -10,137 +10,215 @@ */ class ResponseTest extends \PHPUnit_Framework_TestCase { - public function testAddsDefaultReason() + public function testDefaultConstructor() { - $r = new Response('200'); + $r = new Response(); $this->assertSame(200, $r->getStatusCode()); - $this->assertEquals('OK', $r->getReasonPhrase()); + $this->assertSame('1.1', $r->getProtocolVersion()); + $this->assertSame('OK', $r->getReasonPhrase()); + $this->assertSame([], $r->getHeaders()); + $this->assertInstanceOf('Psr\Http\Message\StreamInterface', $r->getBody()); + $this->assertSame('', (string) $r->getBody()); } - public function testCanGiveCustomReason() + public function testCanConstructWithStatusCode() { - $r = new Response(200, [], null, '1.1', 'bar'); - $this->assertEquals('bar', $r->getReasonPhrase()); + $r = new Response(404); + $this->assertSame(404, $r->getStatusCode()); + $this->assertSame('Not Found', $r->getReasonPhrase()); } - public function testCanGiveCustomProtocolVersion() + public function testStatusCanBeNumericString() { - $r = new Response(200, [], null, '1000'); - $this->assertEquals('1000', $r->getProtocolVersion()); + $r = new Response('404'); + $r2 = $r->withStatus('201'); + $this->assertSame(404, $r->getStatusCode()); + $this->assertSame('Not Found', $r->getReasonPhrase()); + $this->assertSame(201, $r2->getStatusCode()); + $this->assertSame('Created', $r2->getReasonPhrase()); } - public function testCanCreateNewResponseWithStatusAndNoReason() + public function testCanConstructWithHeaders() { - $r = new Response(200); - $r2 = $r->withStatus(201); - $this->assertEquals(200, $r->getStatusCode()); - $this->assertEquals('OK', $r->getReasonPhrase()); - $this->assertEquals(201, $r2->getStatusCode()); - $this->assertEquals('Created', $r2->getReasonPhrase()); + $r = new Response(200, ['Foo' => 'Bar']); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame('Bar', $r->getHeaderLine('Foo')); + $this->assertSame(['Bar'], $r->getHeader('Foo')); } - public function testCanCreateNewResponseWithStatusAndReason() + public function testCanConstructWithHeadersAsArray() { - $r = new Response(200); - $r2 = $r->withStatus(201, 'Foo'); - $this->assertEquals(200, $r->getStatusCode()); - $this->assertEquals('OK', $r->getReasonPhrase()); - $this->assertEquals(201, $r2->getStatusCode()); - $this->assertEquals('Foo', $r2->getReasonPhrase()); + $r = new Response(200, [ + 'Foo' => ['baz', 'bar'] + ]); + $this->assertSame(['Foo' => ['baz', 'bar']], $r->getHeaders()); + $this->assertSame('baz, bar', $r->getHeaderLine('Foo')); + $this->assertSame(['baz', 'bar'], $r->getHeader('Foo')); } - public function testCreatesResponseWithAddedHeaderArray() + public function testCanConstructWithBody() { - $r = new Response(); - $r2 = $r->withAddedHeader('foo', ['baz', 'bar']); - $this->assertFalse($r->hasHeader('foo')); - $this->assertEquals('baz, bar', $r2->getHeaderLine('foo')); + $r = new Response(200, [], 'baz'); + $this->assertInstanceOf('Psr\Http\Message\StreamInterface', $r->getBody()); + $this->assertSame('baz', (string) $r->getBody()); } - public function testReturnsIdentityWhenRemovingMissingHeader() + public function testNullBody() { - $r = new Response(); - $this->assertSame($r, $r->withoutHeader('foo')); + $r = new Response(200, [], null); + $this->assertInstanceOf('Psr\Http\Message\StreamInterface', $r->getBody()); + $this->assertSame('', (string) $r->getBody()); } - public function testAlwaysReturnsBody() + public function testFalseyBody() { - $r = new Response(); + $r = new Response(200, [], '0'); $this->assertInstanceOf('Psr\Http\Message\StreamInterface', $r->getBody()); + $this->assertSame('0', (string) $r->getBody()); } - public function testCanSetHeaderAsArray() + public function testCanConstructWithReason() { - $r = new Response(200, [ - 'foo' => ['baz ', ' bar '] - ]); - $this->assertEquals('baz, bar', $r->getHeaderLine('foo')); - $this->assertEquals(['baz', 'bar'], $r->getHeader('foo')); + $r = new Response(200, [], null, '1.1', 'bar'); + $this->assertSame('bar', $r->getReasonPhrase()); + + $r = new Response(200, [], null, '1.1', '0'); + $this->assertSame('0', $r->getReasonPhrase(), 'Falsey reason works'); } - public function testSameInstanceWhenSameBody() + public function testCanConstructWithProtocolVersion() { - $r = new Response(200, [], 'foo'); - $b = $r->getBody(); - $this->assertSame($r, $r->withBody($b)); + $r = new Response(200, [], null, '1000'); + $this->assertSame('1000', $r->getProtocolVersion()); + } + + public function testWithStatusCodeAndNoReason() + { + $r = (new Response())->withStatus(201); + $this->assertSame(201, $r->getStatusCode()); + $this->assertSame('Created', $r->getReasonPhrase()); + } + + public function testWithStatusCodeAndReason() + { + $r = (new Response())->withStatus(201, 'Foo'); + $this->assertSame(201, $r->getStatusCode()); + $this->assertSame('Foo', $r->getReasonPhrase()); + + $r = (new Response())->withStatus(201, '0'); + $this->assertSame(201, $r->getStatusCode()); + $this->assertSame('0', $r->getReasonPhrase(), 'Falsey reason works'); } - public function testNewInstanceWhenNewBody() + public function testWithProtocolVersion() { - $r = new Response(200, [], 'foo'); - $b2 = Psr7\stream_for('abc'); - $this->assertNotSame($r, $r->withBody($b2)); + $r = (new Response())->withProtocolVersion('1000'); + $this->assertSame('1000', $r->getProtocolVersion()); } public function testSameInstanceWhenSameProtocol() { - $r = new Response(200); + $r = new Response(); $this->assertSame($r, $r->withProtocolVersion('1.1')); } - public function testNewInstanceWhenNewProtocol() + public function testWithBody() { - $r = new Response(200); - $this->assertNotSame($r, $r->withProtocolVersion('1.0')); + $b = Psr7\stream_for('0'); + $r = (new Response())->withBody($b); + $this->assertInstanceOf('Psr\Http\Message\StreamInterface', $r->getBody()); + $this->assertSame('0', (string) $r->getBody()); } - public function testNewInstanceWhenRemovingHeader() + public function testSameInstanceWhenSameBody() + { + $r = new Response(); + $b = $r->getBody(); + $this->assertSame($r, $r->withBody($b)); + } + + public function testWithHeader() { $r = new Response(200, ['Foo' => 'Bar']); - $r2 = $r->withoutHeader('Foo'); - $this->assertNotSame($r, $r2); - $this->assertFalse($r2->hasHeader('foo')); + $r2 = $r->withHeader('baZ', 'Bam'); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['Foo' => ['Bar'], 'baZ' => ['Bam']], $r2->getHeaders()); + $this->assertSame('Bam', $r2->getHeaderLine('baz')); + $this->assertSame(['Bam'], $r2->getHeader('baz')); } - public function testNewInstanceWhenAddingHeader() + public function testWithHeaderAsArray() { $r = new Response(200, ['Foo' => 'Bar']); - $r2 = $r->withAddedHeader('Foo', 'Baz'); - $this->assertNotSame($r, $r2); - $this->assertEquals('Bar, Baz', $r2->getHeaderLine('foo')); + $r2 = $r->withHeader('baZ', ['Bam', 'Bar']); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['Foo' => ['Bar'], 'baZ' => ['Bam', 'Bar']], $r2->getHeaders()); + $this->assertSame('Bam, Bar', $r2->getHeaderLine('baz')); + $this->assertSame(['Bam', 'Bar'], $r2->getHeader('baz')); } - public function testNewInstanceWhenAddingHeaderThatWasNotThereBefore() + public function testWithHeaderReplacesDifferentCase() { $r = new Response(200, ['Foo' => 'Bar']); - $r2 = $r->withAddedHeader('Baz', 'Bam'); - $this->assertNotSame($r, $r2); - $this->assertEquals('Bam', $r2->getHeaderLine('Baz')); - $this->assertEquals('Bar', $r2->getHeaderLine('Foo')); + $r2 = $r->withHeader('foO', 'Bam'); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['foO' => ['Bam']], $r2->getHeaders()); + $this->assertSame('Bam', $r2->getHeaderLine('foo')); + $this->assertSame(['Bam'], $r2->getHeader('foo')); } - public function testRemovesPreviouslyAddedHeaderOfDifferentCase() + public function testWithAddedHeader() { $r = new Response(200, ['Foo' => 'Bar']); - $r2 = $r->withHeader('foo', 'Bam'); - $this->assertNotSame($r, $r2); - $this->assertEquals('Bam', $r2->getHeaderLine('Foo')); + $r2 = $r->withAddedHeader('foO', 'Baz'); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['Foo' => ['Bar', 'Baz']], $r2->getHeaders()); + $this->assertSame('Bar, Baz', $r2->getHeaderLine('foo')); + $this->assertSame(['Bar', 'Baz'], $r2->getHeader('foo')); } - public function testBodyConsistent() + public function testWithAddedHeaderAsArray() { - $r = new Response(200, [], '0'); - $this->assertEquals('0', (string)$r->getBody()); + $r = new Response(200, ['Foo' => 'Bar']); + $r2 = $r->withAddedHeader('foO', ['Baz', 'Bam']); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['Foo' => ['Bar', 'Baz', 'Bam']], $r2->getHeaders()); + $this->assertSame('Bar, Baz, Bam', $r2->getHeaderLine('foo')); + $this->assertSame(['Bar', 'Baz', 'Bam'], $r2->getHeader('foo')); + } + + public function testWithAddedHeaderThatDoesNotExist() + { + $r = new Response(200, ['Foo' => 'Bar']); + $r2 = $r->withAddedHeader('nEw', 'Baz'); + $this->assertSame(['Foo' => ['Bar']], $r->getHeaders()); + $this->assertSame(['Foo' => ['Bar'], 'nEw' => ['Baz']], $r2->getHeaders()); + $this->assertSame('Baz', $r2->getHeaderLine('new')); + $this->assertSame(['Baz'], $r2->getHeader('new')); + } + + public function testWithoutHeaderThatExists() + { + $r = new Response(200, ['Foo' => 'Bar', 'Baz' => 'Bam']); + $r2 = $r->withoutHeader('foO'); + $this->assertTrue($r->hasHeader('foo')); + $this->assertSame(['Foo' => ['Bar'], 'Baz' => ['Bam']], $r->getHeaders()); + $this->assertFalse($r2->hasHeader('foo')); + $this->assertSame(['Baz' => ['Bam']], $r2->getHeaders()); + } + + public function testWithoutHeaderThatDoesNotExist() + { + $r = new Response(200, ['Baz' => 'Bam']); + $r2 = $r->withoutHeader('foO'); + $this->assertSame($r, $r2); + $this->assertFalse($r2->hasHeader('foo')); + $this->assertSame(['Baz' => ['Bam']], $r2->getHeaders()); + } + + public function testSameInstanceWhenRemovingMissingHeader() + { + $r = new Response(); + $this->assertSame($r, $r->withoutHeader('foo')); } - }