From fbd1c5cc59b7e8e5d2ff1ee240fd20d088972633 Mon Sep 17 00:00:00 2001 From: Bastian Date: Fri, 15 Dec 2017 11:14:18 +0100 Subject: [PATCH 1/5] BUGFIX: Throw exception when passing objects to `Headers::set()` According to PSR-7 (http://www.php-fig.org/psr/psr-7/) HTTP headers can only accept values of `string` or `string[]`. This adjusts our `Headers` implementation to throw an exception if an object is passed to the `set()` method in order to make error handling easier. Exceptions prove the rule so we still do allow instances of `\DateTimeInterface` to be set as header value. --- Neos.Flow/Classes/Http/Headers.php | 6 ++- Neos.Flow/Tests/Unit/Http/HeadersTest.php | 57 ++++++++++++++++++++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/Neos.Flow/Classes/Http/Headers.php b/Neos.Flow/Classes/Http/Headers.php index 60b46a66c2..e174be3a85 100644 --- a/Neos.Flow/Classes/Http/Headers.php +++ b/Neos.Flow/Classes/Http/Headers.php @@ -94,7 +94,7 @@ public static function createFromServer(array $server) * GMT previously. GMT is used synonymously with UTC as per RFC 2616 3.3.1. * * @param string $name Name of the header, for example "Location", "Content-Description" etc. - * @param array|string|\DateTime $values An array of values or a single value for the specified header field + * @param string|string[]|\DateTime $values An array of values or a single value for the specified header field * @param boolean $replaceExistingHeader If a header with the same name should be replaced. Default is TRUE. * @return void * @throws \InvalidArgumentException @@ -110,8 +110,10 @@ public function set($name, $values, $replaceExistingHeader = true) $date = clone $values; $date->setTimezone(new \DateTimeZone('GMT')); $values = [$date->format('D, d M Y H:i:s') . ' GMT']; - } else { + } elseif (is_string($values)) { $values = (array) $values; + } elseif (!is_array($values)) { + throw new \InvalidArgumentException(sprintf('The header value must be a string, string array or an instance of DateTimeInterface, given: %s', is_object($values) ? get_class($values) : gettype($values)), 1513332376); } switch ($name) { diff --git a/Neos.Flow/Tests/Unit/Http/HeadersTest.php b/Neos.Flow/Tests/Unit/Http/HeadersTest.php index 1d62b96aff..d861d3907d 100644 --- a/Neos.Flow/Tests/Unit/Http/HeadersTest.php +++ b/Neos.Flow/Tests/Unit/Http/HeadersTest.php @@ -149,6 +149,16 @@ public function setGetAndGetAllConvertDatesFromDateObjectsToStringAndViceVersa() $this->assertEquals($nowInGmt->format(DATE_RFC2822), $headers->get('X-Test-Run-At')->format(DATE_RFC2822)); } + /** + * @test + * @expectedException \InvalidArgumentException + */ + public function setThrowsExceptionWhenValueIsAnObject() + { + $headers = new Headers(); + $headers->set('X-Test', new \stdClass()); + } + /** * @test */ @@ -328,7 +338,7 @@ public function setCacheControlDirectiveSetsVisibilityCorrectly() public function setExceptsHttpsHeaders() { $headers = new Headers(); - $headers->set('HTTPS', 1); + $headers->set('HTTPS', '1'); // dummy assertion to suppress PHPUnit warning $this->assertTrue(true); @@ -471,4 +481,49 @@ public function getCacheControlDirectiveReturnsTheSpecifiedDirectiveValueIfPrese } $this->assertEquals($value, $headers->getCacheControlDirective($name)); } + + /** + * @test + */ + public function getPreparedValuesRendersStringsAsIs() + { + $headers = new Headers(); + $headers->set('X-String', 'Some String'); + + $expectedResult = [ + 'X-String: Some String', + ]; + $this->assertSame($expectedResult, $headers->getPreparedValues()); + } + + /** + * @test + */ + public function getPreparedValuesRendersDateTimeInstancesAsGmtString() + { + $headers = new Headers(); + $headers->set('X-Date', new \DateTimeImmutable('1980-12-13')); + + $expectedResult = [ + 'X-Date: Sat, 13 Dec 1980 00:00:00 GMT', + ]; + $this->assertSame($expectedResult, $headers->getPreparedValues()); + } + + /** + * @test + */ + public function getPreparedValuesRendersOneHeaderPerArrayItem() + { + $headers = new Headers(); + $headers->set('X-Array', ['some', 'array', 123]); + + $expectedResult = [ + 'X-Array: some', + 'X-Array: array', + 'X-Array: 123', + ]; + $this->assertSame($expectedResult, $headers->getPreparedValues()); + } + } From 592e8fe0928eca07c6e13443215d1c77a87a21ef Mon Sep 17 00:00:00 2001 From: Bastian Date: Fri, 15 Dec 2017 11:15:35 +0100 Subject: [PATCH 2/5] TASK: Fix code style to satisfy StyleCI checks --- Neos.Flow/Tests/Unit/Http/HeadersTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/Neos.Flow/Tests/Unit/Http/HeadersTest.php b/Neos.Flow/Tests/Unit/Http/HeadersTest.php index d861d3907d..55687c3e05 100644 --- a/Neos.Flow/Tests/Unit/Http/HeadersTest.php +++ b/Neos.Flow/Tests/Unit/Http/HeadersTest.php @@ -525,5 +525,4 @@ public function getPreparedValuesRendersOneHeaderPerArrayItem() ]; $this->assertSame($expectedResult, $headers->getPreparedValues()); } - } From 6908caae2acd9e816418f933c7019c694c8fa64d Mon Sep 17 00:00:00 2001 From: Bastian Date: Tue, 17 Apr 2018 16:01:27 +0200 Subject: [PATCH 3/5] TASK: Strict type checking for string arrays --- Neos.Flow/Classes/Http/Headers.php | 10 ++++++-- Neos.Flow/Tests/Unit/Http/HeadersTest.php | 30 ++++++++++++++++++++--- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/Neos.Flow/Classes/Http/Headers.php b/Neos.Flow/Classes/Http/Headers.php index e174be3a85..99b1afd852 100644 --- a/Neos.Flow/Classes/Http/Headers.php +++ b/Neos.Flow/Classes/Http/Headers.php @@ -111,8 +111,14 @@ public function set($name, $values, $replaceExistingHeader = true) $date->setTimezone(new \DateTimeZone('GMT')); $values = [$date->format('D, d M Y H:i:s') . ' GMT']; } elseif (is_string($values)) { - $values = (array) $values; - } elseif (!is_array($values)) { + $values = [$values]; + } elseif (is_array($values)) { + array_walk($values, function($item) { + if (!is_string($item)) { + throw new \InvalidArgumentException(sprintf('The header value must be a string, string array or an instance of DateTimeInterface, but the given array contains an instance of %s', is_object($item) ? get_class($item) : gettype($item)), 1523973518); + } + }); + } else { throw new \InvalidArgumentException(sprintf('The header value must be a string, string array or an instance of DateTimeInterface, given: %s', is_object($values) ? get_class($values) : gettype($values)), 1513332376); } diff --git a/Neos.Flow/Tests/Unit/Http/HeadersTest.php b/Neos.Flow/Tests/Unit/Http/HeadersTest.php index 55687c3e05..435dac479b 100644 --- a/Neos.Flow/Tests/Unit/Http/HeadersTest.php +++ b/Neos.Flow/Tests/Unit/Http/HeadersTest.php @@ -149,16 +149,40 @@ public function setGetAndGetAllConvertDatesFromDateObjectsToStringAndViceVersa() $this->assertEquals($nowInGmt->format(DATE_RFC2822), $headers->get('X-Test-Run-At')->format(DATE_RFC2822)); } + public function setThrowsExceptionForInvalidValuesDataProvider() + { + return [ + [new \stdClass()], + [1234], + [true], + [['foo', 'bar', 123]], + ]; + } + /** * @test * @expectedException \InvalidArgumentException + * @dataProvider setThrowsExceptionForInvalidValuesDataProvider */ - public function setThrowsExceptionWhenValueIsAnObject() + public function setThrowsExceptionForInvalidValues($value) { $headers = new Headers(); - $headers->set('X-Test', new \stdClass()); + $headers->set('X-Test', $value); } + /** + * @test + * @expectedException \InvalidArgumentException + */ + public function setThrowsExceptionWhenValueIsAnInteger() + { + $headers = new Headers(); + $headers->set('X-Test', 123); + } + + + #$headers->set('X-Array', ['some', 'array', 123]); + /** * @test */ @@ -516,7 +540,7 @@ public function getPreparedValuesRendersDateTimeInstancesAsGmtString() public function getPreparedValuesRendersOneHeaderPerArrayItem() { $headers = new Headers(); - $headers->set('X-Array', ['some', 'array', 123]); + $headers->set('X-Array', ['some', 'array', '123']); $expectedResult = [ 'X-Array: some', From e24df556c65bd988aebbc654e31f4c7207a6ce62 Mon Sep 17 00:00:00 2001 From: Bastian Date: Tue, 17 Apr 2018 16:02:33 +0200 Subject: [PATCH 4/5] TASK: Fix code style to satisfy StyleCI checks --- Neos.Flow/Classes/Http/Headers.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Neos.Flow/Classes/Http/Headers.php b/Neos.Flow/Classes/Http/Headers.php index 99b1afd852..8d6604810d 100644 --- a/Neos.Flow/Classes/Http/Headers.php +++ b/Neos.Flow/Classes/Http/Headers.php @@ -113,7 +113,7 @@ public function set($name, $values, $replaceExistingHeader = true) } elseif (is_string($values)) { $values = [$values]; } elseif (is_array($values)) { - array_walk($values, function($item) { + array_walk($values, function ($item) { if (!is_string($item)) { throw new \InvalidArgumentException(sprintf('The header value must be a string, string array or an instance of DateTimeInterface, but the given array contains an instance of %s', is_object($item) ? get_class($item) : gettype($item)), 1523973518); } From 3b7560dec82109aa8235e946fe1b4c18ed5eeda0 Mon Sep 17 00:00:00 2001 From: Bastian Date: Tue, 17 Apr 2018 16:21:06 +0200 Subject: [PATCH 5/5] BUGFIX: Adjusts tests and framework code to cast non-string headers! --- Neos.Flow/Classes/Http/AbstractMessage.php | 2 +- Neos.Flow/Classes/Http/BaseRequest.php | 2 +- Neos.Flow/Classes/Http/Headers.php | 4 ++-- Neos.Flow/Classes/Http/Response.php | 4 ++-- Neos.Flow/Tests/Unit/Http/ResponseTest.php | 12 ++++++------ 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Neos.Flow/Classes/Http/AbstractMessage.php b/Neos.Flow/Classes/Http/AbstractMessage.php index e05fecc0e8..afaa3774a1 100644 --- a/Neos.Flow/Classes/Http/AbstractMessage.php +++ b/Neos.Flow/Classes/Http/AbstractMessage.php @@ -107,7 +107,7 @@ public function hasHeader($name) * GMT previously. GMT is used synonymously with UTC as per RFC 2616 3.3.1. * * @param string $name Name of the header, for example "Location", "Content-Description" etc. - * @param array|string|\DateTime $values An array of values or a single value for the specified header field + * @param array|string|\DateTimeInterface $values An array of strings or a single string/DateTime for the specified header field * @param boolean $replaceExistingHeader If a header with the same name should be replaced. Default is TRUE. * @return self This message, for method chaining * @throws \InvalidArgumentException diff --git a/Neos.Flow/Classes/Http/BaseRequest.php b/Neos.Flow/Classes/Http/BaseRequest.php index 3021542047..5392664655 100644 --- a/Neos.Flow/Classes/Http/BaseRequest.php +++ b/Neos.Flow/Classes/Http/BaseRequest.php @@ -152,7 +152,7 @@ public function setContent($content) { if (is_resource($content) && get_resource_type($content) === 'stream' && stream_is_local($content)) { $streamMetaData = stream_get_meta_data($content); - $this->headers->set('Content-Length', filesize($streamMetaData['uri'])); + $this->headers->set('Content-Length', (string)filesize($streamMetaData['uri'])); $this->headers->set('Content-Type', MediaTypes::getMediaTypeFromFilename($streamMetaData['uri'])); } diff --git a/Neos.Flow/Classes/Http/Headers.php b/Neos.Flow/Classes/Http/Headers.php index 8d6604810d..1839e7abc8 100644 --- a/Neos.Flow/Classes/Http/Headers.php +++ b/Neos.Flow/Classes/Http/Headers.php @@ -72,12 +72,12 @@ public static function createFromServer(array $server) foreach ($server as $name => $value) { if (strpos($name, 'HTTP_') === 0) { $name = str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', substr($name, 5))))); - $headerFields[$name] = $value; + $headerFields[$name] = (string)$value; } elseif ($name == 'REDIRECT_REMOTE_AUTHORIZATION' && !isset($headerFields['Authorization'])) { $headerFields['Authorization'] = $value; } elseif (in_array($name, ['CONTENT_TYPE', 'CONTENT_LENGTH'])) { $name = str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', $name)))); - $headerFields[$name] = $value; + $headerFields[$name] = (string)$value; } } return new self($headerFields); diff --git a/Neos.Flow/Classes/Http/Response.php b/Neos.Flow/Classes/Http/Response.php index 00eea92589..4ede8d661a 100644 --- a/Neos.Flow/Classes/Http/Response.php +++ b/Neos.Flow/Classes/Http/Response.php @@ -563,13 +563,13 @@ public function makeStandardsCompliant(Request $request) if ($request->getMethod() === 'HEAD') { if (!$this->headers->has('Content-Length')) { - $this->headers->set('Content-Length', strlen($this->content)); + $this->headers->set('Content-Length', (string)strlen($this->content)); } $this->content = ''; } if (!$this->headers->has('Content-Length')) { - $this->headers->set('Content-Length', strlen($this->content)); + $this->headers->set('Content-Length', (string)strlen($this->content)); } if ($this->headers->has('Transfer-Encoding')) { diff --git a/Neos.Flow/Tests/Unit/Http/ResponseTest.php b/Neos.Flow/Tests/Unit/Http/ResponseTest.php index 02ffa34fde..a99a738f11 100644 --- a/Neos.Flow/Tests/Unit/Http/ResponseTest.php +++ b/Neos.Flow/Tests/Unit/Http/ResponseTest.php @@ -357,9 +357,9 @@ public function getAgeReturnsTimeSpecifiedInAgeHeaderIfExists() $response = new Response(); $response->setNow($now); - $response->setHeader('Age', 123); + $response->setHeader('Age', '123'); - $this->assertSame(123, $response->getAge()); + $this->assertSame('123', $response->getAge()); } /** @@ -472,7 +472,7 @@ public function makeStandardsCompliantRemovesTheContentLengthHeaderIfTransferLen $response->setContent($content); $response->setHeader('Transfer-Encoding', 'chunked'); - $response->setHeader('Content-Length', strlen($content)); + $response->setHeader('Content-Length', (string)strlen($content)); $response->makeStandardsCompliant($request); $this->assertFalse($response->hasHeader('Content-Length')); } @@ -526,9 +526,9 @@ public function makeStandardsCompliantSetsBodyAndContentLengthForHeadRequests() $this->assertEquals(strlen($content), $response->getHeader('Content-Length')); $response = new Response(); - $response->setHeader('Content-Length', 275); + $response->setHeader('Content-Length', '275'); $response->makeStandardsCompliant($request); - $this->assertEquals(275, $response->getHeader('Content-Length')); + $this->assertEquals('275', $response->getHeader('Content-Length')); } /** @@ -560,7 +560,7 @@ public function makeStandardsCompliantRemovesMaxAgeDireciveIfExpiresHeaderIsPres public function makeStandardsCompliantReturns304ResponseIfResourceWasNotModified() { $modifiedSince = \DateTime::createFromFormat(DATE_RFC2822, 'Sun, 20 May 2012 12:00:00 GMT'); - $lastModified = \DateTime::createFromFormat(DATE_RFC2822, 'Fr, 18 May 2012 12:00:00 GMT'); + $lastModified = \DateTime::createFromFormat(DATE_RFC2822, 'Fri, 18 May 2012 12:00:00 GMT'); $request = Request::create(new Uri('http://localhost')); $response = new Response();