Having a location header does not mean that there should be a redirection.

[hakre]

Having a location header does not mean that there should be a redirection.

Automatic redirection depends foremost on the response code, not not having a location header.

See the descrption of #16888 for related RFC links.

Related: #11305

[hakre]

First digit counts

[hakre]

Out of scope of this ticket but related in the code: It fails if the response contains more than one location header line. Just running over it.

[hakre]

curl_getinfo might not return string - user comment in ​http://www.php.net/manual/en/function.curl-getinfo.php

[hakre]

And there was an additional error in the if clause which prevented it from working.

[dd32]

Unfortunately, I don't think it's going to be possible to adhere to this specification.

cURL(CURLOPT_FOLLOWLOCATION), HTTP Extension, and Streams (therefor PHP Internally) all follow redirects on 200 (ie. non-3xx) responses, this seems to be done (From what I can understand) for compatibility reasons with older web servers (unsure of which ones specifically)

It's probably best to standardise on following location regardless of the response code as we're currently doing.

[dd32]

based on my previous comment, and further research into if it's possible, I'm closing as invalid. It's simply not possible to adhere to it whilst relying on any current-generation HTTP API offered by PHP.

[hakre]

Replying to dd32:

Unfortunately, I don't think it's going to be possible to adhere to this specification.

cURL(CURLOPT_FOLLOWLOCATION), HTTP Extension, and Streams (therefor PHP Internally) all follow redirects on 200 (ie. non-3xx) responses, this seems to be done (From what I can understand) for compatibility reasons with older web servers (unsure of which ones specifically)

Most certainly by ​6.2 Response Header Fields and ​14.30 Location.

It's probably best to standardise on following location regardless of the response code as we're currently doing.

I now think so as well now. Thanks for your support so far.

[dd32]

I'm re-opening this to look into it again, hopefully things have changed enough, or at least changed that we can partially fix this.

In 3.4 the way Curl follows redirects has changed, it's moved from internal in curl, to being handled in PHP by us to make it more straight forward.

In addition, it's been pointed out that following Location headers causes problems for 201 (Created) requests, since WP_HTTP blindly follows the location, the result from the Created command is lost.

An example of a request which this fouls up is this the following Google Contacts API request (note, this output is taken directly from the curl command line binary)

> POST /m8/feeds/contacts/default/full/?access_token=......
> HTTP/1.1
> User-Agent: curl/7.21.4 (universal-apple-darwin11.0) libcurl/7.21.4 OpenSSL/0.9.8r zlib/1.2.5
> Host: www.google.com
> Accept: */*
> Content-Type: application/atom+xml
> Content-Length: 465
> 
< HTTP/1.1 201 Created
< Content-Type: application/atom+xml; charset=UTF-8
< Expires: Sat, 05 May 2012 01:36:29 GMT
< Date: Sat, 05 May 2012 01:36:29 GMT
< Cache-Control: private, max-age=0, must-revalidate, no-transform
< Vary: Accept, X-GData-Authorization, GData-Version
< GData-Version: 1.0
< Location: https://www.google.com/m8/feeds/contacts/user%40gmail.com/full/123/345
< Content-Location: https://www.google.com/m8/feeds/contacts/user%40gmail.com/full/123/345
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-XSS-Protection: 1; mode=block
< Server: GSE
< Transfer-Encoding: chunked

<?xml version='1.0' encoding='UTF-8'?>
...

If WP_HTTP makes the request it'll follow the Location header and the client can't retrieve the XML payload.

One can pass 'redirection' => 0 to the request and it "just works" (at least in the case of the curl transport) so hopefully that's a sign we can properly fix this now

[dd32]

In 1231/tests:

Rename the WP_HTTP testcase 'test_location_header_on_200' to 'test_location_header_on_201' and modify test / test script accordingly, PHP doesn't allow us to issue a Location with a 200 response, and the ticket it refers to is specifically about 201 responses.
See #16889

[dd32]

16889.diff​ is a patch which worksforme, and passes the unit tests (once I corrected the unit tests from attempting a 200 response (which PHP declines to send) to a 201) - Thats the first time in a long time, that I've seen 100% of the HTTP unit tests pass ;)

[Viper007Bond]

Worth having a filter in there?

[dd32]

In [24843]:
WP_HTTP: Abstract out the Redirection handling code into it's own method
and fix a bunch of redirection edgecases at the same time.
Fixes #17588
Fixes 16889
Props wonderboymusic and kovshenin for initial patches

[DrewAPicture]

16889.2.diff​ adds a trailing zero to the @since tag for the new WP_Http::handle_redirects() method, as well as types for the @param tags.

[dd32]

In 24890:

WP_HTTP: PHPDoc updates for WP_Http::handle_redirects(). Props DrewAPicture. Fixes #16889

[DrewAPicture]

phpdoc fixes for WP_Http::handle_redirects()