WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 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)

16978.patch (1.2 KB) - added by hakre 3 years ago.
Make it filterabled (and bugfix)
16978.2.diff (8.2 KB) - added by sivel 3 years ago.
Goodbye HTTP Extension

Download all attachments as: .zip

Change History (9)

comment:1 nacin3 years ago

This for 3.2? I think most of us agree that the order should change.

comment:2 dd323 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.

hakre3 years ago

Make it filterabled (and bugfix)

comment:3 hakre3 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.

Last edited 3 years ago by hakre (previous) (diff)

comment:4 dd323 years ago

(In [17566]) Use the correct variable name; Simpler static initialisation. Props hakre. See #11613 & #16978

comment:5 sivel3 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.

sivel3 years ago

Goodbye HTTP Extension

comment:6 sivel3 years ago

New patch rips out the HTTP Extension.

comment:7 dd323 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

Note: See TracTickets for help on using tickets.