Opened 14 years ago
Closed 13 years ago
#16978 closed defect (bug) (fixed)
Move the PHP HTTP Extension priority down the list for WP_HTTP
Reported by: | dd32 | Owned by: | |
---|---|---|---|
Milestone: | 3.2 | Priority: | normal |
Severity: | normal | Version: | 3.1 |
Component: | HTTP API | Keywords: | dev-feedback 2nd-opinion |
Focuses: | Cc: |
Description
Currently the PHP HTTP Extension is receiving the top priority slot for which transport HTTP requests go out through.
However, the extension is also the least tested, and also now uses the most memory when using Stream-to-file functionality for downloads.
In addition to this, in most cases the HTTP Extension appears to be a simple cURL wrapper, falling back to fopen() & friends for certain functionality. This leads to cURL being available in most cases, and given we can control cURL's options easier than we can the HTTP extension, it makes sense to bump that up to position 1.
In addition to this, the extension's ::test() method does not seem to cover all bases, leading back to the least-tested. For example, it requires the .ini setting 'allow_url_fopen' to be enabled for certain functionalities (HEAD Requests & Certain redirection cases) - For example, it fails with this on 4 of the 14 unit tests:
file_get_contents(): http:// wrapper is disabled in the server configuration by allow_url_fopen=0
So reasons:
- Least Tested (historically at least, It may become more tested as PHP5 is more common)
- Requires more memory for Stream-to-file (Can use an extra ~9MB for downloading a 3MB file compared to using Curl, Streams, or fsockopen)
- Can't make non-blocking requests (In this case, it's already the last transport that is tried)
Attachments (2)
Change History (9)
#2
@
14 years ago
- Milestone changed from Future Release to 3.2
I mainly created the ticket for tracking purposes, and to seek more public opinion on it
However, I do feel that this should be changed in 3.2.
#3
@
14 years ago
Patch introduces a filter. That's an idea that originally grew out of some comments in other tickets about testing. I think this makes much sense to deal with differently configured systems and should benefit testing and is somewhat in scope of this ticket so I added it here.
In [17550] code was moved from one function into a new one while refactoring variable names. One variable related to the default list of transports ($r['blocking']
) still has the old name and is never set then.
Next to that, thanks to the nature of a private function the 'blocking' argument is always set.
#5
@
14 years ago
I think we are rapidly coming to the conclusion that the HTTP Extension is largely useless, very limited and should likely be removed. As it stands the HTTP Extension is largely a wrapper for cURL.
http://us3.php.net/manual/en/http.requirements.php :
The extension must be built with libcurl support to enable request functionality
I can see the potential of the cURL module not being installed, but the HTTP Extension being installed, however the environment would also have to not have fsockopen or streams available before we hit the HTTP extension with the reordering.
In addition, the HTTP Extension is a pecl module and not included with PHP whereas cURL, fsockopen and streams are included with PHP.
By just having this transport we are complicating things, especially when you consider that the HTTP Extension contains far less functionality than our other transports, so we are only hurting people in the end by keeping it around.
It seems the vote from myself, dd32 and scribu is to proceed with removing this transport.
#7
@
13 years ago
- Resolution set to fixed
- Status changed from new to closed
(In [17659]) Remove support for the PHP HTTP Extension from WP_HTTP. The PHP HTTP Extension is a wrapper around libcurl and fopen() providing limited configuration and is supported on a minority of servers due to its non-default inclusion. Props sivel. Fixes @16978
This for 3.2? I think most of us agree that the order should change.