HTTP API may make a request on an unsupporting transport

Description

At present, When the HTTP API is called, It iterates through the transports testing each one to see if it can support the current request params (eg. SSL). This result is then stored for future requests.

The issue i've run into, Is that i've added a case to a transport, that it should not be used if its a HEAD request (As it simply doesnt support it on that transport), The problem is, if a request is made before me, for say, a GET request, that transport gets cached for it, even if it doesnt support it..

The attached patch adds a $key to the cache, which is a md5 of the serialized args.

In most cases, this doesnt cause any issues, but if a host is using a lesser transport which cannot do everything (Such as FOpen) then errors can be hit that the transport doesnt support it.

won't the serialized args based key result in each transport being tested on every call?

Just the calls which have varying args. The only non-arg in that list is the post body, which can probably be nulled out, that way multiple post requests will be able to use the cache again. It doesnt contain the URL, so not a problem there.. But does differentiate between GET/POST/etc.

It is unavoidable that they get tested multiple times, It'd be much simpler if the fallback to the next transport should the first fail worked properly, #8622 that way, the arg could run ::test() itself when being called, and if it wasnt up for the job, simply pass it onto the next best transport.

Most pages will only have a small number of requests made, while it may add another couple of ms to the function, in my opinion, the benifit of the request actually suceeding and returning the correct data is worth more.

I've uploaded 11613.5.diff, which is based on the .4.diff. The difference is it checks for isset() rather than empty(), since checking for empty when there are no valid transports could potentially lead the tests to be redone each time.

Re tests done, I merely stuck to verifying that RSS widgets and update checking still work.

If this is the case, then why won't you just do the check in the transport and then change the HTTPS scheme to a HTTP one? The transports should only be checking whether it supports SSL, supports sending a body and supported by PHP. Everything else should just be done in the transport, unless it is something that needs to be supported by every transport then needs to be a method or a class.

then why won't you just do the check in the transport and then change the HTTPS scheme to a HTTP one?

Changing from HTTPS to HTTP is a no-no, If someone asks for a HTTPS connection, You give them a HTTPS connection.

The problem here is, that not all the transports can support a HTTPS connection, If you have multiple connections being made on a pageload, the first request determines which transport is used for that connection

So if the first request is a HTTP request, It may be made over Class_NoSSL, Then a HTTPS request comes along (of the same GET/POST type), it'll be passed to Class_NoSSL as ::test() passed last time.

This is due to #8622, Which should probably be fixed instead of this, or along side.

I see, thanks for the clarification. To me the ticket seemed to be that servers did not support secure HEAD requests and should be non-SSL instead. Given that it is quite frankly improbable that a WordPress installation is going to change servers any time soon, it seems fair to say that a refactoring of the code is in order. Right now it seems quite the mess.

With the removal of the fopen transport (Which only supported GET requests), there is no longer a requirement for a difference in the GET/POST transport orders.

This in addition to #8622 is preventing unit testing of the HTTP API as well.

The following commit replaces _getTransport() & _postTransport() and adds a _dispatch_request() private method, the simple purpose of _dispatch_request() is to take the $args and determine a transport which the request can be made on, and then dispatch the request to that transport worker. (In addition, it clarifys the PHPDoc on the return of WP_HTTP::request() that it can return a WP_Error instance upon failure)

(In [17550]) Fix WP_HTTP to only make a request upon a working transport, as well as to allow Unit Testing. Removes the getTransport() & postTransport() methods as they're no longer needed, replaces them with a single _dispatch_request() method. Fixes #11613