Make WordPress Core

Opened 13 years ago

Last modified 6 weeks ago

#18525 reopened defect (bug)

zlib.output_compression "on" in server conflicts with autoupdate

Reported by: avidre's profile 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 12 years ago.
18525.2.diff (2.6 KB) - added by kurtpayne 12 years ago.
Replaced ob_get_flush() with ob_end_flush().
18525.3.diff (2.8 KB) - added by kurtpayne 12 years ago.
Re-reverted patch 11020, changed padding to html comments
18525.4.diff (3.0 KB) - added by kurtpayne 12 years ago.
Using an HTML comment with null bytes for padding
18525.5.diff (2.9 KB) - added by kurtpayne 12 years ago.
Suppressing ob_end_flush notices, bailing during CLI mode
18525.6.diff (3.7 KB) - added by kurtpayne 12 years ago.
Compatibility with ob_gzhandler

Download all attachments as: .zip

Change History (40)

#1 @SergeyBiryukov
13 years ago

Possibly related: #18239

#2 @dd32
13 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..

#3 @kurtpayne
13 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);

#4 @kurtpayne
13 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);

@kurtpayne
12 years ago

#5 @kurtpayne
12 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.

#6 follow-up: @dd32
12 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?

#7 in reply to: ↑ 6 @kurtpayne
12 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.

@kurtpayne
12 years ago

Replaced ob_get_flush() with ob_end_flush().

#8 @dd32
12 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..

#9 follow-up: @dd32
12 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.

@kurtpayne
12 years ago

Re-reverted patch 11020, changed padding to html comments

#10 in reply to: ↑ 9 @kurtpayne
12 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.

#11 @kurtpayne
12 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

#12 follow-up: @dd32
12 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.

@kurtpayne
12 years ago

Using an HTML comment with null bytes for padding

#13 in reply to: ↑ 12 @kurtpayne
12 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
}

#14 follow-up: @dd32
12 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)

@kurtpayne
12 years ago

Suppressing ob_end_flush notices, bailing during CLI mode

#15 in reply to: ↑ 14 @kurtpayne
12 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.

@kurtpayne
12 years ago

Compatibility with ob_gzhandler

#16 follow-up: @ryan
12 years 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.

#17 @SergeyBiryukov
11 years ago

#22430 was marked as a duplicate.

#18 @SergeyBiryukov
11 years ago

#22430 was marked as a duplicate.

#19 @SergeyBiryukov
11 years ago

#22430 was marked as a duplicate.

#20 @nacin
11 years ago

#22430 was marked as a duplicate.

#21 @nacin
11 years 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.

#22 @nacin
10 years ago

  • Component changed from General to Bootstrap/Load

#23 in reply to: ↑ 16 @harmr
10 years ago

Replying to ryan:

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.

is there a chance that this patch makes it into 4.1?

#24 @selnomeria
9 years ago

SOLUTIONS I have came across so far:

======================== SOLUTION 1 ====================

In plugins (or somewhere) you probably have this code:

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

so, I replaced that code with

if (!is_admin()) ob_start('ob_gzhandler');     //because, in admin pages, it causes plugin installation freezing

and Compression will be remained still ON.

======================== SOLUTION 2 ====================
You may have to use:

remove_action( 'shutdown', 'wp_ob_end_flush_all', 1 );

but read notes in such case: https://core.trac.wordpress.org/ticket/22430#comment:4

Last edited 9 years ago by selnomeria (previous) (diff)

#25 @simonbcn
8 years ago

I have the same problem with PHP 7.0

This ticket was mentioned in Slack in #themereview by poena. View the logs.


7 years ago

#28 @skorasaurus
5 years ago

  • Status changed from new to reopened

I have reopened this ticket.
Upon updating to 5.1, I have a message at the bottom of all pages of WP installations stating:

"Notice: ob_end_flush(): failed to send buffer of zlib output compression (0) in /path/to/my/install/wp-includes/functions.php on line 4212"

Although this error is different than the original ticket, my error was described in Ticket 22430 and Ticket 22340 was designated as a duplicate of this ticket.

In addition to having 5.1 installed, this notice only appears if:
'zlib.output_compression', 'ON' in the server's php configuration (I've tested this across php 7.1 apache and php 7.2 and ngnix)
'WP_DEBUG' must be set to true in your wp-config.php.

I've been able to specifically isolate that all 3 characteristics must occur for the message to appear.

I can confirm that nacin's one-line plugin suggestion does remove this notice.

Last edited 5 years ago by skorasaurus (previous) (diff)

#29 @SergeyBiryukov
5 years ago

  • Milestone set to Awaiting Review

#30 @gilmore_love
5 years ago

This seems to be an ongoing issue for over 8 years which surfaced again after I upgraded to 5.1 and not sure why the development team is not doing anything about it and this time is not only causing a problem on the auto update but almost on every page and I don't think turning of zlib compression is the solution. it's more like erasing the problem than finding a solution. If it's deprecated then it should be removed in the next update and if it should be fixed then fix it, please. Some of us put lots of effort and money on a website that once in a while these simple issues will be a tool for hackers to hijack our websites. WP is a great tool but I believe they should do a better job in fixing bugs and increasing security. this might not be a security issue but there are so many of these issues that can one day soon or late cause a security issue. now days almost every week we hear about a new security bug surfacing and scares the hell out of all of us.

#31 @tradesouthwest@…
4 years ago

Would like to report the same errors in 5.2.4 with PHP 7.2.21 and 7.3.9, Apache 2.4.41, 5.5.62-cll, 2.6.32-954.3.5.lve1.4.61.el6.x86_64.

Notice: ob_end_flush(): failed to send buffer of zlib output compression (0) in /home/fencedir/plus.unitizr.com/wp-includes/functions.php on line 4344"

Error occurs with on all public side and Admin pages with _every_ plugin disabled and with any plugin enabled; no mu-plugins used. And _only_ with WP_DEBUG true<sup>1</sup>

remove_action( 'shutdown', 'wp_ob_end_flush_all', 1 ); Has no effect

<?php
function wp_ob_end_flush_all_fixed() {
        $start = (int) ini_get('zlib.output_compression');
        $levels = ob_get_level();
        for ( $i = $start; $i < $levels; $i++ ) {
                ob_end_flush();
        }
}
remove_action('shutdown', 'wp_ob_end_flush_all', 1);
add_action('shutdown', 'wp_ob_end_flush_all_fixed', 1);

Does remove error!
_(see)_ https://core.trac.wordpress.org/ticket/22430#comment:13 (whitespace & comments removed)

Q.:
Any reason to think that a core update to check for the state of php.ini and maybe throw a dismissable warning, as apposed to an error that is derived from core file (functions.php)?

<sub>1</sub> I tend to agree with @gilmore_love in that there are plenty of reasons to fix this; mostly to help developers and to keep core WP clean. Granted not all server configs and not all PHP versions (as well as NewRelic and a ton of other server 'tools') can be supported but to reiterate; security is priority and spending time on this before spending time on changing WP components is most salient for the WP community.

This ticket was mentioned in Slack in #core-auto-updates by helen. View the logs.


4 years ago

#33 @pbiron
4 years ago

#18239 was marked as a duplicate.

#34 @swissspidy
6 weeks ago

#22430 was marked as a duplicate.

#35 @swissspidy
6 weeks ago

#55833 was marked as a duplicate.

Note: See TracTickets for help on using tickets.