WP_HTTP uses transports that incorrectly claim to support a request
|Reported by:||dd32||Owned by:|
|Component:||HTTP API||Keywords:||needs-patch dev-feedback|
Description (last modified by dd32)
Following on from #25716
We now have 2 HTTP transports, and 2 types of requests that we need to support:
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)
- 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:
- Disable a transport after multiple failed requests (See #16870)
- 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
Change History (18)
3 years ago
- Cc gennady@… added