WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#9049 closed enhancement (fixed)

Support for sending and receiving cookies in the HTTP API

Reported by: beaulebens Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: HTTP API Keywords: has-patch needs-testing
Focuses: Cc:

Description

Attached is a patch for the HTTP API (wp-includes/http.php) which adds support for sending and receiving cookies. I've also included some tests (cookie*.php) that work in the same fashion as the "http-tests.zip" attached to #4779 (put them in a folder called "http-tests" immediately in the root of a WP install).

This patch supersedes/assumes #9037 to allow for handling multiple Set-Cookie: headers which some servers send. If this gets committed then it would also close #8727.

It works something like this:

$response = wp_remote_get( $url_that_will_send_back_cookies );
$cookies = array();
if (isset($response['cookies'])) {
	$cookies = $response['cookies']; // Array of WP_Http_Cookie objects
}
$cookies[] = new WP_Http_Cookie( array( 'name' => 'custom-cookie', 'value' => 'adding a manually created cookie' ) );
$response = wp_remote_get($url_that_will_accept_cookies, array('cookies'=>$cookies));

Cookies are never written to file (unless you choose to do so yourself), they are only maintained in memory between requests.

I have not tested the ExtHTTP (PHP 5.2+ http extension) implementation at all yet, because I'm having trouble getting the extension installed. It should work the same as the other transports, but it needs testing.

Attachments (5)

http-cookie-support.diff (17.2 KB) - added by beaulebens 10 years ago.
cookie-http-tests.zip (5.0 KB) - added by beaulebens 10 years ago.
Intended to be added to the existing http-tests (from #4779)
http-cookie-support-2.diff (16.9 KB) - added by beaulebens 10 years ago.
Changed WP_Http_Fopen() so that it silently ignores cookies, as per other headers (since it can't send request headers)
9049.notices.diff (531 bytes) - added by ionfish 10 years ago.
9049-defaults.diff (718 bytes) - added by beaulebens 10 years ago.
These notices should be addressed in the default args for each request. This patch addresses them that way so that ionfish's patch shouldn't ever be necessary.

Download all attachments as: .zip

Change History (14)

@beaulebens
10 years ago

Intended to be added to the existing http-tests (from #4779)

#1 @beaulebens
10 years ago

I forgot to mention that this patch also fixes these 2 issues, which should otherwise be addressed separately:

  1. WP_Http_Curl() currently doesn't handle custom request headers properly, since cURL expects a different format array than the other transports.
  2. Adjusted the format of response arrays when they are returned early (due to non-blocking) so that they are the same format as normally, just empty. Their response code/message elements were different.

#2 follow-up: @jacobsantos
10 years ago

I think you should silently fail, when the transport doesn't support cookies, but since it isn't used anywhere in core, then I think it should be okay. I think it should still send the request without it, unless it is for authentication or something.

#3 in reply to: ↑ 2 @beaulebens
10 years ago

Replying to jacobsantos:

I think you should silently fail, when the transport doesn't support cookies, but since it isn't used anywhere in core, then I think it should be okay. I think it should still send the request without it, unless it is for authentication or something.

I actually thought about that after I did it. Just sending the request without the cookies at all seems to make sense, although it'd be nice to throw a warning perhaps?

@beaulebens
10 years ago

Changed WP_Http_Fopen() so that it silently ignores cookies, as per other headers (since it can't send request headers)

#4 @ryan
10 years ago

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

(In [10512]) Cookie support for HTTP API. Props beaulebens. fixes #9049 #9037 #8727

#5 @beaulebens
10 years ago

I'm now using this on the LiveJournal importer (#8999) and the cookie handling is working just fine :)

#6 @ionfish
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The cookie check is generating a notice; patch is attached.

@beaulebens
10 years ago

These notices should be addressed in the default args for each request. This patch addresses them that way so that ionfish's patch shouldn't ever be necessary.

#7 @ryan
10 years ago

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

(In [10563]) Add cookies arg to defaults to avoid warnings. Props beaulebens. fixes #9049

#8 @ryan
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This or a related cookie change seems to have broken spawn_cron() for the curl transport.

#9 @ryan
10 years ago

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

The problem was due to the change in how headers are passed for curl. Since the change makes curl consistent with the transports, I just changed the calling code to accommodate. Putting this back as fixed.

Note: See TracTickets for help on using tickets.