WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#18525 new defect (bug)

zlib.output_compression "on" in server conflicts with autoupdate

Reported by: avidre Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.2.1
Component: Bootstrap/Load Keywords: has-patch dev-feedback
Focuses: Cc:

Description

If zlib.output_compression is "on" in server (my vps server), then auto-update works, but without verbose output or any indication that install has succeeded.

This error is consistent for all auto-updates WordPress Application and all plugins.

It is NOT a plugin conflict. Occurs on different servers.

Testing has confirmed that when zlib.output_compression is returned to "off", then updates work as expected.

In my opinion this is a minor bug and probably a note in the readme file will suffice.

Thank You,

Neil Miller
zx@…

Attachments (6)

18525.diff (2.6 KB) - added by kurtpayne 3 years ago.
18525.2.diff (2.6 KB) - added by kurtpayne 3 years ago.
Replaced ob_get_flush() with ob_end_flush().
18525.3.diff (2.8 KB) - added by kurtpayne 3 years ago.
Re-reverted patch 11020, changed padding to html comments
18525.4.diff (3.0 KB) - added by kurtpayne 3 years ago.
Using an HTML comment with null bytes for padding
18525.5.diff (2.9 KB) - added by kurtpayne 3 years ago.
Suppressing ob_end_flush notices, bailing during CLI mode
18525.6.diff (3.7 KB) - added by kurtpayne 3 years ago.
Compatibility with ob_gzhandler

Download all attachments as: .zip

Change History (28)

comment:1 SergeyBiryukov3 years ago

Possibly related: #18239

comment:2 dd323 years ago

If zlib.output_compression is "on" in server (my vps server), then auto-update works, but without verbose output or any indication that install has succeeded.

You should still see all the messages, and that it's completed.

zlib at the PHP, or server level, has the disadvantage that it creates an output buffer, and waits for the entire page to be generated before it can send it to the client, as a result, The page is never delivered to the browser until WordPress has finished the updates. You should however, see the page once it finishes generating.. Are you seeing something different?

This is something that could be documented on the codex, but there isn't really much more that can be said or done, different server configurations will affect different applications differently - Most users will not even know what zlib is..

comment:3 kurtpayne3 years ago

  • Cc kpayne@… added

Is there a reason we can't use ini_set() to turn the appropriate values off in wp_ob_end_flush_all() ?

Untested example:

ini_set('zlib.output_handler', '');
ini_set('zlib.output_compression', 0);
ini_set('output_handler', '');
ini_set('output_buffering', false);
ini_set('implicit_flush', true);

comment:4 kurtpayne3 years ago

Minor testing indicates that these two lines cause a "headers already sent" warning:

ini_set('zlib.output_handler', '');
ini_set('zlib.output_compression', 0);

After removing them, the modified example looks like:

ini_set('output_handler', '');
ini_set('output_buffering', false);
ini_set('implicit_flush', true);

kurtpayne3 years ago

comment:5 kurtpayne3 years ago

  • Cc kpayne@… removed
  • Keywords has-patch dev-feedback added

18525.diff will disable gzip (on apache2, IIS not tested) and php's zlib compression. It will also send a 4K block of null bytes in an attempt to defeat chunking. There doesn't seem to be any other way to force apache2 to stop chunking output without sending a content-length header.

There's also a modification to force theme/plugin updates to flush immediately after each step.

comment:6 follow-up: dd323 years ago

Any reason for ob_get_flush() over ob_end_flush()?

Despite what I thought, it appears that ini_set('zlib.output_handler', ''); does indeed do what the packet says, and it can be called at any point of the pageload too.

as for $disable_compression, we should be able to process that once per page load, if the function's called, we want to flush now, no need for it to be a variable.. If we use a static variable within the function, we can make sure that we only do that extra processing once per pageload too.

Do you have a document/reference which explains the 4k null bytes? Given that'll be output during the middle of the pageload, I'm not entirely sure what that achieves? or is it to just push it over the block boundary so a chunk is sent to the browser rather than waiting for the next?

