Determine if any WP_HTTP transport can be used before making the request

[mdawaffe]

It can be useful to know if any WP_HTTP transport passes its ::test() before making the corresponding request.

For example, consider OAuth.

Some OAuth providers (servers) offer both HTTP and HTTPS APIs (say ​http://api.example.com/ and ​https://api.example.com/) and leave it up to the Consumer (client) to decide which to use. The same request, however, cannot be made to both ​http://api.example.com/ and ​https://api.example.com/ because of the way the OAuth signature is calculated: the request URL is part of the hash. To calculate the signature, you have to know whether you will make an HTTP or an HTTPS request.

Some WordPress installations cannot make outgoing HTTPS requests because their hosts have disabled the OpenSSL extension. On such hosts, plugins would ideally use the HTTPS API and fallback to the HTTP API. It is not currently convenient to use WP_HTTP's ::test()s to determine ahead of time if outgoing HTTPS requests are possible.

Currently, plugins can do something like the following.

list( $url_signed, $args_signed ) = oauth_sign_request( $url, $args );
$request = wp_remote_request( $url_signed, $args_signed );
if ( is_wp_error( $request ) && 'http_failure' == $request->get_error_code() ) {
   // HTTPS -> HTTP
   $url = substr_replace( $url, '', 4, 1 );
   list( $url_signed, $args_signed ) = oauth_sign_request( $url, $args );
   $request = wp_remote_request( $url_signed, $args_signed );
}

The above is inefficient since the OAuth signature must be calculated twice and the ::test()s must be run twice.

Attached:

• Adds new WP_Http::get_first_available_transport( $url, $args ) method which returns the first transport to pass its ::test( $args, $url ).
• Adds a new transport_class option to WP_Http::request() so that the ::test()s don't have to be run multiple times.

The patch allows the following:

$http = _wp_http_get_object();
$args['ssl'] = true;
if ( $transport_class = $http->get_first_available_transport( $url, $args ) ) {
   // We already know which one to use
   $args['transport_class'] = $transport_class;
} else {
   // HTTPS -> HTTP
   $url = substr_replace( $url, '', 4, 1 );
   $args['ssl'] = false;
} 
list( $url_signed, $args_signed ) = oauth_sign_request( $url, $args );
$request = wp_remote_request( $url_signed, $args_signed );

In this snip, both the ::test()s and the OAuth signature are only run once. There's a few extra lines in this snip than in the previous one. Those lines could be removed with a couple straightforward tweaks to the patch.

See #16606, which also deals with making the WP_HTTP transports and their ::test()s more useful.

[sivel]

This is just a thought, but what if we added the functionality for a fallback URL, or fallback protocol? If the request can not be completed for the URL based on the tests, it can fallback to another URL or protocol.

Not that I am not open to new ideas, but it has come up several times that we should allow someone to be able to specify the transport they wanted to use, but it was generally dismissed, as that is not what the HTTP API was really meant for. I realize that this can already be done with filters, and I am actually for this change, I just want to make sure that we aren't just throwing out previous decisions without considering why they were done previously.

[mdawaffe]

Replying to sivel:

This is just a thought, but what if we added the functionality for a fallback URL, or fallback protocol? If the request can not be completed for the URL based on the tests, it can fallback to another URL or protocol.

That wouldn't help here. In the example above, the code needs to know what protocol can be used before signing the request. Having WP_HTTP auto-fallback to a different protocol would either: A. send the same request to the new URL (which wouldn't work because the signature would be wrong) or B. require the plugin to sign both requests ahead of time (which is even worse than the situation we're in now).

Not that I am not open to new ideas, but it has come up several times that we should allow someone to be able to specify the transport they wanted to use, but it was generally dismissed, as that is not what the HTTP API was really meant for.

It's a slight efficiency boost in this case, since the code will already have done the ::test()s; they don't need to be run again. With this use case, there's no change to how the WP_HTTP API is used. The code isn't transport bigoted; it still lets the WP_HTTP API choose.

[dd32]

My preferred method would be to keep the "worker" classes of the HTTP API private.

My alternate suggestion here is something akin to "supports", ie. wp_http_supports('ssl'), wp_http_supports('non-blocking'), or wp_http_supports( array('ssl', 'ssl-signing') );

[mdawaffe]

Replying to dd32:

My preferred method would be to keep the "worker" classes of the HTTP API private.

Part of the "straightforward tweaks" I mentioned would be to wrap things up in public functions, so this approach works for me.

My alternate suggestion here is something akin to "supports", ie. wp_http_supports('ssl'), wp_http_supports('non-blocking'), or wp_http_supports( array('ssl', 'ssl-signing') );

Since the ::test()s are filterable by $url, $args, it would need to be wp_http_supports( array('ssl', 'ssl-signing'), $url, $args );

Running wp_http_supports() before wp_remote_request() means the ::test()s would be run twice, which could (in theory) break some sketchy plugins doing crazy things with the ::test() filters.

That aside, it sounds like a good solution to me.

[dd32]

My initial thinking was that the supports argument would build a "fake" $args to simulate what it was testing, ie. request ssl, it runs ::test() with ssl = true , ssl signing = false, etc. If we take that method, it could just optionally override it with the 2nd arg (or really, merge it) when specified..

Also, It's only the $args passed to the test filters, the test filters are mainly there to disable the transport, not to do crazy things with it (that being said, I'm sure there are some out there)

