Ticket #3528 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

HTTP 304 status not sent correctly on PHP 5.2, breaking conditional GET on feeds

Reported by: kelson Assigned to: anonymous
Priority: high Milestone: 2.0.7
Component: General Version: 2.0.6
Severity: major Keywords: needs-testing
Cc:

Description (Last modified by foolswisdom)

After upgrading to WordPress 2.0.6, I noticed that my feeds were no longer working. In several browsers and readers, including Firefox, IE7, Konqueror and Akregator, only blank files were being sent. Server logs indicated a 200 response with a few hundred bytes, but browsers showed only an empty file. Oddly, Opera, Dillo, and command-line GET displayed the files fine.

Looking at the actual HTTP response headers, it turned out that on conditional GETs that were supposed to issue 304 Not Modified, the server was actually issuing the following:

HTTP/1.1 200 OK
(other headers)
Status: 304 Not Modified

This resulted in a 200 OK status and empty response body.

I looked through and found that the status was being set in wp-includes/functions.php, in the status_header function. I changed the following line:

@header("Status: $header $text");

to this:

@header("Status: $header $text", TRUE, $header);

After making that change, status headers were sent correctly. Once I cleared the browser cache, feeds started loading again.

Going by the recommended method in the PHP manual, I also tried commenting out the if statement regarding the PHP API, so that only the following statement would run:

@header("HTTP/1.1 $header $text");

This also worked correctly.

This is with PHP 5.2.0 on Apache 1.3.37 (yes, I do intend to upgrade it eventually) using the mod_php interface.

Write-up at http://www.hyperborea.org/journal/archives/2007/01/05/feed-problems/

Change History

01/05/07 19:18:49 changed by kelson

  • version set to 2.0.5.

01/05/07 19:34:35 changed by kelson

Since Wikiformatting wrapped the lines, let's try this again. WP 2.0.6 out of the box does this: 

HTTP/1.1 200 OK
(other headers)
Status: 304 Not Modified

01/05/07 20:09:19 changed by foolswisdom

  • description changed.

01/05/07 20:19:20 changed by foolswisdom

  • version changed from 2.0.5 to 2.0.6.

Fantastic bug report!

Viper007Bond reported this issue #3435, unfortunately we (markjaquith) thought the issue was only present on trunk. Marking #3435 as duplicate of this ticket as this is more detailed/complete.

01/06/07 06:30:43 changed by kelson

Some additional info from MediaWiki? developer Brion, posted in the comments at my writeup:

This is the breaking change: 

-       @header("HTTP/1.1 $header $text");
-       @header("Status: $header $text");
+               if ( substr(php_sapi_name(), 0, 3) == 'cgi' )
+                       @header("HTTP/1.1 $header $text");
+               else
+                       @header("Status: $header $text");

Thus instead of hedging its bets by sending both forms to the server, it's only sending one form depending on how PHP's set up.

If I'm not mistaken, the condition should be reversed: the 'Status' pseudoheader should be used only for CGI, while Apache/mod_php and other environments want the HTTP/1.x line directly.

In MediaWiki? we use the HTTP/1.x form pretty exclusively, actually, and it's fine with my lighttpd/FastCGI test box as well as our production apache/mod_php boxes... I'm not entirely sure what it does break with.

01/06/07 08:19:05 changed by markjaquith

Sending both forms breaks FastCGI setups. Reversing the test causes issues.

IIRC, just using the HTTP/1.x line caused issues on some sort of setup... but if that's what you're using in MediaWiki? without issue, that might be our best bet. I'll put that into Trunk and 2.0

01/06/07 08:23:23 changed by ryan

Using Status for CGI sapis used to be a common workaround for some PHP bugs where HTTP/1.x would not work. See #2628. Some PHP 5 versions behaves differently. We've gone around with this for #3215. PHP bug 36705 is related.

http://bugs.php.net/bug.php?id=36705

Note that the http_response_code argument to header() requires php 4.3.0. If we were to add this argument as suggested in the description, we would need to do a version check.

01/06/07 08:40:47 changed by markjaquith

I think the problem with #2628 was that we were sending both and it was sending duplicate status headers.

So far, this is working for me, which is what Brion says MediaWiki? is using.

(In [4684]) burn in Hades, status_header(), destroyer of souls. fixes #3528

Please test. I hate this bug with every fiber of my being. Let's squash it dead. Stake-in-heart encased-in-concrete at the bottom of the ocean dead.

01/07/07 10:21:45 changed by Otto42

According to here: http://www.fastcgi.com/docs/faq.html#httpstatus

How do I send an HTTP status other than 200 (the default, HTTP OK) To return an HTTP status other than 200, add a 'Status:' header from your CGI. mod_fastcgi will look for that header and set the HTTP status. The Status: header is not sent to the client, but the HTTP status (first line of the server response) is.

Not that I agree with this or anything, but it is worth noting that that's how FastCGI says to do it. Can anybody make the thing break by using only the HTTP 1.x header?

01/07/07 10:56:04 changed by markjaquith

Otto42, if there's anything we've learned from this fun series of bugs, it's that there is often a chasm between documented procedures and actual results.

I haven't seen any ill effects from using only the HTTP 1.x header in either mod_php or FastCGI setups. Really hoping this is the winner. Please, test the hell out of it.

01/10/07 14:00:49 changed by markjaquith

Would appreciate feedback here, even if it is just "Hey, no problems on [your setup info]"

01/10/07 19:29:43 changed by foolswisdom

  • keywords set to needs-testing.

#wordpress-dev (02:27:48 AM) io_error: thinks this will fail on PHP < 4.3.5 and/or if cgi.rfc2616_headers is set (it's off by default)

01/11/07 04:47:52 changed by markjaquith

I tested on PHP 4.3.2 without problems (Apache 2.0, mod_php)

01/12/07 21:47:20 changed by markjaquith

(In [4724]) Use http_response_code for in status_header() on PHP >= 4.3.0 per Ryan's suggestion. relates to #3528

(In [4725]) That'll teach me to trust a code snippet from php.net ... typo fix from last commit. relates to #3528

This should buy us a little safety net for PHP 4.3.0 and above (which is the majority of our users). Please test!

01/16/07 01:17:33 changed by foolswisdom

  • status changed from new to closed.
  • resolution set to fixed.

Marking fixed as WordPress 2.0.7 has shipped.