WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#2360 closed defect (bug) (invalid)

Error caused by forcing a 200 HTML status code in URL rewrites

Reported by: capt_kirk Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.0
Component: Optimization Keywords: URL rewrite error bg|has-patch bg|dev-feedback
Focuses: Cc:

Description

In wp-includes/classes.php, near the bottom, there is the following function:

	function handle_404() {
		global $wp_query;
		// Issue a 404 if a permalink request doesn't match any posts.  Don't
		// issue a 404 if one was already issued, if the request was a search,
		// or if the request was a regular query string request rather than a
		// permalink request.
		if ( (0 == count($wp_query->posts)) && !is_404() && !is_search() && ( $this->did_permalink || (!empty($_SERVER['QUERY_STRING']) && (false === strpos($_SERVER['REQUEST_URI'], '?'))) ) ) {
			$wp_query->set_404();
			status_header( 404 );
		}	elseif( is_404() != true ) {
			status_header( 200 );
		}
	}

The last part

			elseif( is_404() != true ) {
			status_header( 200 );
		}

causes a 200 HTML status code to be issued on all failed page lookups in a PHP-CGI server environment. This incorrectly overrides the 302 HTML status code that Apache is looking for on mod rewrite initiated rewrites.

Specifically, if an embedded program (like Gallery2 using the WPG2 plugin) has .htaccess rules to redirect pages to the Gallery2 pages, they should have a 302 status code, but classes.php is incorrectly assigning a 200 status code.

Please elimiate the offending elseif statement. It appears to have no particular purpose and is breaking embedded links using URL rewrites.

If someone needs to see specific examples, I can convert my testbed back to the original classes.php and show you the errors induced by the 200 status error.

Thanks,
Kirk Steffensen

Change History (13)

comment:1 @capt_kirk10 years ago

  • Component changed from Administration to Optimization

comment:2 @westi10 years ago

  • Keywords bg|has-patch bg|dev-feedback added

comment:4 @ryan10 years ago

Too late for 2.0.1. Let's consider what with this for the next release.

comment:5 @capt_kirk10 years ago

Another option to deleting the elseif would be to capture the original HTML status code and offer it up at the elseif. That way an original 302 from Apache's mod rewrite would stay 302 and Apache wouldn't choke on the return from WP.

comment:6 @skeltoac10 years ago

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

PHP header() replaces already-defined headers. Why can't your plugin just do header("HTTP/1.1 200")? Any plugin that needs to control the header should simply do so. The last header call can overwrite earlier calls to set the same header. http://us2.php.net/manual/en/function.header.php

My testing with this function shows that you can leave the status text ("Not Found", etc.) out of the message and it will be populated correctly by the time it reaches the browser.

comment:7 @capt_kirk10 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Here's the deal: It's NOT the plugin setting the header.

Please go to http://www.steffensenfamily.com/

Then click on the "Photo Gallery" link at the top of the page.

The resulting page is a WordPress wrapper (header and footer) around the Gallery2 content. This is the definition of Gallery2 in embedded mode.

Gallery2's URL rewrites are correct in mod rewrite. On a server that is running PHP as an Apache module, there is no problem. But on a server that is running PHP-CGI, the incorrect 200 status causes an error. The problem is that Apache knew it sent a 302 status, and when it comes back as a 200 status, it sees it as a failed rewrite by PHP-CGI and dies, instead of seeing it as a rewrite that PHP-CGI passed on and needs to take further action.

It is not just Gallery2 rewrites that are affected. Any rewrites that Apache assigns a 302 are affected. Please see the thread that Ryan linked to above to see that at least one other tester has the same problem in a different environment.

I don't understand why this was set to "wontfix." This is clearly an error in handling HTML status codes on rewrites. An error in programming should be fixed.

comment:8 @capt_kirk10 years ago

I have "un-commented-out" the elseif in my classes.php to fully demonstrate the error.

Please go to
http://www.steffensenfamily.com/wordpress/v/2004_12_Christmas/ and click on any of the photos.

Then click on the "details" link just below the "Photo Properties" header under the photo.

You will get the error. If you look at the top of the page it says "200 OK" but if you look in my raw access logs, the POST was a 302 status message. This confuses Apache, which dumps the error screen you see. If you look at the Apache code where this error screen is generated, the message "The document has moved here." is for a status code of 302, but it is putting "200 OK" in the window title because it thinks that the rewrite failed, instead of just being passed on.

The mod rewrite rules for both WordPress and Gallery2 are perfectly well formed. They work in every case when not running under PHP-CGI (probably at the cost of extra computer cycles to overcome errors like this one), but under PHP-CGI, the incorrect 200 status code chokes Apache.

This is not an error in either WPG2 or Gallery2. It is an error in forcing an incorrect status code at the end of classes.php.

comment:9 @capt_kirk10 years ago

For WPG2, changing line 31 in wp-gallery2.php from

require('./wp-blog-header.php');

to

require('./wp-config.php');
$wp->init();
$wp->parse_request();
$wp->send_headers();
$wp->query_posts();
$wp->register_globals();

fixes the 200 HTTP status code error by omitting $wp->handle_404() while initializing everything else.

This is a kludge that works around the error induced by handle_404(). While it fixes the problem for WPG2, it does not fix the root problem of incorrectly handling an Apache mod rewrite 302 HTTP status code in a PHP-CGI environment.

comment:10 @capt_kirk10 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

I finally found the root cause thanks to Andy Skelton's assistance in pointing me in the right direction.

There is a discussion of the PHP-CGI bug in http://us3.php.net/header that winds around, but gave me the answer.

In gallery2/main.php, after the line

$phpVm->header("Location: $redirectUrl");


I inserted

   /* Following line added to fix a bug in PHP-CGI where the status code is not properly set */ 
   $phpVm->header("Status: 302");


and it worked perfectly with the original wp-gallery2.php.

It turns out that PHP does not properly implement the CGI spec on setting headers and you have to do it manually.

A huge thanks to Andy for the great dialog that eventually narrowed in on the root cause and an elegant solution.

I still question forcing the 200 at the end of the elseif in classes.php's handle_404(), but I'll close this ticket out for now. If someone else has an error induced by handle_404() they can reopen it.

comment:12 @foolswisdom9 years ago

leflo commented in #3166 that it will not fix this issue. "It won't fix anything where wp_redirect isn't called."

comment:13 @Nazgul8 years ago

  • Milestone 2.1 deleted
Note: See TracTickets for help on using tickets.