The ::test() classes are reasonably simple beasts, running them multiple times is going to be a heck of a lot faster than most http requests. That might be a worthwhile 2nd ticket, caching of the ::test results for a certain $args+$url combination.

[mdawaffe]

Replying to dd32:

Also, It's only the $args passed to the test filters, the test filters are mainly there to disable the transport, not to do crazy things with it (that being said, I'm sure there are some out there)

Wow. I would have sworn the test filters had a URL as an argument :) Since they don't, your suggestion works fine for me.

The ::test() classes are reasonably simple beasts, running them multiple times is going to be a heck of a lot faster than most http requests. That might be a worthwhile 2nd ticket, caching of the ::test results for a certain $args+$url combination.

Yes - they are very fast, I'm not worried about caching - that seems like over optimization to me.

My concern was back-compat with these hypothetical crazy plugins, but that's much less of a concern for me now that I know the filters don't have a URL argument; the craziness of these hypothetical plugins is limited. Note that truly crazy plugins can unlimit the crazy with the http_request_args filter :)

PS: Perhaps ​http://core.trac.wordpress.org/browser/trunk/wp-includes/class-http.php?rev=17692#L229 is why I was thinking the filters had a URL parameter; the ::test() functions are called with $url as a second (but unused) parameter

[dd32]

Wow. I would have sworn the test filters had a URL as an argument :) Since they don't, your suggestion works fine for me.

PS: Perhaps.....

$url is passed to the functions, but they don't actually use it. There is a few tickets where using $url within the function might be needed (ie. to get the scheme), but none of them use it, that's why you're confused :) I added it whilst changing that section of code, and once it was in, I've seen no reason to change it.. It's flexibility if we need it in the future, as it is now, it does no harm

that seems like over optimization to me.

Exactly why it's not already done

[mdawaffe]

This ticket was marked as a bug because with the removed-in-3.2 http_transport_get_debug and http_transport_post_debug hooks, it was possible to implement the functionality which this ticket requests.

17251.2.diff

• Remove reference to old hooks.
• Add wp_http_supports( $capabilities = array(), $url = null ). $capabilities}...
• ... based on public WP_Http::_get_first_available_transport( $args ) (@access private).

wp_http_supports() accepts either enumerated capabilities or wp_remote_request() style $args.

wp_http_supports( 'ssl' );
wp_http_supports( array( 'ssl', 'sslverify' ) );
$url = 'https://example.com';
$args = array(
    'headers' => $blah,
    'timeout' => 30,
);
wp_http_supports( $args, $url );

In the last form, the first argument is the more useful, but it's nice to be able to pass $url as well so the function calculates the SSLness of the request for you. That means the arguments are reversed from the other functions in the family.

[westi]

In [17914]:

Introduce wp_http_supports as a much less hacky replacement for the http_transport_(get|post)_debug hooks that plugins could have
been using to detect if things like ssl requests were working.
See #17251 props mdawaffe

[dd32]

mdawaffe: Can we call this ticket fixed? or is there anything left?

[mdawaffe]

Suits my use case.