WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12529 closed defect (bug) (duplicate)

Specify a response for more wp_die calls

Reported by: aaroncampbell Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:

Description

Since moving to wp_die(), we've started returning 500 errors (the default) in a lot of cases where we shouldn't (see #12341). For example, wp-comments-post.php gives error messages like 'Error: please fill the required fields (name, email).' and 'Error: please enter a valid email address.' but it sends them with a 500 HTTP response along with them.

Currently wp_die is used 278 times in 86 files. However, we only specify an HTTP response code 3 times:
Once in wp-includes/comment.php, specifying a 403
Twice in wp-includes/functions.php, specifying a 404 and a 403

I can't imagine that there are 275 times that we want to return a 500 response. I'd like to use this ticket to point out specific places this should be changed, and figure out what it should be changed to.

Part of the reason I want to do this, is that I've noticed that automated systems trying to break a site seem to see a 500 error as a clue that they are getting closer to their goal. They seem to pound a lot harder on the scripts that return 500 errors, so I'd like to clean these up and only return them when they're actually appropriate.

Attachments (1)

12529.001.diff (1.6 KB) - added by aaroncampbell 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 nacin4 years ago

More or less a duplicate of #10551. Possibly wp_forbid, wp_404, etc., as wp_die wrapper functions.

comment:3 aaroncampbell4 years ago

I should have linked to the HTTP Status Code Definitions.

I'll start out by mentioning the ones that have been plaguing me most. If you look at wp-comments-post.php, it gives the following error messages, all returning 500 status codes:

  • Sorry, comments are closed for this item.
    • I think this should return a 403
  • Sorry, you must be logged in to post a comment.
    • I think this should return a 401 Unauthorized
  • Error: please fill the required fields (name, email).
    • Honestly, I think this should return a 200. It's an application error, but it seems like a 200 is appropriate.
  • Error: please enter a valid email address.
    • I think this should be a 200 as well.
  • Error: please type a comment.
    • It seems like this should also be a 200.

I'm attaching a patch for the ones listed above.

aaroncampbell4 years ago

comment:4 follow-up: aaroncampbell4 years ago

nacin: You're quick. Should we consolidate everything to one of those other tickets?

comment:5 in reply to: ↑ 4 nacin4 years ago

  • Milestone Unassigned deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Replying to aaroncampbell:

nacin: You're quick. Should we consolidate everything to one of those other tickets?

Sounds like a plan.

comment:6 aaroncampbell4 years ago

So this is a duplicate of #11286 then?

comment:7 nacin4 years ago

The patch in particular sounds more like #11286. #10551 seems like a good ticket to introduce wrappers.

Note: See TracTickets for help on using tickets.