[wp-trac] [WordPress Trac] #25738: WP_HTTP uses transports that incorrectly claim to support a request
WordPress Trac
noreply at wordpress.org
Fri Nov 15 03:32:31 UTC 2013
#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: |
--------------------------+------------------
Description changed by dd32:
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 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
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 7.10.6 w/ any SSL||✓||~||✓||~||
||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
--
--
Ticket URL: <http://core.trac.wordpress.org/ticket/25738#comment:6>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software
More information about the wp-trac
mailing list