WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#3528 closed defect (bug) (fixed)

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

Reported by: kelson Owned by:
Milestone: 2.0.7 Priority: high
Severity: major Version: 2.0.6
Component: General Keywords:
Focuses: 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 (19)

#1 @kelson
11 years ago

  • Version set to 2.0.5

#2 @kelson
11 years ago

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

#3 @foolswisdom
11 years ago

  • Description modified (diff)

#4 @foolswisdom
11 years ago

  • 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.

#5 @kelson
11 years ago

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.

#6 @markjaquith
11 years ago

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

#7 @ryan
11 years ago

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.

#8 @markjaquith
11 years ago

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.

#9 @Otto42
11 years ago

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?

#10 @markjaquith
11 years ago

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.

#11 @markjaquith
11 years ago

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

#12 @foolswisdom
11 years ago

  • Keywords needs-testing added

#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)

#13 @markjaquith
11 years ago

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

#14 @markjaquith
11 years ago

(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!

#15 @foolswisdom
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Marking fixed as WordPress 2.0.7 has shipped.

#16 @rubys
8 years ago

  • Cc rubys@… added

#17 @rubys
8 years ago

  • Cc rubys@… removed

#18 @hakre
8 years ago

  • Keywords needs-testing removed

The Bug in PHP is fixed since a longer time (looks like it was done that way originally to address an issue with the underlying SAPI/CGI/FCGI implementations [said: apache]). Not an Issue for PHP 5.3 (cgi-fcgi) and PHP 5.2.6 (cgi-fcgi) [at least 5.2.6 might be fixed earlier.].

So these PHP Versions do not need that fix any longer.

Some impropper written Server / CGI APIs might need this change still but I suggest to move that into the plugin domain if this ain't a bug for a min req. php version any longer.

I think three years of needing testing, is enough, right?

#19 @hakre
8 years ago

Status Header Related: #13940

Note: See TracTickets for help on using tickets.