cache_set and cache_get base_url brokenosity

First of all, thanks for the detailed report. :)

Second, request_uri() being broken when Drupal runs in a subdir is a ling standing bug which nobody seems to care about. I do not think that it is related to the issue at hand. http://drupal.org/node/10917

The issue is that apparently unzipped or scrambled data gets inserted into the cache table if a client which does not support gzip accesses the page in question before one that does.

This needs to be investigated. The code is in bootrap.inc. I will try to have a look, but am a bot short on time.

@killes: we probably need to only write to page cache for gzip browsers. but that means that many crawlers will never get cached. of course what we do today is even worse - we show the crawler garbage which can't be good for pagerank.

This needs to be investigated more closely. Can anyone reproduce this?

Ok, I've thought a bit about it and come to the conclusion that I cannot reproduce the problem.

I have created a patch that will create some debug info. Can somebody run it and report back?

What should happen is that if you have the problem, you will have "data compressed" info in your logs. The patch would also fix the problem if it exists for some combination of apache and php versions and settings.

I can reproduce.

Visit dev.newsphoto.nl/ with IE6, visit a page twice (first to make a cahce entry) then youll see that the paged served from cache comes out blannk.

www.newsphoto.nl/ has the same codebase, but only cache switched off.

Bèr

the previous post contains a log from my site. From about line 35, is where the problems occur. Those are the headers sent. As per killes request.

@Ber - those can't be all the request headers. no 'accept encoding' or http 1.0/1.1 are specified.

Can anyone please take a look at this also?

http://drupal.org/node/46272

the attached patch is meant to be used todebug the problem, not for includion in core.

I've come across a site that's getting these errors randomly. Will see if we can apply killes's debugging patch.

any news about this?

The news is that it's still a problem, due to Drupal being a bit crap and issues never being resolved.

Hm. I think you meant "Due to no one trying the debugging patch that Gerhard posted at http://drupal.org/node/43462#comment-367659 and reporting back with their full results on a site that was having this problem." :)

We need people who can reproduce the problem to give us enough information to fix it. Until then, it won't be.

Putting this to critical so it get's the attention it needs...

Also critical since binary returns, and WSOD's are not good!

(needs to be fixed before 7 ships...)

any news on this?

Steps to reproduce:

1. Add zlib.output_compression On to .htaccess.
2. Enable normal caching or clear cache on http://example.org/admin/settings/performance.
3. Issue the following request (e.g. using a telnet client: telnet example.org 80):
   

   GET / HTTP/1.0
   Host: example.org
   User-Agent: foo
   Accept-Encoding: gzip

4. Notice that the output is binary.
5. Notice that the cache_page table now contains uncompressed HTML — this is not expected behaviour.
6. Issue the following request:
   

   GET / HTTP/1.0
   User-Agent: foo
   Host: example.org

7. Notice error in output:
   Warning: gzinflate(): data error in /home/chsc/www/drupal7/includes/bootstrap.inc on line 735

page_set_cache() in common.inc assumes that ob_get_contents() returns compressed data if zlib.output_compression is enabled. This is not the case at my box, and IMO it would be surprising if it did considering the layered nature of PHP's output buffers. But this comment on the issue where this code was added seems to indicate otherwise. Can anybody explain that?

The attached patch fixes the problem on my box.

Could reproduce this, patch fixes the problem. Patch applies with a bit of fuzz though.

Changing status

It is really hard to test this patch. It would be best if we could write some simpletests for this. Anyone wants to try and take this on?

I'll write some tests when #330582: Retrieve HTTP response headers land.

subscribing

Reroll. Has been tested with and without zlib.output_compression and the mod_deflate Apache module.

Are you sure this is working well ?

I don't understand why you are wrapping bootstrap.inc test to determine if the browser
accepts gzipped data with the following "if":

