kjdev / php-ext-brotli

Brotli Extension for PHP
MIT License
168 stars 29 forks source link

Output handler is killed by flush() #25

Closed chrisgraham closed 3 years ago

chrisgraham commented 4 years ago

If you do a flush(), PHP will send headers, and the extension will fail to be able to set the Content-Encoding header. You'll get a binary mess in the browser.

At minimum I'd: 1) Document not to do flush(); 2) If you can't do headers, bomb out of the output handler completely without doing Brotli

But actually this problem doesn't exist for the zlib output handler. I have no idea how they do it, as I can see PHP plumbs flush(); directly into the SAPI. But I have just double checked that I can use flush with zlib and still get a zlib-encoded web response. PHP also documents that flush is a null-op on zlib output.

kjdev commented 3 years ago

Can you provide a sample code?

chrisgraham commented 3 years ago
<?php

ini_set('zlib.output_compression', 'Off');
ini_set('brotli.output_compression', 'On');

echo 'test';

flush();
kjdev commented 3 years ago

Thanks.

I will check and correct it.

kjdev commented 3 years ago

To make it equal to zlib.output_compression in cae70ba Fixed.


For the case of mode_php (httpd/prefork) and fpm (with nginx), you can use It seemed to behave differently.

gzip behavior

Accessed by Accept-Encoding:gzip.

ini_set('zlib.output_compression', 'On');

echo 'test';

// For the process so far
// Content-Encoding is gzip for both mode_php and fpm

flush();
// If you do a flush(), you can use the
// mode_php has no Content-Encoding (uncompressed output)
// fpm has a Content-Encoding of gzip

brotli behavior

Fixed in cae70ba.

Accessed by Accept-Encoding:br.

ini_set('brotli.output_compression', 'On');

echo 'test';

// For the process so far
// Content-Encoding is br for both mode_php and fpm

flush();
// If you do a flush(), you can use the
// mode_php has no Content-Encoding (uncompressed output)
// fpm has a Content-Encoding of br
chrisgraham commented 3 years ago

Thanks. That looks like a good fix.

Looking back at this with fresh eyes, I don't think I explained the issue very well. So for anyone else reading this, the issue is that a compression output handler has to compress everything in one go (compression is not iterative at the byte level). Doing a 'flush' will break that basic assumption.

Except for CGI I guess, because CGI is not iterative, everything will be sent to the SAPI in a single go regardless due to the nature of the CGI/web-server interface.

Edit: Fix confirmed working.