Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#37456 closed defect (bug) (fixed)

HTTP API regression in transmitted content and headers

Reported by: clorith's profile Clorith Owned by: ocean90's profile ocean90
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.6
Component: HTTP API Keywords: has-patch
Focuses: Cc:

Description

The HTTP API changed how it sends data in 4.6, possibly related with the move to Requests, as it appears to be the only noticeable change to the HTTP API since 4.5?

The following test call was made on both 4.5 and 4.6

<?php
wp_remote_post(
        'http://127.0.0.1/http_test.php',
        array(
                'method'  => 'DELETE',
                'body'    => array( 'tags' => array( 'example' ) ),
                'timeout' => 20
        )
)

http_test.php is just a simple capture file that does a var_dump on apache_request_headers, $_GET $_POST and stream content.

In 4.5 this transmits the following headers with empty _POST and _GET and transmits the data as a stream (capturable with file_get_contents( 'php://input' )):

'User-Agent' => 'WordPress/4.5.3; http://127.0.0.1/wordpress',
'Host' => '127.0.0.1',
'Accept' => '*/*',
'Accept-Encoding' => 'deflate;q=1.0, compress;q=0.5, gzip;q=0.5',
'Content-Type' => 'application/x-www-form-urlencoded; charset=UTF-8',
'Content-Length' => '19'

The content of the streamed data is a single string as such

tags%5B0%5D=example

In 4.6 this behavior changed, we are now seeing the following headers, with an empty _POST and stream, but a populated _GET value:

'User-Agent' => 'WordPress/4.6-beta4-38147; http://127.0.0.1/wordpress',
'Host' => '127.0.0.1',
'Accept' => '*/*',
'Accept-Encoding' => 'deflate, gzip',
'Referer' => 'http://127.0.0.1/http_test.php?tags%5B0%5D=example',
'Connection' => 'close',
'Content-Length' => '0',
'Content-Type' => 'application/x-www-form-urlencoded'

And as mentioned the streamed data is empty, but the _GET contains the array data:

array (
  'tags' => 
  array (
    0 => 'example',
  ),
)

As reported by @ze3kr in https://wordpress.org/support/topic/a-bug-of-delete-method-of-wp_remote_post-in-v46-beta4

Attachments (2)

37456.diff (4.2 KB) - added by dd32 8 years ago.
One method of respecting the data format
37456.2.diff (1.9 KB) - added by dd32 8 years ago.
The better method of setting data_format=body.

Download all attachments as: .zip

Change History (8)

#1 @swissspidy
8 years ago

  • Keywords needs-patch added

Although a request body isn't explicitly allowed or disallowed for DELETE requests in the spec, it looks like Requests supports it. See https://github.com/rmccue/Requests/issues/91

Sort of related: #37364

#2 @ocean90
8 years ago

@rmccue, @dd32: Do you have any idea what's going on here?

#3 @dd32
8 years ago

This happens because Requests defaults to _GET/query for HEAD/GET/DELETE and _POST/body for POST/PUT/OPTIONS/PATCH.

@rmccue could explain why that is.. but I'm fairly sure to retain compatibility with WP_HTTP we should just force data_format to body for all non-GET/HEAD requests.

@dd32
8 years ago

One method of respecting the data format

@dd32
8 years ago

The better method of setting data_format=body.

#4 @dd32
8 years ago

  • Keywords has-patch added; needs-patch removed

@rmccue Can you review 37456.2.diff please when you're off a plane? :)

#5 @rmccue
8 years ago

Patch looks good to me, though I'd switch to strict equals.

Requests does this to fit the common use case; until recently, DELETE was commonly treated the same as GET, with bodies being ignorable.

#6 @ocean90
8 years ago

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

In 38165:

HTTP API: All non-GET/HEAD requests should put the arguments in the form body.

Requests defaults to _GET/query for HEAD/GET/DELETE and _POST/body for POST/PUT/OPTIONS/PATCH. For backward compatibility WP_HTTP needs to force data_format to 'body' for all non-GET/HEAD requests.

Props dd32.
Fixes #37456.

Note: See TracTickets for help on using tickets.