WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 9 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 9 years ago.
cookie-http-tests.zip (5.0 KB) - added by beaulebens 9 years ago.
Intended to be added to the existing http-tests (from #4779)
http-cookie-support-2.diff (16.9 KB) - added by beaulebens 9 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 9 years ago.
9049-defaults.diff (718 bytes) - added by beaulebens 9 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
9 years ago

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

#1 @beaulebens
9 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
9 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
9 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
9 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
9 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
9 years ago

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

#6 @ionfish
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

@ionfish
9 years ago

@beaulebens
9 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
9 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
9 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
9 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.