if (variable_get('page_compression', TRUE) && extension_loaded('zlib')) {

What would happen if I disable zlib.output_compression, enable mod_deflate and disable
'page_compression' variable ? AFAIK, this would still save compressed data in the cache_page table, because ob_get_contents() returns compressed data (thanks to mod_deflate). Then inside bootstrap.inc, as I dont have 'page_compression' enabled, cache->data could not been decompressed if the browser request no compression, ¿am I right?

What would happen if I disable zlib.output_compression, enable mod_deflate and disable 'page_compression' variable ? AFAIK, this would still save compressed data in the cache_page table, because ob_get_contents() returns compressed data (thanks to mod_deflate).

I cannot reproduce that with PHP 5.2.0 and Apache 2.2.3. I'd be surprised if PHP's output is sent to mod_deflate and then back into PHP's output buffer. Do you really get the output from mod_deflate with ob_get_contents()?

Sorry I was wrong. I was thinking that my compressed data was coming from the last comment in the following snippet of code:

   if ($data = ob_get_contents()) {
      $cache = TRUE;
      if (function_exists('gzencode')) {
        // We do not store the data in case the zlib mode is deflate.
        // This should be rarely happening.
        if (zlib_get_coding_type() == 'deflate') {
          $cache = FALSE;
        }
        else if (zlib_get_coding_type() == FALSE) {
          $data = gzencode($data, 9, FORCE_GZIP);
        }
        // The remaining case is 'gzip' which means the data is
        // already compressed and nothing left to do but to store it.

But it was coming from the "zlib_get_coding_type() === FALSE" case, because I have zlib.output_compression = Off

I think that I have suffered the same issue you describe in #19... that was the reason we decided to set zlib.output_compression = Off.

Thanks for the patch!

But I still don't quite understand, I am reading your first patch where it says:

   '#description' => t("By default, Drupal compresses the pages it caches in order to save bandwidth and improve download times. This option should be disabled when using a webserver that performs compression."),
+    '#description' => t('By default, Drupal compresses the pages it caches in order to save bandwidth and improve download times. This option should be disabled when using a webserver that performs compression. This option requires that the <a href="@url">PHP zlib extension</a> is enabled.', array('@url' => 'http://www.php.net/zlib')),

I don't understand why you say "this option should be disabled when using a webserver that performs compression.". I am using zlib.output_compression = off, mod_deflate = On (so I am using web server compression), but still I want to save anonymous pages in compressed format. This is how my system is running and I'm not experiencing any issues.... if I follow your "description", I would have to set "page_compression" to FALSE, then I wouldn't save compressed pages in cache_page, ¿right?

Well, I am assuming that the following line:

AddOutputFilterByType DEFLATE text/html text/plain text/xml text/css application/x-javascript

is compressing my PHP output, because the PHP output has mimetype "text/html", ¿right?
maybe I am confused because this line is not compressing Drupal's output.... that might be my mistake,
I have found in google that to compress PHP output with mod_deflate the mimetype should be "application/x-httpd-php" ????

Sorry this will be my last post, I swear :)

So, what's the best option in terms of performance ?

0. zlib.output_compression = On, mod_deflate = On (ERROR)
1. zlib.output_compression = On, mod_deflate = Off (cache_page compressed data thx to zlib)
2. zlib.output_compression = Off, mod_deflate = Off (cache_page compressed data thx to zlib invoked directly)
3. zlib.output_compression = Off, mod_deflate = On (cache_page not compressed)

Your patch tries to fix an issue with the assumption that ob_get_contents() always returns compressed data when zlib.output_compression is On, which is number 1. in my enumeration list. ¿am I absolutely right :) ?

if I follow your "description", I would have to set "page_compression" to FALSE, then I wouldn't save compressed pages in cache_page, ¿right?

Yes, that is right. The contents of {cache_page}.data is only compressed when page_compression is TRUE. But pages would be compressed by mod_deflate on the fly.

Note that I didn't write this description. It was originally written in #121820: Cache with zlib results in double compression and later modified in #100581: Interface text for "page caching" -- query & suggested revision. AFAICT the reporter only talks about conflicts with zlib.output_compression and output_handler=ob_gzhandler, and not with external modules like mod_deflate. I believe the problem with zlib.output_compression is fixed by my patch. I am not sure there is a problem with mod_deflate.

So, what's the best option in terms of performance ?

I'd guess you get the best performance by using Drupal's page_compression in combination with either zlib.output_compression or mod_deflate (with page_compression data is compressed once when it is saved to the cache instead of on every request). But I think we should discuss this elsewhere :-)

Your patch tries to fix an issue with the assumption that ob_get_contents() always returns compressed data when
zlib.output_compression is On, which is number 1. in my enumeration list. ¿am I absolutely right :) ?

No. The output from ob_get_contents() is never compressed.

Reroll.

I just noticed that cache-gzip-2.patch contained an unrelated file. This patch is much smaller.

subscribe