comment:7 in reply to: ↑ 6 kurtpayne3 years ago

Replying to dd32:

Any reason for ob_get_flush() over ob_end_flush()?

Thanks, good catch. This was from an earlier version of the code.

Despite what I thought, it appears that ini_set('zlib.output_handler', ''); does indeed do what the packet says, and it can be called at any point of the pageload too.

As long as you're not currently buffering output. Otherwise, you can never clear the output buffer properly. This is from my own experience and from the first comment here.

as for $disable_compression, we should be able to process that once per page load, if the function's called, we want to flush now, no need for it to be a variable.. If we use a static variable within the function, we can make sure that we only do that extra processing once per pageload too.

This function is registered as a shutdown hook, though. We don't always want to use the $disable_compression flag, right? Just when it's absolutely necessary to give instant feedback?

Do you have a document/reference which explains the 4k null bytes? Given that'll be output during the middle of the pageload, I'm not entirely sure what that achieves? or is it to just push it over the block boundary so a chunk is sent to the browser rather than waiting for the next?

Glad you questioned this. I can use eyes on it. From my research, it looks like apache2 uses chunked encoding for any HTTP 1.1 response and you can't turn it off unless you specify a content-length header. Since the output isn't known before-hand, this isn't possible.

http://serverfault.com/questions/222148/how-to-disable-chunking-in-apache
http://bytes.com/topic/php/answers/10395-chunked-encoding-php-apache2

From my testing (apache2 / mod_php), it does seem like the output is being buffered, even pulling out all of the other tricks in this patch. Sending a 4K block of null bytes shouldn't have any affect on the output, but does make the responses feel snappier.

There could also be a buffer in the browser that needs to be filled before output is rendered. As noted on php.net:

Some versions of Microsoft Internet Explorer will only start to display the page after they have received 256 bytes of output, so you may need to send extra whitespace before flushing to get those browsers to display the page.

Generally, for WordPress pages, the 256 byte limit will be reached, but browser-buffering is a concept I wanted to keep in mind.

kurtpayne3 years ago

Replaced ob_get_flush() with ob_end_flush().

comment:8 dd323 years ago

This function is registered as a shutdown hook, though. We don't always want to use the $disable_compression flag, right? Just when it's absolutely necessary to give instant feedback?

I wasn't aware of that.. Seems rather useless in all honesty.. I'm sure there's a reason for it though.

From my testing (apache2 / mod_php), it does seem like the output is being buffered, even pulling out all of the other tricks in this patch. Sending a 4K block of null bytes shouldn't have any affect on the output, but does make the responses feel snappier.

I'll do some testing on a stand-alone script, see what I can come up with. Usually a few loops, well placed sleep()'s will give you a explanation as to what's happening where..

comment:9 follow-up: dd323 years ago

The patch reverts [11020] - that being said, without knowing the original Notice it's hard to tell.
Trunk will currently emmit a Notice: Notice: ob_end_flush() [ref.outcontrol]: failed to delete buffer zlib output compression when that function is called, your code/reverted code will produce it too, just hide it from display.

I've played with it a bit, and it certainly helps push the content out in all configurations where some form of buffering is in use.

Found a weird bug though, Opera can't handle the nullbytes.. or at least, not when it's part of the source, within HTML comment tags they're fine. I believe I've seen a similar bug for Safari, where a Null byte is on a chunk boundary, it can cause the rest of the page to be "lost" (just not rendered). (I'd link to an example, but I can't afford to have a 5+ second script running on my vps that search engines will hit :))

Outputting the Nullbytes within HTML comments appears to get around it:

foreach ( range(1, 5) as $i ) {
	echo $i . '<br />';
	echo '<!--' . str_repeat(chr(0), 4096) . '-->';
	sleep(2);
}

4k does appear to be the correct sizing to use for Apache's chunked handler as well, and should serve to be large enough for other web servers hopefully.

