WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#8726 closed defect (bug) (invalid)

cron implementation uses POST method instead of GET method

Reported by: patpro Owned by:
Milestone: Priority: normal
Severity: major Version: 2.7
Component: Administration Keywords:
Focuses: Cc:

Description

Background:
On my server, I use an antispam system that requires you to provide proper credentials before you can use the POST method. Only users with regular client software can POST to my webserver.

Problem:
Since the release of WordPress 2.7, the cron implementation (file wp-includes/cron.php) has switched from GET to POST. As WordPress is not a regular web browser, it's impossible for it to use POST successfully on my server (or on any server requiring credentials before allowing POST).

Solution:
Switch back to GET by using wp_remote_get instead of wp_remote_post.
By the way, $_POST is empty, so there is no reason to use POST method.

Note:
Possibility exists that wp_remote_post is used else where in the code to trigger local actions. This should be changed to GET unless POST is absolutely necessary (and then, it should be documented).

Attachments (1)

8726.diff (383 bytes) - added by jacobsantos 7 years ago.
change

Download all attachments as: .zip

Change History (5)

@jacobsantos7 years ago

change

comment:1 @jacobsantos7 years ago

  • Keywords has-patch commit added

comment:2 @Sam_a6 years ago

  • Keywords 2nd-opinion dev-feedback added

I think the current behavior may be correct.

The HTTP specs say GET requests retrieve a resource without side-effects, whereas POST requests take actions that can have side effects (including permanent changes).

URIs, Addressability, and the use of HTTP GET and POST

1.3 Quick Checklist for Choosing HTTP GET or POST

  • Use GET if:

o The interaction is more like a question (i.e., it is a safe operation such as a query, read operation, or lookup).

  • Use POST if:

o The interaction is more like an order, or
o The interaction changes the state of the resource in a way that the user would perceive (e.g., a subscription to a service), or
o The user be held accountable for the results of the interaction.

If it's important that WordPress be able to send credentials for cron requests, we could probably add a filter to do that -- the WP_Http_Request class can do HTTP authentication.

But I don't understand how switching from POST to GET would be a fix, because any credentials WordPress can (or can't) provide via POST, the same is true for GET.

comment:3 @jacobsantos6 years ago

Yeah, no, HTTP authentication support is in theory supported, but only if you, the plugin author, send the correct Headers yourself. The WordPress HTTP API will not automatically send the correct headers, or use the correct options for cURL and HTTP extension APIs.

So, yes, it could do HTTP authentication, but a patch would need to be made to support it and it wouldn't look pretty unless the HTTP API had native support for it.

I think the problem is more or less, that the behavior has changed and it shouldn't have changed. I think I originally used POST instead of GET, because the API is not retrieving anything.

I think it is a WTF that the server has such requirements for POST requests, but the change was unexpected for the user, so the patch was made to put the behavior back to the original. If it is affecting this user who reported the ticket, then it might be affecting more people.

If it isn't affecting more people, then yes, the setup the user has is fringe and isn't worth supporting. Well, if the change was major, but it wasn't. If it will fix the problem for the reporter with a simple change that doesn't require the user to keep making the same change for each upgrade, then I say commit it.

comment:4 @westi6 years ago

  • Keywords has-patch commit 2nd-opinion dev-feedback removed
  • Resolution set to invalid
  • Status changed from new to closed

I don't see a great benifit in changing this for this fringe case.

To be honest if you start adding requirements like HTTP Auth into the mix it is just going to get messy.

I assume this site doesn't support pingbacks or trackbacks either because the remote servers sending those def won't have credentials.

If we need some extra API to support sending AUTH that is a new issue and should be a new ticket.

Closing this a INVALID.

Note: See TracTickets for help on using tickets.