WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 13 months ago

#37705 new defect (bug)

Remove unnecessary parts of WP_HTTP which remain

Reported by: dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: HTTP API Keywords: has-patch needs-refresh
Focuses: Cc:

Description

After Requests was merged in WordPress 4.6, WP_HTTP now wraps it with a few of the previous classes remaining for compatibility.

Some of these have significant code left in them which is no longer used, and we've still got the entire cURL and Streams WP_HTTP request classes in core.

We should be able to remove a large percentage of these without too much issue.

There's a chance that some developers have been using these classes directly, especially as a number of items were marked as public (due to how it was developed).

I'd like to take the hardline approach and rip everything not needed out early, and restore compatibility shims where needed if it turns out developers were using things directly (incorrectly).

Attachments (3)

37705.diff (50.4 KB) - added by dd32 15 months ago.
37705.2.diff (69.5 KB) - added by dd32 15 months ago.
37705.txt (42.4 KB) - added by jorbin 15 months ago.

Download all attachments as: .zip

Change History (13)

@dd32
15 months ago

@dd32
15 months ago

#1 @dd32
15 months ago

@rmccue I'm curious if you have any objections / reasoning for any of these pieces to remain functional, although I'd prefer to stub them in instead.

#2 @swissspidy
15 months ago

Note that removed files would need to be added to $_old_files global.

@jorbin
15 months ago

#3 @jorbin
15 months ago

ack-grep --php --ignore-dir=Aws --ignore-dir=Guzzle --ignore-dir=guzzle --ignore-dir=aws "_get_first_available_transport|processResponse|buildCookieHeader|chunkTransferDecode|handle_redirects|is_ip_address"

Returned a bunch of results (attached above). Digging through, here is what I found ( besides a number of people that include copies of WP_Http :( )

WP_Http::is_ip_address
WP_Http::is_ip_address
WP_Http::buildCookieHeader
$wp_http->_get_first_available_transport <-- @dd32
WP_Http::is_ip_address
WP_Http::buildCookieHeader
$http->_get_first_available_transport
WP_Http::_get_first_available_transport
WP_Http::is_ip_address
WP_Http::buildCookieHeader and WP_HTTP::handle_redirects
_get_first_available_transport
_get_first_available_transport
WP_Http::is_ip_address
WP_Http::_get_first_available_transport (in one other file as well)
$http_obj->_get_first_available_transport

#4 @dd32
15 months ago

Judging by that list, I think we can..

  • Keep WP_Http::is_ip_address() as a util
  • Stub _get_first_available_transport() / $wp_http->_get_first_available_transport and return null
  • Possibly keep WP_Http::buildCookieHeader or stub it out and return null.

Looking at a few uses where WP_Http::buildCookieHeader was used, the HTTP api has been extended in such a way that it's broken under 4.6 or they're replacing large chunks of it anyway.

#5 @jorbin
15 months ago

I think stubbing WP_Http::buildCookieHeader makes sense. Agree with the other two pieces. These should get a dev note when this goes in.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


14 months ago

#7 @desrosj
14 months ago

  • Keywords needs-refresh added

This needs a refresh based on @jorbin and @dd32's feedback above if it is going to land in 4.7.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


13 months ago

This ticket was mentioned in Slack in #core by stevenkword. View the logs.


13 months ago

#10 @stevenkword
13 months ago

  • Milestone changed from 4.7 to Future Release

Still a bit of work needs to be done here and after looking at @jorbin's report, I'm thinking this one really needs to be done early in a release cycle. Moving to 'future release'.

Note: See TracTickets for help on using tickets.