also note, PHPDoc can't be parsed when it's inline like that I don't believe, so best to puts @links in the function docs rather than inline.

kurtpayne3 years ago

Re-reverted patch 11020, changed padding to html comments

comment:10 in reply to: ↑ 9 kurtpayne3 years ago

Replying to dd32:

The patch reverts [11020]

Fixed. Thanks.

Trunk will currently emmit a Notice: Notice: ob_end_flush() [ref.outcontrol]: failed to delete buffer zlib output compression when that function is called, your code/reverted code will produce it too, just hide it from display.

I'm not able to reproduce this even using E_ALL. What setup are you using?

Found a weird bug though, Opera can't handle the nullbytes.. or at least, not when it's part of the source, within HTML comment tags they're fine. I believe I've seen a similar bug for Safari, where a Null byte is on a chunk boundary, it can cause the rest of the page to be "lost" (just not rendered).

Changed the patch to just emit <!-- -> a bunch of times instead. I've seen different approaches to this ... some use one big html comment, some prefer whitespace, some use small comments, etc. This patch uses the alphabet.

also note, PHPDoc can't be parsed when it's inline like that I don't believe, so best to puts @links in the function docs rather than inline.

Fixed. Does WP generate documentation anywhere? I thought these blocks made more sense to the reader inline, but I agree that phpdoc/docblox/etc. wouldn't pick up on them.

comment:11 kurtpayne3 years ago

Just for reference, I did come across a case of chromium doing some buffering up to 512 bytes before rendering. It looks like this is for mime type sniffing.

It seems like this is a known issue in the ajax handler, which sends the X-Content-Type-Options: nosniff header.

http://code.google.com/p/chromium/issues/detail?id=31410

comment:12 follow-up: dd323 years ago

I'm not able to reproduce this even using E_ALL. What setup are you using?

E_ALL (Well, that doesn't really matter, WordPress's WP_DEBUG overrides the PHP error reporting level) with zlib PHP compression enabled. ob_get_level() will return > 1, and ob_end_flush() can't turn off that style of output buffer. You may also need to be running PHP 5.3 as I've seen other reports about it too, php 5.3+zlib is the only combination that i'm aware of amongst them

Changed the patch to just emit <!-- -> a bunch of times instead.

Personally I prefered <!-- NULLNULL.. --> as that way the browsers I tested don't include a tonne of extra markup in the source view (Since the null byte character can't be displayed). As long as it's within a HTML tag/comment browsers seem to handle it properly.

Fixed. Does WP generate documentation anywhere?

http://xref.wordpress.org/

I did come across a case of chromium doing some buffering up to 512 bytes before rendering.

Might be worth checking that the headers of the plugin upgrader iframes have at least 512bytes of length before it outputs the steps then, but it's an old bug, so hopefully doesn't affect any current generation browsers.

kurtpayne3 years ago

Using an HTML comment with null bytes for padding

comment:13 in reply to: ↑ 12 kurtpayne3 years ago

Replying to dd32:

E_ALL (Well, that doesn't really matter, WordPress's WP_DEBUG overrides the PHP error reporting level)

Ha. Missed that, thanks.

with zlib PHP compression enabled. ob_get_level() will return > 1, and ob_end_flush() can't turn off that style of output buffer.

The buffer flush at the top of the function should solve this. That way the buffer is empty before zlib is turned off. According to the docs I've seen, that's the correct order of operations.

You may also need to be running PHP 5.3 as I've seen other reports about it too, php 5.3+zlib is the only combination that i'm aware of amongst them

Tried running with WP_DEBUG on on 5.3 with apache 2.2 on two different machines. I cannot reproduce the notice you're seeing. Are you running under a cgi/suphp/fastcgi setup?

Personally I prefered <!-- NULLNULL.. --> as that way the browsers I tested don't include a tonne of extra markup in the source view

Changed. Also added a link to this thread for anyone who reads the code and scratches their head.

