#18589 closed defect (bug) (fixed)

class WP_Http_Curl missing support for DELETE method

Reported by: jbrinley Owned by: dd32
Priority: normal Milestone: 3.4
Component: HTTP Version:
Severity: normal Keywords: has-patch needs-testing 3.4-early
Cc: jonathanbrinley@…

Description

WP_Http_Curl ignores any request methods other than HEAD, POST, and PUT, treating anything else as GET.

I'm particularly interested in the DELETE method, although a reasonable case could be made for also handling OPTIONS and TRACE.

The switch statement in WP_Http_Curl::request() just needs a new case for each:

case 'DELETE':
	curl_setopt( $handle, CURLOPT_CUSTOMREQUEST, 'DELETE' );
	break;
case 'OPTIONS':
	curl_setopt( $handle, CURLOPT_CUSTOMREQUEST, 'OPTIONS' );
	break;
case 'TRACE':
	curl_setopt( $handle, CURLOPT_CUSTOMREQUEST, 'TRACE' );
	break;

Attachments (2)

18589.diff (668 bytes) - added by jbrinley 21 months ago.
Patch to add DELETE, OPTIONS, and TRACE to WP_Http_Curl
wordpress-curl-http-custom-method-fix.patch (522 bytes) - added by zx2c4 20 months ago.
A much better more generic solution to support all sorts of methods.

Download all attachments as: .zip

Change History (20)

Patch to add DELETE, OPTIONS, and TRACE to WP_Http_Curl

  • Keywords has-patch added
  • Keywords needs-testing added

We'll need to ensure that the way those requests are handled by Curl, is the same as the way it's handled by the other transports.

comment:3 follow-up: ↓ 4   dd3220 months ago

Closed duplicate: #18727 - wp_remote_request with HTTP Curl Class Does Not Support Custom HTTP Methods

The patch noted there of simply setting default: curl_setopt( $handle, CURLOPT_CUSTOMREQUEST, $r['method'] ); seems like a reasonable option at first glance.

comment:4 in reply to: ↑ 3   jbrinley20 months ago

Replying to dd32:

The patch noted there of simply setting default: curl_setopt( $handle, CURLOPT_CUSTOMREQUEST, $r['method'] ); seems like a reasonable option at first glance.

Agreed. zx2c4's patch looks like a better, more flexible option, and it's more in line with how the other transports handle arbitrary methods.

  • Owner set to dd32
  • Status changed from new to accepted
  • Type changed from enhancement to defect (bug)

Marking as a defect as the curl class handles them differently than the others.

It seems no validation occurs on the other classes at all, with Curl limiting it to GET/POST/HEAD/PUT.

zx2c420 months ago

A much better more generic solution to support all sorts of methods.

I'm satisfied now; it's been sufficiently tested for my standards:

My patch has been running on a site that receives ~300k hits a week for the last couple days with zero regressions.

I tested this and it works perfectly.

Can this be included on the next release ?

  • Type changed from defect (bug) to enhancement
  • Type changed from enhancement to defect (bug)

This is most certainly not an enhancement, but rather a bug, as it introduces a significant regression, since prior versions of wordpress contained the correct functionality, and so 3rd party code that depended on that functionality is now broken. This is a bug, a regression. This is not a mere enhancement request.

I agree this is a bug, but where is the evidence that it is a regression?

comment:12 follow-up: ↓ 13   zx2c419 months ago

It worked with the previous version of wordpress. This is widely accepted. Popular plugins such as w3 cache rely on it (for the Varnish support).

You acknowledged that it is a bug now.

worked on previous version + is a bug now = what we call a regression

comment:13 in reply to: ↑ 12   dd3219 months ago

  • Keywords 3.4-early added
  • Milestone changed from Awaiting Review to Future Release

Replying to zx2c4:

It worked with the previous version of wordpress. This is widely accepted. Popular plugins such as w3 cache rely on it (for the Varnish support).

You acknowledged that it is a bug now.

worked on previous version + is a bug now = what we call a regression

It has never worked in previous versions reliably, WP_HTTP has 3 different transports which might be used depending on server configuration, The Curl handler has never handled non-GET/HEAD/POST requests. The other transports have supported it, but simply by coincidence. The now-removed HTTP Extension transport didn't support it either.

This is a defect, however, not a regression.

The default chosen handler supported it. Now the default chosen one doesn't. But sure. Maybe it's been partially broken (unimplemented) all along, and it just never surfaced until now.

  • Milestone changed from Future Release to 3.4
  • Resolution set to fixed
  • Status changed from accepted to closed

In [19696]:

Support custom HTTP Methods in WP_Http_Curl. Brings it in line with the other HTTP transports of respecting the requested method. Props zx2c4. Fixes #18589

  • Resolution fixed deleted
  • Status changed from closed to reopened

One critical part was left out from this change, GET variables are processed as expected, however the body being sent (that we usually refer to as POST data) isn't being sent.

As a result, using methods such as PATCH which expect a body, causes the remote server to wait for the data.. which isn't going to be sent. causing a timeout. Ignoring the timeout issue, custom requests are useless without adding the body to the request.

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

In [20183]:

WP_HTTP: Send the body with custom method requests when using cURL. Fixes #18589

Note: See TracTickets for help on using tickets.