WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

#2409 closed defect (bug) (fixed)

wordpress errors return status 200

Reported by: glub Owned by: rob1n
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.0.1
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Wordpress notices when a page doesn't exist or the database is down and
returns a human-readable error. However, the HTTP status code for these
pages remains 200. This means the page looks like a "real" page to
monitoring systems and programs like wget.

This bug is to request a change to the themes/default/404.php file and wp-includes/wp-db.php
file. Basically wordpress should call the php header() function to set the return
header when there is an error.

Here are the diffs that will accomplish this:

Index: wp-includes/wp-db.php
===================================================================
--- wp-includes/wp-db.php	(revision 420)
+++ wp-includes/wp-db.php	(working copy)
@@ -42,6 +42,7 @@
 	function wpdb($dbuser, $dbpassword, $dbname, $dbhost) {
 		$this->dbh = @mysql_connect($dbhost, $dbuser, $dbpassword);
 		if (!$this->dbh) {
+			header('HTTP/1.0 500 Database Not Available');
 			$this->bail("
 <h1>Error establishing a database connection</h1>
 <p>This either means that the username and password information in your <code>wp-config.php</code> file is incorrect or we can't contact the database server at <code>$dbhost</code>. This could mean your host's database server is down.</p>

Index: wp-content/themes/default/404.php
===================================================================
--- wp-content/themes/default/404.php	(revision 420)
+++ wp-content/themes/default/404.php	(working copy)
@@ -1,3 +1,4 @@
+<?php header('HTTP/1.0 404 Not Found'); ?>
 <?php get_header(); ?>
 
 	<div id="content" class="narrowcolumn">

Attachments (3)

500_error_when_database_fubar.diff (1.8 KB) - added by markjaquith 8 years ago.
2409.diff (3.0 KB) - added by davidhouse 8 years ago.
2409-rework.diff (3.7 KB) - added by rob1n 7 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 NathanWong8 years ago

The wp-db.php error should be in wpdb::bail(), I think, instead of right before the function is called (or a standalone error handling class, really, but that's a different problem).

comment:2 markjaquith8 years ago

  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

+1 for the idea

+1 for Nathan's suggestion of putting the 500 error in wpdb::bail();

comment:3 markjaquith8 years ago

  • Keywords bg|has-patch added
  • Milestone set to 2.1

Patch adds 500 error to wpdb::bail()

I also modified status_header() to accept 500 and also to optionally accept alternative $text messages.

Adding header() to the top of a template file is a bad idea, however... and that's really a separate issue than WP errors.

comment:4 markjaquith8 years ago

  • Keywords has-patch 2nd-opinion added; bg|has-patch removed

Any thoughts on this patch?

comment:5 davidhouse8 years ago

Perhaps we should include a general Status Code -> Message lookup array, pulled from the RFCs. So we can feel comfortable passing anything into it.

comment:6 markjaquith8 years ago

Yeah, would be cleaner. Those if() else if()s are about at capacity.

comment:7 davidhouse8 years ago

I'll get on to this tonight (in about 6 hours) if you can't in the mean time.

comment:8 technosailor8 years ago

This should be a 503 Service Unavailable error, not a 500 Internal Server Error. 500's are generally reserved for Web server configuration errors (i.e. Perl permissions, .htaccess/httpd.conf errors).

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

davidhouse8 years ago

comment:9 davidhouse8 years ago

Patch attached which replaces MarkJaquith's: send a 503 and include a resp code -> message array in vars.php with an accessor func, header_num_to_desc().

comment:11 foolswisdom8 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:12 matt7 years ago

  • Milestone changed from 2.1 to 2.2

comment:13 foolswisdom7 years ago

  • Milestone changed from 2.2 to 2.3

comment:14 rob1n7 years ago

  • Owner changed from markjaquith to rob1n
  • Status changed from assigned to new

comment:15 rob1n7 years ago

  • Status changed from new to assigned

rob1n7 years ago

comment:16 rob1n7 years ago

Okay, new patch does everything. Not so sure about the placing of the 404 one in WP_Query, though.

comment:17 rob1n7 years ago

(In [5446]) New status_header code, and WP-DB bail() errors send a 503 Service Unavailable. see #2409

Also, the new header code to text array has 302 as Found. fixes #4183

comment:18 rob1n7 years ago

Okay, that was the first half. Now we (I) need to figure out where to put the 404. I had it in WP_Query (didn't commit it), but that doesn't feel right. Maybe wp-settings.php, after WP_Query is initiated?

comment:19 rob1n7 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.