Might be worth checking that the headers of the plugin upgrader iframes have at least 512bytes of length before it outputs the steps then

On my test site, one plugin update generated > 11K of output in the iframe. Looks like it's safe.

I've seen some wpshell / wp-cli projects in the wild ... should this patch include some code to wrap the 4K echo that checks to make sure the sapi isn't CLI? Or is that just too detailed?

if ('cli' != php_sapi_name() {
    echo '<!--' . str_repeat(chr(0), 4089) . '-->'; // 4096 bytes
}

comment:14 follow-up: dd323 years ago

Are you running under a cgi/suphp/fastcgi setup?

That could be it, I'm running a FastCGI setup here.

with zlib PHP compression enabled. ob_get_level() will return > 1, and ob_end_flush() can't turn off that style of output buffer.

The buffer flush at the top of the function should solve this. That way the buffer is empty before zlib is turned off. According to the docs I've seen, that's the correct order of operations.

The bottom of the login page, as well as ajax loads are the main locations I see it. Ultimately, @ob_end_clean(); might be the best solution unfortunately.

I've seen some wpshell / wp-cli projects in the wild ... should this patch include some code to wrap the 4K echo that checks to make sure the sapi isn't CLI?

If anything was included of that, I'd return from the function early (ie. before your additional disable cache IF block)

kurtpayne3 years ago

Suppressing ob_end_flush notices, bailing during CLI mode

comment:15 in reply to: ↑ 14 kurtpayne3 years ago

Replying to dd32:

The bottom of the login page, as well as ajax loads are the main locations I see it. Ultimately, @ob_end_clean(); might be the best solution unfortunately.

Thanks. The ajax pages made the notices show up for me. I was focused on the main pages and the installer/updater pages.

I used ob_list_handlers(), ob_get_status(), and ob_get_length() to look at this a bit further. It looks like there is data in the buffer (ob_get_length() returns > 0) and ob_end_flush() does end buffering (ob_get_level() doesn't result in an infinite loop) and put the data on the wire (you see it on the screen), but it doesn't completely flush the buffer because the data isn't successfully deleted from the buffer, hence the notice.

Ultimately, @ob_end_clean(); might be the best solution unfortunately.

I agree. I don't see a way to code around this. I couldn't find anything in php.net comments or on google either.

I'm submitting a refactored version of the patch after testing it by stacking mod_deflate with with ob_start("ob_gzhandler");. ob_gzhandler sends its own headers, so it's not something that can be coded around.

Reference: http://us.php.net/ob_gzhandler second-to-last comment

if you call ob_end_clean() after ob_start("ob_gzhandler"), the "Content-Encoding: gzip" header will still get sent (assuming the browser supports the encoding). if you don't call ob_start() again with the ob_gzhandler callback function, the output will not be compressed, but the header will say it is. this causes mozilla (as of build 2002032808) to display a blank page.

kurtpayne3 years ago

Compatibility with ob_gzhandler

comment:16 ryan20 months ago

With trunk r21645 the following notice is issued with PHP 5.4.4:

PHP Notice:  ob_end_flush(): failed to send buffer of zlib output compression (0) in /.../trunk/wp-includes/functions.php on line 2508

18525.6.diff avoids the notice.

comment:17 SergeyBiryukov18 months ago

#22430 was marked as a duplicate.

comment:18 SergeyBiryukov18 months ago

#22430 was marked as a duplicate.

comment:19 SergeyBiryukov18 months ago

#22430 was marked as a duplicate.

comment:20 nacin18 months ago

#22430 was marked as a duplicate.

comment:21 nacin9 months ago

We should investigate if calling wp_ob_end_flush_all() is necessary anymore. If I recall correctly, this was to partially work around object destruction ordering weirdness in PHP 4 versus PHP 5.

comment:22 nacin3 months ago

  • Component changed from General to Bootstrap/Load
Note: See TracTickets for help on using tickets.