Reroll (the latest test failures were due to #346529: Failures in node.test with assertFieldByXPath).

Reroll.

I could not reproduce the testbot failure on my machine. But here is a reroll.

Hi -
The monitoring tool I am using does not support gzip encoding and although "accept-encoding" is not set in the header, the server most always sends Drupal cached content with gzip encoding. With caching enable in Drupal, Page compression can be enable or disabled and the encoding is most always gzip. If caching is disabled the encoding is NOT gzip.

zlib.output_compression is set to off in php.ini and mod_deflate is commented out in httpd.conf.

Drupal version 6.5 is being used. The cache-gzip-6 patch was applied; however that seemed to make no difference in the behavior.

Is it possible for cached data to not be sent with gzip encoding?

Any help is appreciated.

Joe

My apologizes. I found the solution to my problem in this post: http://drupal.org/node/273618.

My issue was related to the cacherouter module.

Joe

I hope that some of you who are experiencing this bug will help review or test the patch. There is a description of how to reproduce the bug in comment #19. Thanks :-)

Reroll.

Related topics:
#187912: Problems with cache and zlib.output_compression

Possible duplicates:
#324890: weird unreadable compressed front page
#97847: cached pages appear blank, or garbage, or gzinflate error

Reroll.

+  /**
+   * Test page compression when zlib.output_compression is disabled. The test
+   * should pass even if zlib.output_compression is enabled in php.ini,
+   * .htaccess or similar, or if compression is done outside PHP, e.g. by the
+   * mod_deflate Apache module.
+   */
...
+  /**
+   * Test page compression when zlib.output_compression is enabled. Requires
+   * that zlib.output_compression is enabled in php.ini, .htaccess or similar.
+   */

Function summaries should go on one line and _not_ wrapped at 80 chars. If further description is required, add a blank line after the summary, followed by the description, wrapped at 80 chars.

Aside from that, patch looks good to go.

Updated with reformatted comment blocks.

Much better.

However, we need some feedback from testing framework maintainers for this patch, because zlib.compression might not be enabled on testbots (so this test won't ever run).

The testing servers meet the D7 requirements and a few specific memory requirements we put in place. Other than that I do not have direct access to the testing servers themselves. This is something we hope to fix with the deployment of the second generation framework.

In the meantime I'll leave a note (with Drupalcon) to ask hunmonk, DamZ, and greggles who control the three testing servers that are running. (I only have access to testing.drupal.org itself which does not run tests)

+    variable_set('zlib_output_compression', 'O');

This looks like the letter O.

Two remarks:

* zlib.output_compression cannot be enabled at runtime, but it can be disabled?

+    // zlib.output_compression cannot be enabled at runtime, so this test can
+    // only run, if it has been enabled in php.ini, .htaccess or similar. See
+    // http://bugs.php.net/bug.php?id=33653
+    if (!ini_get('zlib.output_compression')) {
+      return;
+    }

+      // $cache->data is already gzip'ed, so make sure zlib.output_compression
+      // does not compress it once more.
+      ini_set('zlib.output_compression', '0');

* Why not simply disabling output compression in .htaccess? Or make a requirement that it is disabled?

Changing the configuration of test slaves to enable output compression, while that is generally not recommended, makes no sense at all for me.

* zlib.output_compression cannot be enabled at runtime, but it can be disabled?

Apparently so :-/ See also http://bugs.php.net/bug.php?id=35936

Why not simply disabling output compression in .htaccess? Or make a requirement that it is disabled?

It seems that some users are using it (e.g. these). By default, Drupal only supports compression of anonymous page views, so there is a legitimate reason for using it. Personally I wouldn't miss this feature, but perhaps somebody would? Anyway, supporting it isn't a big problem (it's only one extra line of code in drupal_page_cache_header()), so for now I suggest we stick with that.

I think we should treat zlib.output_compression like we treat magic_quotes_gpc: We support it, but don't write explicit tests for it and (for now) we don't test it on the test slaves. This doesn't prevent people using unusual configurations from running the test suite and report issues for tests that are failing under certain circumstances.

I have updated the patch to not alter zlib.output_compression during tests. It should still be possible to run the complete test suite on a machine with zlib.output_compression turned on.

Changing the configuration of test slaves to enable output compression, while that is generally not recommended, makes no sense at all for me.

I agree. If we had a *lot* of test slaves, we could make some of them run the tests under various obscure configurations (with output compression and magic quotes), but for now this isn't possible.

My testing slave is set with

zlib.output_compression	Off	Off
zlib.output_compression_level	-1	-1
zlib.output_handler	no value	no value

If it should be changed let me know.

Not sure if this is covered already. When page cache is NOT enabled... I think one should be able to enable zlib.output_compression in PHP and exempt Drupal from doing the job. If so, when Drupal enables output buffering, then it probably needs to use ob_start('ob_gzhandler') rather than simply ob_start().

Also, would it be possible to back port this to D6?

When page cache is NOT enabled... I think one should be able to enable zlib.output_compression in PHP and exempt Drupal from doing the job.

This is already possible. Drupal itself does not compress pages when the cache is disabled or bypassed (authenticated users always bypass the cache), but zlib.output_compression will enable output compression for all page views. An alternative is mod_deflate.

Also, would it be possible to back port this to D6?

Yes, definitely.

Reroll.

Reroll (following #477944: Fix and streamline page cache and session handling).

Logic with patch is more clear, tested locally with FF3 IE6

+    $this->assertTrue(strpos(gzinflate(substr($this->drupalGetContent(), 10, -8)), t('No front page content has been created yet.')), t('Page was gzip compressed.'));

Help text of from page may change, suppose better to check site name

I changes the test to look for the title instead. The same pattern is used in simpletest.test and common.test, so changing it should be easy if the default title changes some day.

I changed the tests to simply look for </html> – much simpler.

#187912 is a duplicate of this.

Test client crapped.

Reroll (due to #500866: [META] remove t() from assert message).

... and here is the patch.

@c960657 Are you sure remove check for gzip unavailability?

-      if (zlib_get_coding_type() == 'deflate') {
-        $cache_page = FALSE;
-      }

Is there any UX about page-compression? I found only http://groups.drupal.org/node/24318 most recomended is "apache-driven" way

@andypost
zlib_get_coding_type() only reflects the current encoding used by zlib.output_compression if that is enabled. It is not necessarily an indication of whether gzip is supported in the client or server. These are checked at the top of drupal_serve_page_from_cache() - client support is detected via the $_SERVER[HTTP_ACCEPT_ENCODING], and server support using extension_loaded('zlib').

Newcomer here. I'd like to try the patch as I'm experiencing this issue on my 6.13 site. Would someone provide me with some instructions? Thanks.

This patch is outdated so it needs reroll

Reroll.

1) Added line to .htaccess
php_flag zlib.output_compression On

Directive Local Value Master Value
zlib.output_compression Off Off

2) Clean setup of cvs HEAD with normal profile + enabled page compression at /admin/config/development/performance

Results of testing:

3) telnet drupal7 80

GET / HTTP/1.0
Host: drupal7
User-Agent: foo
Accept-Encoding: gzip

result is binary stream cache_page holds binary

GET / HTTP/1.0
User-Agent: foo
Host: drupal7

result is plain

4) clear cache and again but different order

GET / HTTP/1.0
Host: drupal7
User-Agent: foo

result is plain, cache_page holds binary [!!!updated this binary NOT plain]

GET / HTTP/1.0
User-Agent: foo
Host: drupal7
Accept-Encoding: gzip

result is binary stream

So suppose this one fixed!!!

Now tests without patch but with zlib.output_compression On Off

 telnet drupal7 80

GET / HTTP/1.0
Host: drupal7
User-Agent: foo
Accept-Encoding: gzip

result is binary stream but cache_page holds plain

but without compression result is empty

debian:/var/www/d7# telnet drupal7 80
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
GET / HTTP/1.0
User-Agent: foo
Host: drupal7

HTTP/1.1 200 OK
Date: Sat, 25 Oct 2008 16:16:48 GMT
Server: Apache/2.2.9 (Debian) PHP/5.2.10
X-Powered-By: PHP/5.2.10 ZendServer/4.0
Set-Cookie: ZDEDebuggerPresent=php,phtml,php3; path=/
X-Drupal-Cache: HIT
Etag: "1224951291-0"
X-Generator: Drupal 7 (http://drupal.org)
Cache-Control: public, max-age=0
Last-Modified: Sat, 25 Oct 2008 16:14:51 +0000
Expires: Sun, 19 Nov 1978 05:00:00 GMT
Vary: Cookie,Accept-Encoding
Content-Length: 0
Connection: close
Content-Type: text/html; charset=utf-8

Connection closed by foreign host.

This could be related to that issue as well: #440182: Sometimes website generate Content Encoding Error

tracking this

Tested on another server, suppose it's ready

Nice! Looks like this simplifies the logic quite a bit. Dries's concern was lack of tests and we have tests now (nice to have for this feature), although I understand it might be inconsistently run by testing slaves.

Would've been nice for at least one of the people formerly crabbing in this issue about the problem to have tested the freaking patch, but can't have everything, I suppose. :P Thanks to andypost and a few others for your help, and c960657 for your excellent testing instructions.

Committed to HEAD. Thanks! Since this changes no APIs and this is apparently a bug in 6.x too, marking down to be ported.

So here is backport

subscribing

Successfully tested on D6.

Will this be backported for drupal 5.x ? Thanks

This one still not commited to 6, so chances are low to see it in D5

the batch 4 d6 doesn't seem to work, because i got an error like the pictore below (content encoding failure...the file cannot be shown, because you'r using an unknown kind of encoding..) !
i commented ob_gzhandler and it works again.

carligraph, did you clear the cache after having installed the patch?

subscribing

Great, thanks, committed.