Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Approach: Build on top of stream filters (zlib.deflate / zlib.inflate) #3

Closed
clue opened this issue Nov 3, 2015 · 5 comments
Closed

Comments

@clue
Copy link
Owner

clue commented Nov 3, 2015

Refs #1.

@clue
Copy link
Owner Author

clue commented Nov 4, 2015

PHP offers a number of stream compression filters:

  • zlib.deflate (compression)
  • zlib.inflate (decompression)

These filters can be used to implement streaming (de)compression.
However, PHP's stream design doesn't fit particularly well with React's stream design.

As such, I've looked into making this more accessible, with the plan of then building a (React) stream wrapper around this. I've got quite far here: clue/stream-filter#5

Unfortunately, I've also come across quite a few inconsistencies: clue/stream-filter#5 (comment)

Also, HHVM's zlib.deflate filter function buffers the whole string. This means that compressing a stream of 100 MB actually stores the whole string in memory before invoking the zlib compression algorithm.

Despite all this I still think this is probably the best approach there currently is.

I'll look into finalizing clue/stream-filter#5 and will then start building a (React) stream wrapper around this.

There's currently not much we can do about the inconsistencies mentioned above, so we should look into adding some documentation / known limitations for the time being.

@joelwurtz
Copy link

Actually working on some implementation for zlib / gzip stream, don't see the mention but there is a bug within php zlib.deflate filter depending on the stream being used (file:// or data:// is good but all others not), it's does not deflate :

Some example (using clue/php-stream-filter) :

<?php

require_once __DIR__ . "/vendor/autoload.php";

use Clue\StreamFilter as Filter;

$string = "Test chain";
$stream = fopen('php://memory', 'rw');
fwrite($stream, $string);
rewind($stream);

$deflateFilter = Filter\fun('zlib.deflate', ['level' => 6, 'window' => 15, 'memory' => 9]);

Filter\append($stream, $deflateFilter, STREAM_FILTER_READ);

$encoded = stream_get_contents($stream);

// Return false (but should return true)
// $encoded only have the zlib header
var_dump($encoded == gzdeflate($string));

EDIT : reference of the bug https://bugs.php.net/bug.php?id=48725

@clue
Copy link
Owner Author

clue commented Nov 12, 2015

Hi @joelwurtz, thanks for looking into this and posting this issue. I guess I should have made it more obvious I've already been working on this very issue for the past few days :-)

I've already pushed a first skeleton which implements just this a few minutes ago.

Setting window => 15 causes the zlib (library) to use the ZLIB format (RFC 1950) around the raw DEFLATE compression (RFC 1951). However, the gzdeflate() expects just the raw DEFLATE format (RFC 1951). This means we'd have two options:

  • Check against gzuncompress() which expects ZLIB format (RFC 1950) or
  • Set window => -15 to cause zlib (library) to use raw DEFLATE format (RFC 1951) (more details)

Marking this as resolved.

@joelwurtz
Copy link

And i suppose, by reading the code, having 'window' => 15|16 will force GZIP format (RFC 1952) ? (Withtout parameter it default to the DEFLATE format, RFC 1951)

That's nice thanks for the answer.

@clue
Copy link
Owner Author

clue commented Nov 12, 2015

@joelwurtz I see you've got this sorted out already: https://twitter.com/JoelWurtz/status/664900933889970176

I agree this may look strange at first, but it's actually very close to how the underlying zlib works: http://www.zlib.net/manual.html#Advanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants