WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#3471 closed defect (bug) (fixed)

Fatal error messages are cached by proxy servers

Reported by: justdave Owned by:
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

If your wordpress blog is behind a caching proxy server, error messages such as "the database connection could not be established" get cached, so even after the database problem is fixed, you still have an error until you go flush the proxy server or wait for it to expire.

I understand that it's a bit difficult for PHP to tell a server to send a 500 error, but perhaps some cache control headers could be added to tell it not to cache.

Attachments (7)

3471.wp-die-header-patch.patch (803 bytes) - added by DD32 8 years ago.
3471.500.diff (2.4 KB) - added by DD32 8 years ago.
3471.500.2.diff (3.1 KB) - added by DD32 8 years ago.
same as 3471.500.diff, but adds a pretty error message for bad table prefix rather than a plain one.
3471.500.3.diff (3.2 KB) - added by DD32 8 years ago.
change based on markjaquith's comment
3471.500.4.diff (3.3 KB) - added by DD32 8 years ago.
3471.500.5.diff (3.2 KB) - added by markjaquith 8 years ago.
3471.500.6.diff (3.2 KB) - added by markjaquith 8 years ago.
slight change, because of [6108]

Download all attachments as: .zip

Change History (22)

comment:1 @JeremyVisser8 years ago

+101. Additionally, this will stop Google from indexing the database error page.

I mean, if I can do it (look at the header() statements in some of the templates), why can't WordPress? ;)

comment:2 @DD328 years ago

Added

header("HTTP/1.1 500 Internal Server Error");

to the 2 error pages/functions( Database failing to connect, and wp_die())
Hopefully this should prevent the page from being cached(I assume based on JeremyVisser's template), while still showing a pretty error page to the Browsers.

comment:3 @DD328 years ago

  • Version set to 2.1

comment:4 @DD328 years ago

  • Component changed from Administration to General
  • Keywords has-patch added

comment:5 @foolswisdom8 years ago

  • Milestone changed from 2.2 to 2.3

comment:6 @ryan8 years ago

If we do this the patch should use status_header()

comment:7 @DD328 years ago

If we do this the patch should use status_header()

Thats fine for in wp-includes/functions.php, however if the database is down, then the databse error is triggered before its included.

I've attached a new patch, It removes the html from wp-db.php, and calls wp_die instead, wp_die uses status_header().

I had to do a bit more hacking to make it work correctly, have a look at the diff for more info.

status_header() is defined in functions.php, so to use that for the database error, functions.php needed to be included first, Once doing that, i realised that the code was now duplicated in wp-db and wp_die, So i've got the datbase to just use wp_die(), and added a few extra checks in wp_die() to see if functions exist.

Also, If this gets slated for 2.4 rather than 2.3, the CSS location in wpdb->bail() points to the wrong location, so if this gets knocked back, that'll need fixing.

@DD328 years ago

@DD328 years ago

same as 3471.500.diff, but adds a pretty error message for bad table prefix rather than a plain one.

comment:8 @markjaquith8 years ago

$title = function_exists('__') ? __($title) : $title; 

You can't use the translation functions like this (passing a variable). You'll have to do an if/else.

comment:9 follow-up: @DD328 years ago

You can't use the translation functions like this (passing a variable).

Out of pure interest, What causes that?

@DD328 years ago

change based on markjaquith's comment

comment:10 in reply to: ↑ 9 @markjaquith8 years ago

Replying to DD32:

Out of pure interest, What causes that?

gettext is not a PHP parser, so it has no idea what is inside $title. So the issue is in generating the strings to be translated, for delivery to translators.

comment:11 @markjaquith8 years ago

Applied the patch, shut down MySQL.

Got a HTTP/1.x 200 OK header, not a 500.

comment:12 @DD328 years ago

Got a HTTP/1.x 200 OK header, not a 500.

just rechecked, status_header() requires the filters to be loaded, and for the wp-includes/vars.php to be included as well(for the status desc.).

So, it doesnt look like its possible at this stage of the code(before DB is connected) to rely on status_header().
which leaves 2 options, hard code a 500 error into wp_die(), or hard code a 500 error(and duplicate the wp_die() html) into wp-db.php

i'll attach a patch which uses status_header() if hooks are loaded, else, hard code to 500

@DD328 years ago

@markjaquith8 years ago

comment:13 @markjaquith8 years ago

See:

[6104] and [6107], which make 3471.500.5.diff possible.

comment:14 @DD328 years ago

which make 3471.500.5.diff possible.

Looks good to me.

@markjaquith8 years ago

slight change, because of [6108]

comment:15 @markjaquith8 years ago

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

(In [6110]) Use wp_die() for WPDB bails. Send 500 response and no-cache headers so wp_die()s are not cached. props DD32. fixes #3471

Note: See TracTickets for help on using tickets.