[wp-trac] [WordPress Trac] #25738: WP_HTTP uses transports that incorrectly claim to support a request

[WordPress Trac]

#25738: WP_HTTP uses transports that incorrectly claim to support a request
--------------------------+------------------
 Reporter:  dd32          |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  3.8
Component:  HTTP          |     Version:
 Severity:  normal        |  Resolution:
 Keywords:                |
--------------------------+------------------

Old description:

> Following on from #25716
>
> We now have 2 HTTP transports, and 2 types of requests that we need to
> support:
> || ||HTTP||HTTPS||
> ||cURL|| || ||
> ||PHP Streams|| || ||
>
> If we break it down further based on known compatibility issues, we can
> end up with this:
>
> || ||HTTP GET||HTTPS GET||HTTP POST||HTTPS POST||
> ||cURL ''no SSL''||✓||✘||✓||✘||
> ||cURL w/ OpenSSL||✓||✓||✓||✓||
> ||cURL w/ GnuTLS||✓||~||✓||~||
> ||cURL 7.31.0 w/ OpenSSL||✓||✓||✓||✘||
> ||cURL + DNS timeout||✘||✘||✘||✘||
> ||PHP Streams ''no SSL''||✓||✘||✓||✘||
> ||PHP Streams w/ OpenSSL||✓||✓||✓||✓||
> (✓ = should work, ✘ = doesn't work, ~ = known issues)
>
> For background of cURL 7.31.0 and GnuTLS see #25716
>
> WP_HTTP currently
> * Tries to use cURL first, Streams second
> * Only uses cURL for HTTPS if it ''claims'' to support HTTPS, we have no
> check in place to verify it can make a SSL connection
> * Doesn't fall over to a 2nd transport in event of error
> * Doesn't disable a transport in event of continued troubles
>
> I think there are a few things we can possibly do here:
> 1. Disable a transport after multiple failed requests (See #16870)
> 1. Add a parameter to try the other transport if available (and if that
> works, lock it to using that transport for the rest of that page load so
> we have less timeouts)
>
> !#1 would be a win for most users who have issues with cURL and DNS
> lookup failures, and any sites which have plugins that make a bunch of
> external HTTP calls.
> !#1 can also have false positives, for example, oEmbed calls to sites
> that are down could disable a sites ability to make requests, so we might
> want to limit it to *.wordpress.org requests as a way of saying "This
> site SHOULD be working"
> !#1 and !#2 might not play nicely though if !#2 was implemented, we might
> not have enough data to reliably trigger the blocking.
>
> !#2 would mean that we could use that for WordPress.org API requests,
> which would cause any HTTPS connections that fail via cURL succeeding
> through Streams (if supported).
> Since it's possible for a site to have cURL with broken SSL, and no
> OpenSSL installed, we would still need extra code to handle a HTTPS ->
> HTTP fallback in the update API's though (perhaps this would be good as a
> wrapper "connect over SSL if possible, else, use HTTP)
>
> We also probably don't want to re-try every HTTP request, which makes
> adding a parameter for it probably a better idea, the other option would
> be to simply detect certain cURL/streams error codes and re-try those.
>
> potentially we wouldn't want it to re-try zip package downloads too

New description:

 Following on from #25716

 We now have 2 HTTP transports, and 2 types of requests that we need to
 support:
 || ||HTTP||HTTPS||
 ||cURL|| || ||
 ||PHP Streams|| || ||

 If we break it down further based on known compatibility issues, we can
 end up with this:

 || ||HTTP GET||HTTPS GET||HTTP POST||HTTPS POST||
 ||cURL ''no SSL''||✓||✘||✓||✘||
 ||cURL w/ OpenSSL||✓||✓||✓||✓||
 ||cURL w/ GnuTLS||✓||~||✓||~||
 ||cURL 7.31.0 w/ OpenSSL||✓||✓||✓||✘||
 ||cURL cURL 7.10.6||✓||~||✓||~||
 ||cURL + DNS timeout||✘||✘||✘||✘||
 ||PHP Streams ''no SSL''||✓||✘||✓||✘||
 ||PHP Streams w/ OpenSSL||✓||✓||✓||✓||
 (✓ = should work, ✘ = doesn't work, ~ = known issues)

 For background of cURL 7.31.0 and GnuTLS see #25716
 For background of cURL 7.10.6 see #25751 - SSL subjectAltNames is not
 supported by 7.10.6 or earlier.

 WP_HTTP currently
 * Tries to use cURL first, Streams second
 * Only uses cURL for HTTPS if it ''claims'' to support HTTPS, we have no
 check in place to verify it can make a SSL connection
 * Doesn't fall over to a 2nd transport in event of error
 * Doesn't disable a transport in event of continued troubles

 I think there are a few things we can possibly do here:
 1. Disable a transport after multiple failed requests (See #16870)
 1. Add a parameter to try the other transport if available (and if that
 works, lock it to using that transport for the rest of that page load so
 we have less timeouts)

 !#1 would be a win for most users who have issues with cURL and DNS lookup
 failures, and any sites which have plugins that make a bunch of external
 HTTP calls.
 !#1 can also have false positives, for example, oEmbed calls to sites that
 are down could disable a sites ability to make requests, so we might want
 to limit it to *.wordpress.org requests as a way of saying "This site
 SHOULD be working"
 !#1 and !#2 might not play nicely though if !#2 was implemented, we might
 not have enough data to reliably trigger the blocking.

 !#2 would mean that we could use that for WordPress.org API requests,
 which would cause any HTTPS connections that fail via cURL succeeding
 through Streams (if supported).
 Since it's possible for a site to have cURL with broken SSL, and no
 OpenSSL installed, we would still need extra code to handle a HTTPS ->
 HTTP fallback in the update API's though (perhaps this would be good as a
 wrapper "connect over SSL if possible, else, use HTTP)

 We also probably don't want to re-try every HTTP request, which makes
 adding a parameter for it probably a better idea, the other option would
 be to simply detect certain cURL/streams error codes and re-try those.

 potentially we wouldn't want it to re-try zip package downloads too

--

Comment (by dd32):

 See Also: #26010

--
Ticket URL: <http://core.trac.wordpress.org/ticket/25738#comment:5>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software