Skip to content

Commit

Permalink
better implementation of headers handling
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
Tobion committed May 20, 2016
1 parent 41f2e9e commit 45054d9
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 141 deletions.
97 changes: 49 additions & 48 deletions src/MessageTrait.php
Expand Up @@ -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';
Expand All @@ -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)
Expand All @@ -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;
}
Expand All @@ -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;
}
}
}
Expand Down
23 changes: 14 additions & 9 deletions src/Request.php
Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
}
18 changes: 9 additions & 9 deletions src/Response.php
Expand Up @@ -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,
Expand All @@ -94,12 +94,12 @@ public function __construct(
) {
$this->statusCode = (int) $status;

if ($body !== null) {
if ($body != '') {

This comment has been minimized.

Copy link
@tjlytle

tjlytle Jun 7, 2016

I haven't dug in too far, but this change ($body != '' from $body !== null) has me getting back null because the StreamHandler creates a $sink, but doesn't drain the stream to it until after the response is created (so, when the response is created, casting the stream to a string results in an empty string).

This comment has been minimized.

Copy link
@SilverFire
$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;
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions tests/FunctionsTest.php
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion tests/RequestTest.php
Expand Up @@ -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'));
}

Expand Down

0 comments on commit 45054d9

Please sign in to comment.