Run tests on PHP 7.3 and 7.4 on Travis #356
Conversation
src/Io/ChunkedDecoder.php
Outdated
| @@ -122,12 +122,12 @@ public function handleData($data) | |||
| } | |||
| } | |||
|
|
|||
| $this->chunkSize = \hexdec($hexValue); | |||
| if (\dechex($this->chunkSize) !== $hexValue) { | |||
| if (\ctype_xdigit($hexValue) === false || \dechex(\hexdec($hexValue)) !== $hexValue) { | |||
There was a problem hiding this comment.
Changes LGTM 👍
Note that this requires ext-ctype which could potentially be disabled explicitly. As an alternative we could also keep the previous logic and suppress errors for PHP 7.4 support. What do you think about this?
There was a problem hiding this comment.
Was more thinking along the lines of using: https://github.com/symfony/polyfill-ctype
There was a problem hiding this comment.
Alternatively I can do a PHP version check and only run it on PHP 7.4 and above and do some extra trickery so we can seamlessly do this
There was a problem hiding this comment.
It's funny that there's no changelog about the deprecation notice on the function page.
There was a problem hiding this comment.
There was a problem hiding this comment.
lol doh I assumed I missed it 😂
src/Io/ChunkedDecoder.php
Outdated
| !( | ||
| \PHP_VERSION_ID >= 70400 && | ||
| ( | ||
| (\function_exists('ctype_xdigit') && \ctype_xdigit($hexValue) === false) || | ||
| // From: https://github.com/symfony/polyfill-ctype/blob/f8f0b461be3385e56d6de3dbb5a0df24c0c275e3/Ctype.php#L196 | ||
| (\is_string($hexValue) && '' !== $hexValue && !preg_match('/[^A-Fa-f0-9]/', $hexValue)) | ||
| ) | ||
| ) || | ||
| \dechex(\hexdec($hexValue)) !== $hexValue |
There was a problem hiding this comment.
That's a lot of code 👀
What do you think about this https://3v4l.org/pSZCT vs https://3v4l.org/FJAD6? PHP's documentation suggests it should ignore all invalid characters, which matches the output, so we may as well ignore the deprecation warning (which would only affect PHP 7.4+)?
There was a problem hiding this comment.
@clue here is the issue: https://3v4l.org/Hiqht
There was a problem hiding this comment.
There was a problem hiding this comment.
@clue could still do @\dechex(@\hexdec($hexValue)) !== $hexValue to validate it tho
There was a problem hiding this comment.
@clue just pushed a simpler version, which falls back to the @\dechex(@\hexdec($hexValue)) !== $hexValue one fails (because the maxlength one was failing while the rest passed)
Most of these changes have been based on reactphp/stream#147 Additionally some changes have been made to ensure chunked encoding works on PHP 7.4.
No description provided.