BUGFIX: Throw exception when passing objects to Headers::set()
#1140
Conversation
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.
|
See #1138 for the original approach to this |
|
NOTE: Right now |
|
With |
| public function getPreparedValuesRendersOneHeaderPerArrayItem() | ||
| { | ||
| $headers = new Headers(); | ||
| $headers->set('X-Array', ['some', 'array', 123]); |
There was a problem hiding this comment.
Well, the 123 violates the string[] declared in the docblock, and should be rejected as well?
There was a problem hiding this comment.
"Eagle Eye Karsten" ;)
You're right.. we need to check every array item for its type. mäh
There was a problem hiding this comment.
Could be done with an elseif ($values !== array_filter( $values, 'is_string' )) check after the other type checks.
Neos.Flow/Classes/Http/Headers.php
Outdated
| @@ -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; | |||
There was a problem hiding this comment.
I have to say, I prefer the $values = [$values]; version better, because it is more explicit. I really had to check if (array) $string; would actually end up in an array with the single string item, or for some php-ish reasons create an array of characters or something else bizarre.
| public function getPreparedValuesRendersOneHeaderPerArrayItem() | ||
| { | ||
| $headers = new Headers(); | ||
| $headers->set('X-Array', ['some', 'array', 123]); |
There was a problem hiding this comment.
Could be done with an elseif ($values !== array_filter( $values, 'is_string' )) check after the other type checks.
|
@albe @kdambekalns Thanks a lot for your feedback and sorry for the long delay.. I just rebased this and incorporated your suggestions |
|
And immediately I got bitten. By the |
| @@ -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'); | |||
There was a problem hiding this comment.
The change got bigger than I wanted it to be. But this one is a good example why it might be worth it:
$lastModified evalutated false because the given date was invalid. The test didn't actually test what it was supposed to test. It only failed because false is no longer allowed
|
I will revisit this for next release but we need to get there anyway. |
|
Probably related to #1552 so depending on how we implement PSR-7 (with or without our own Headers implementation), this might be relevant or obsolete. Linking here to not lose track. |
|
I guess this can be closed now.... |
According to PSR-7 (http://www.php-fig.org/psr/psr-7/) HTTP headers
can only accept values of
stringorstring[].This adjusts our
Headersimplementation to throw an exceptionif an object is passed to the
set()method in order to makeerror handling easier.
Exceptions prove the rule so we still do allow instances of
\DateTimeInterfaceto be set as header value.