Skip to content
Permalink
Browse files

Fix a number of header related issues and clean up tests.

Unfortunately this spiraled a bit out of control as I wanted to keep the
tests passing. However, I've:

* Fixed getHeaders() not always following PSR7 interface requirements
  when headers are set with `header()`.
* Restored behavior where the status code is set to 302 when a location
  header is set and the current status is 200.
* Eagerly set the 'Content-Type' header so that charset(), type() and
  withType() all result in the content-type header being correctly set.
  This also fixes an issue where the content type would not be correctly
  set when the response was emitted.
  • Loading branch information...
markstory committed Nov 18, 2016
1 parent ea0e454 commit 5ff002e13c480075024aa1ecc713c33fa89cbfc9
Showing with 262 additions and 293 deletions.
  1. +94 −20 src/Network/Response.php
  2. +168 −273 tests/TestCase/Network/ResponseTest.php
@@ -454,13 +454,14 @@ public function __construct(array $options = [])
if (isset($options['status'])) {
$this->statusCode($options['status']);
}
if (isset($options['type'])) {
$this->type($options['type']);
}
if (!isset($options['charset'])) {
$options['charset'] = Configure::read('App.encoding');
}
$this->charset($options['charset']);
$this->_charset = $options['charset'];
if (isset($options['type'])) {
$this->_contentType = $this->resolveType($options['type']);
}
$this->_setContentType();
}
/**
@@ -519,7 +520,10 @@ public function sendHeaders()
$codeMessage = $this->_statusCodes[$this->_status];
$this->_setCookies();
$this->_sendHeader("{$this->_protocol} {$this->_status} {$codeMessage}");
$this->_setContentType();
if (in_array($this->_status, [304, 204])) {
$this->_setContentType();
}
foreach ($this->headers as $header => $values) {
foreach ((array)$values as $value) {
@@ -559,9 +563,6 @@ protected function _setCookies()
*/
protected function _setContentType()
{
if (in_array($this->_status, [304, 204])) {
return;
}
$whitelist = [
'application/javascript', 'application/json', 'application/xml', 'application/rss+xml'
];
@@ -675,7 +676,7 @@ protected function _sendContent($content)
public function header($header = null, $value = null)
{
if ($header === null) {
return $this->headers;
return $this->getSimpleHeaders();
}
$headers = is_array($header) ? $header : [$header => $value];
@@ -687,16 +688,36 @@ public function header($header = null, $value = null)
list($header, $value) = explode(':', $header, 2);
}
if ($this->hasHeader($header)) {
$header = $this->headerNames[strtolower($header)];
$lower = strtolower($header);
if (array_key_exists($lower, $this->headerNames)) {
$header = $this->headerNames[$lower];
} else {
$this->headerNames[strtolower($header)] = $header;
$this->headerNames[$lower] = $header;
}
$this->headers[$header] = is_array($value) ? array_map('trim', $value) : trim($value);
$this->headers[$header] = is_array($value) ? array_map('trim', $value) : [trim($value)];
}
return $this->headers;
return $this->getSimpleHeaders();
}
/**
* Backwards compatibility helper for getting flattened headers.
*
* Previously CakePHP would store headers as a simple dictionary, now that
* we're supporting PSR7, the internal storage has each header as an array.
*
* @return array
*/
protected function getSimpleHeaders()
{
$out = [];
foreach ($this->headers as $key => $values) {
$header = $this->headerNames[strtolower($key)];
$out[$header] = implode(',', $values);
}
return $out;
}
/**
@@ -720,20 +741,31 @@ public function location($url = null)
return $result;
}
if ($this->_status === 200) {
$this->_status = 302;
}
$this->_setHeader('Location', $url);
return null;
}
/**
* Return an instance with an updated location header
* Return an instance with an updated location header.
*
* If the current status code is 200, it will be replaced
* with 302.
*
* @param string $url The location to redirect to.
* @return static A new response with the Location header set.
*/
public function withLocation($url)
{
return $this->withHeader('Location', $url);
$new = $this->withHeader('Location', $url);
if ($new->_status === 200) {
$new->_status = 302;
}
return $new;
}
/**
@@ -747,7 +779,7 @@ protected function _setHeader($header, $value)
{
$normalized = strtolower($header);
$this->headerNames[$normalized] = $header;
$this->headers[$header] = $value;
$this->headers[$header] = [$value];
}
/**
@@ -810,7 +842,7 @@ protected function _handleCallableBody(callable $content)
* @param int|null $code the HTTP status code
* @return int Current status code
* @throws \InvalidArgumentException When an unknown status code is reached.
* @deprecated 3.4.0 Use `getStatusCode()` and `withStatusCode()` instead.
* @deprecated 3.4.0 Use `getStatusCode()` and `withStatus()` instead.
*/
public function statusCode($code = null)
{
@@ -994,7 +1026,47 @@ public function type($contentType = null)
return false;
}
return $this->_contentType = $contentType;
$this->_contentType = $contentType;
$this->_setContentType();
return $contentType;
}
/**
* Get an updated response with the content type set.
*
* Either a file extension which will be mapped to a mime-type or a concrete mime-type
*
* @param string $contentType Content type key alias or mime-type
* @return static
*/
public function withType($contentType)
{
$mappedType = $this->resolveType($contentType);
$new = clone $this;
$new->_contentType = $mappedType;
$new->_setContentType();
return $new;
}
/**
* Translate and validate content-types.
*
* @param string $contentType The content-type or type alias.
* @return string The resolved content-type
* @throws \InvalidArgumentException When an invalid content-type or alias is used.
*/
protected function resolveType($contentType)
{
$mapped = $this->getMimeType($contentType);
if ($mapped) {
return is_array($mapped) ? current($mapped) : $mapped;
}
if (strpos($contentType, '/') === false) {
throw new InvalidArgumentException(sprintf('"%s" is an invalid content type.', $contentType));
}
return $contentType;
}
/**
@@ -1050,7 +1122,9 @@ public function charset($charset = null)
return $this->_charset;
}
return $this->_charset = $charset;
$this->_charset = $charset;
$this->_setContentType();
return $this->_charset;
}
/**
Oops, something went wrong.

0 comments on commit 5ff002e

Please sign in to comment.
You can’t perform that action at this time.