From 8918853ce21be6fe4577eed79e6d88cd807adc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 19 Apr 2020 17:31:32 +0200 Subject: [PATCH 1/4] Require PHP 7 for new streaming compressor and decompressor --- .travis.yml | 3 --- README.md | 8 +++----- composer.json | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index cf1da5c..5361e87 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,9 +5,6 @@ dist: trusty matrix: include: - - php: 5.4 - - php: 5.5 - - php: 5.6 - php: 7.0 - php: 7.1 - php: 7.2 diff --git a/README.md b/README.md index 5e6f927..d5d0e02 100644 --- a/README.md +++ b/README.md @@ -188,9 +188,7 @@ $ composer require clue/zlib-react:^0.2.2 See also the [CHANGELOG](CHANGELOG.md) for details about version upgrades. This project aims to run on any platform and thus does not require any PHP -extensions besides `ext-zlib` and supports running on legacy PHP 5.4 through current -PHP 7+. -It's *highly recommended to use PHP 7+* for this project. +extensions besides `ext-zlib` and supports running on current PHP 7+. The `ext-zlib` extension is required for handling the underlying data compression and decompression. @@ -200,8 +198,8 @@ builds by default. If you're building PHP from source, you may have to [manually enable](https://www.php.net/manual/en/zlib.installation.php) it. We're committed to providing a smooth upgrade path for legacy setups. -If you need to support legacy PHP 5.3 and legacy HHVM, you may want to check out -the legacy `v0.2.x` release branch. +If you need to support legacy PHP versions and legacy HHVM, you may want to +check out the legacy `v0.2.x` release branch. This legacy release branch also provides an installation candidate that does not require `ext-zlib` during installation but uses runtime checks instead. diff --git a/composer.json b/composer.json index b956cd0..9f72267 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ } ], "require": { - "php": ">=5.4", + "php": ">=7.0", "ext-zlib": "*", "clue/stream-filter": "~1.3", "react/stream": "^1.0 || ^0.7 || ^0.6" From fc2816f7663d5df1df7da0b62eaa09f799d64752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 24 Apr 2020 10:52:23 +0200 Subject: [PATCH 2/4] Change Compressor to use more efficient deflate_init() context --- src/Compressor.php | 35 ++++++++++++++++++++++++++++++----- src/ZlibFilterStream.php | 15 --------------- tests/CompressorTest.php | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 tests/CompressorTest.php diff --git a/src/Compressor.php b/src/Compressor.php index 5cf7135..b9f5bc9 100644 --- a/src/Compressor.php +++ b/src/Compressor.php @@ -31,18 +31,43 @@ * For more details, see ReactPHP's * [`DuplexStreamInterface`](https://github.com/reactphp/stream#duplexstreaminterface). */ -final class Compressor extends ZlibFilterStream +final class Compressor extends TransformStream { + /** @var ?resource */ + private $context; + /** * @param int $encoding ZLIB_ENCODING_GZIP, ZLIB_ENCODING_RAW or ZLIB_ENCODING_DEFLATE * @param int $level optional compression level */ public function __construct($encoding, $level = -1) { - parent::__construct( - Filter\fun('zlib.deflate', array('window' => $encoding, 'level' => $level)) - ); + $context = @deflate_init($encoding, ['level' => $level]); + if ($context === false) { + throw new \InvalidArgumentException('Unable to initialize compressor' . strstr(error_get_last()['message'], ':')); + } + + $this->context = $context; + } + + protected function transformData($chunk) + { + $ret = deflate_add($this->context, $chunk, ZLIB_NO_FLUSH); + + if ($ret !== '') { + $this->forwardData($ret); + } + } + + protected function transformEnd($chunk) + { + $ret = deflate_add($this->context, $chunk, ZLIB_FINISH); + $this->context = null; + + if ($ret !== '') { + $this->forwardData($ret); + } - $this->emptyWrite = $encoding; + $this->forwardEnd(); } } diff --git a/src/ZlibFilterStream.php b/src/ZlibFilterStream.php index d53159e..86429a8 100644 --- a/src/ZlibFilterStream.php +++ b/src/ZlibFilterStream.php @@ -20,13 +20,6 @@ class ZlibFilterStream extends TransformStream { private $filter; - /** - * @var int|null - * @see Compressor - * @internal - */ - protected $emptyWrite; - public function __construct($filter) { $this->filter = $filter; @@ -38,7 +31,6 @@ protected function transformData($chunk) $ret = $filter($chunk); if ($ret !== '') { - $this->emptyWrite = null; $this->forwardData($ret); } } @@ -48,13 +40,6 @@ protected function transformEnd($chunk) $filter = $this->filter; $ret = $filter($chunk) . $filter(); - // Stream ends successfully and did not emit any data whatsoever? - // This happens when compressing an empty stream with PHP 7 only. - // Bypass filter and manually compress/encode empty string. - if ($this->emptyWrite !== null && $ret === '') { - $ret = \zlib_encode('', $this->emptyWrite); - } - if ($ret !== '') { $this->forwardData($ret); } diff --git a/tests/CompressorTest.php b/tests/CompressorTest.php new file mode 100644 index 0000000..1d93db2 --- /dev/null +++ b/tests/CompressorTest.php @@ -0,0 +1,14 @@ + Date: Fri, 24 Apr 2020 11:40:22 +0200 Subject: [PATCH 3/4] Change Decompressor to use more efficient inflate_init() context Among others, this also supports error reporting when decompressing an invalid data stream and fixes all known inconsistencies. --- README.md | 12 --------- src/Decompressor.php | 42 ++++++++++++++++++++++++++++--- tests/DecompressorTest.php | 14 +++++++++++ tests/DeflateDecompressorTest.php | 10 ++++++-- tests/GzipDecompressorTest.php | 10 ++++++-- tests/ZlibDecompressorTest.php | 10 ++++++-- 6 files changed, 76 insertions(+), 22 deletions(-) create mode 100644 tests/DecompressorTest.php diff --git a/README.md b/README.md index d5d0e02..4b74536 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,6 @@ supporting compression and decompression of GZIP, ZLIB and raw DEFLATE formats. * [Usage](#usage) * [Compressor](#compressor) * [Decompressor](#decompressor) - * [Inconsistencies](#inconsistencies) * [Install](#install) * [Tests](#tests) * [License](#license) @@ -162,17 +161,6 @@ $input->pipe($decompressor)->pipe($filterBadWords)->pipe($output); For more details, see ReactPHP's [`DuplexStreamInterface`](https://github.com/reactphp/stream#duplexstreaminterface). -### Inconsistencies - -The stream compression filters are not exactly the most commonly used features of PHP. -As such, we've spotted some inconsistencies (or *bugs*) in different PHP versions. -These inconsistencies exist in the underlying PHP engines and there's little we can do about this in this library. - -* All PHP versions: Decompressing invalid data does not emit any data (and does not raise an error) - -Our test suite contains several test cases that exhibit these issues. -If you feel some test case is missing or outdated, we're happy to accept PRs! :) - ## Install The recommended way to install this library is [through Composer](https://getcomposer.org). diff --git a/src/Decompressor.php b/src/Decompressor.php index cd05891..2a8cbb1 100644 --- a/src/Decompressor.php +++ b/src/Decompressor.php @@ -31,15 +31,49 @@ * For more details, see ReactPHP's * [`DuplexStreamInterface`](https://github.com/reactphp/stream#duplexstreaminterface). */ -final class Decompressor extends ZlibFilterStream +final class Decompressor extends TransformStream { + /** @var ?resource */ + private $context; + /** * @param int $encoding ZLIB_ENCODING_GZIP, ZLIB_ENCODING_RAW or ZLIB_ENCODING_DEFLATE */ public function __construct($encoding) { - parent::__construct( - Filter\fun('zlib.inflate', array('window' => $encoding)) - ); + $context = @inflate_init($encoding); + if ($context === false) { + throw new \InvalidArgumentException('Unable to initialize decompressor' . strstr(error_get_last()['message'], ':')); + } + + $this->context = $context; + } + + protected function transformData($chunk) + { + $ret = @inflate_add($this->context, $chunk); + if ($ret === false) { + throw new \RuntimeException('Unable to decompress' . strstr(error_get_last()['message'], ':')); + } + + if ($ret !== '') { + $this->forwardData($ret); + } + } + + protected function transformEnd($chunk) + { + $ret = @inflate_add($this->context, $chunk, ZLIB_FINISH); + $this->context = null; + + if ($ret === false) { + throw new \RuntimeException('Unable to decompress' . strstr(error_get_last()['message'], ':')); + } + + if ($ret !== '') { + $this->forwardData($ret); + } + + $this->forwardEnd(); } } diff --git a/tests/DecompressorTest.php b/tests/DecompressorTest.php new file mode 100644 index 0000000..a3aa264 --- /dev/null +++ b/tests/DecompressorTest.php @@ -0,0 +1,14 @@ +assertEquals($data, $buffered); } - public function testInflateInvalid() + public function testDecompressInvalidDataEmitsError() { - $this->markTestSkipped('Not supported by any PHP version (neither does reject invalid data)'); + $this->decompressor->on('data', $this->expectCallableNever()); + $this->decompressor->on('error', $this->expectCallableOnce()); + $this->decompressor->write('invalid'); + } + + public function testDecompressInvalidOnEndEmitsError() + { $this->decompressor->on('data', $this->expectCallableNever()); $this->decompressor->on('error', $this->expectCallableOnce()); diff --git a/tests/GzipDecompressorTest.php b/tests/GzipDecompressorTest.php index 40277d1..1ebaffd 100644 --- a/tests/GzipDecompressorTest.php +++ b/tests/GzipDecompressorTest.php @@ -48,10 +48,16 @@ public function testDecompressBig() $this->assertEquals($data, $buffered); } - public function testDecompressInvalid() + public function testDecompressInvalidDataEmitsError() { - $this->markTestSkipped('Not supported by any PHP version (neither does reject invalid data)'); + $this->decompressor->on('data', $this->expectCallableNever()); + $this->decompressor->on('error', $this->expectCallableOnce()); + $this->decompressor->write('invalid'); + } + + public function testDecompressInvalidOnEndEmitsError() + { $this->decompressor->on('data', $this->expectCallableNever()); $this->decompressor->on('error', $this->expectCallableOnce()); diff --git a/tests/ZlibDecompressorTest.php b/tests/ZlibDecompressorTest.php index 370004f..7d2c9dc 100644 --- a/tests/ZlibDecompressorTest.php +++ b/tests/ZlibDecompressorTest.php @@ -48,10 +48,16 @@ public function testDecompressBig() $this->assertEquals($data, $buffered); } - public function testDecompressInvalid() + public function testDecompressInvalidDataEmitsError() { - $this->markTestSkipped('Not supported by any PHP version (neither does reject invalid data)'); + $this->decompressor->on('data', $this->expectCallableNever()); + $this->decompressor->on('error', $this->expectCallableOnce()); + $this->decompressor->write('invalid'); + } + + public function testDecompressInvalidOnEndEmitsError() + { $this->decompressor->on('data', $this->expectCallableNever()); $this->decompressor->on('error', $this->expectCallableOnce()); From ef81edf8b91ffe2ce17f5b11ac7a97f265c2099e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 24 Apr 2020 11:57:06 +0200 Subject: [PATCH 4/4] Refactor to remove outdated code and achieve 100% test coverage --- composer.json | 1 - src/Compressor.php | 7 +-- src/Decompressor.php | 7 +-- src/TransformStream.php | 96 ++++++++-------------------------------- src/ZlibFilterStream.php | 50 --------------------- 5 files changed, 27 insertions(+), 134 deletions(-) delete mode 100644 src/ZlibFilterStream.php diff --git a/composer.json b/composer.json index 9f72267..c54b3ec 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,6 @@ "require": { "php": ">=7.0", "ext-zlib": "*", - "clue/stream-filter": "~1.3", "react/stream": "^1.0 || ^0.7 || ^0.6" }, "require-dev": { diff --git a/src/Compressor.php b/src/Compressor.php index b9f5bc9..a494096 100644 --- a/src/Compressor.php +++ b/src/Compressor.php @@ -55,7 +55,7 @@ protected function transformData($chunk) $ret = deflate_add($this->context, $chunk, ZLIB_NO_FLUSH); if ($ret !== '') { - $this->forwardData($ret); + $this->emit('data', [$ret]); } } @@ -65,9 +65,10 @@ protected function transformEnd($chunk) $this->context = null; if ($ret !== '') { - $this->forwardData($ret); + $this->emit('data', [$ret]); } - $this->forwardEnd(); + $this->emit('end'); + $this->close(); } } diff --git a/src/Decompressor.php b/src/Decompressor.php index 2a8cbb1..d2ea6ba 100644 --- a/src/Decompressor.php +++ b/src/Decompressor.php @@ -57,7 +57,7 @@ protected function transformData($chunk) } if ($ret !== '') { - $this->forwardData($ret); + $this->emit('data', [$ret]); } } @@ -71,9 +71,10 @@ protected function transformEnd($chunk) } if ($ret !== '') { - $this->forwardData($ret); + $this->emit('data', [$ret]); } - $this->forwardEnd(); + $this->emit('end'); + $this->close(); } } diff --git a/src/TransformStream.php b/src/TransformStream.php index 2241a1f..92145aa 100644 --- a/src/TransformStream.php +++ b/src/TransformStream.php @@ -35,7 +35,8 @@ public function write($data) return true; } catch (Exception $e) { - $this->forwardError($e); + $this->emit('error', [$e]); + $this->close(); return false; } } @@ -53,7 +54,8 @@ public function end($data = null) } $this->transformEnd($data); } catch (Exception $e) { - $this->forwardError($e); + $this->emit('error', [$e]); + $this->close(); } } @@ -107,109 +109,49 @@ public function pipe(WritableStreamInterface $dest, array $options = array()) return $dest; } - /** - * Forwards a single "data" event to the reading side of the stream - * - * This will emit an "data" event. - * - * If the stream is not readable, then this is a NO-OP. - * - * @param string $data - */ - protected function forwardData($data) - { - if (!$this->readable) { - return; - } - $this->emit('data', array($data)); - } - - /** - * Forwards an "end" event to the reading side of the stream - * - * This will emit an "end" event and will then close this stream. - * - * If the stream is not readable, then this is a NO-OP. - * - * @uses self::close() - */ - protected function forwardEnd() - { - if (!$this->readable) { - return; - } - $this->readable = false; - $this->writable = false; - - $this->emit('end'); - $this->close(); - } - - /** - * Forwards the given $error message to the reading side of the stream - * - * This will emit an "error" event and will then close this stream. - * - * If the stream is not readable, then this is a NO-OP. - * - * @param Exception $error - * @uses self::close() - */ - protected function forwardError(Exception $error) - { - if (!$this->readable) { - return; - } - $this->readable = false; - $this->writable = false; - - $this->emit('error', array($error)); - $this->close(); - } - /** * can be overwritten in order to implement custom transformation behavior * - * This gets passed a single chunk of $data and should invoke `forwardData()` + * This gets passed a single chunk of $data and should emit a `data` event * with the filtered result. * - * If the given data chunk is not valid, then you should invoke `forwardError()` - * or throw an Exception. + * If the given data chunk is not valid, then you should throw an Exception + * which will automatically be turned into an `error` event. * - * If you do not overwrite this method, then its default implementation simply - * invokes `forwardData()` on the unmodified input data chunk. + * If you do not overwrite this method, then its default implementation + * simply emits a `data` event with the unmodified input data chunk. * * @param string $data - * @see self::forwardData() */ protected function transformData($data) { - $this->forwardData($data); + $this->emit('data', [$data]); } /** * can be overwritten in order to implement custom stream ending behavior * - * This may get passed a single final chunk of $data and should invoke `forwardEnd()`. + * This may get passed a single final chunk of $data and should emit an + * `end` event and close the stream. * - * If the given data chunk is not valid, then you should invoke `forwardError()` - * or throw an Exception. + * If the given data chunk is not valid, then you should throw an Exception + * which will automatically be turned into an `error` event. * * If you do not overwrite this method, then its default implementation simply * invokes `transformData()` on the unmodified input data chunk (if any), - * which in turn defaults to invoking `forwardData()` and then finally - * invokes `forwardEnd()`. + * which in turn defaults to emitting a `data` event and then finally + * emits an `end` event and closes the stream. * * @param string $data * @see self::transformData() - * @see self::forwardData() - * @see self::forwardEnd() */ protected function transformEnd($data) { if ($data !== '') { $this->transformData($data); } - $this->forwardEnd(); + + $this->emit('end'); + $this->close(); } } diff --git a/src/ZlibFilterStream.php b/src/ZlibFilterStream.php deleted file mode 100644 index 86429a8..0000000 --- a/src/ZlibFilterStream.php +++ /dev/null @@ -1,50 +0,0 @@ -filter = $filter; - } - - protected function transformData($chunk) - { - $filter = $this->filter; - $ret = $filter($chunk); - - if ($ret !== '') { - $this->forwardData($ret); - } - } - - protected function transformEnd($chunk) - { - $filter = $this->filter; - $ret = $filter($chunk) . $filter(); - - if ($ret !== '') { - $this->forwardData($ret); - } - - $this->forwardEnd(); - $this->filter = null; - } -}