Make WordPress Core

Opened 22 months ago

Last modified 19 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:


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 22 months ago.
37705.2.diff (69.5 KB) - added by dd32 22 months ago.
37705.txt (42.4 KB) - added by jorbin 21 months ago.

Download all attachments as: .zip

Change History (13)

22 months ago

22 months ago

#1 @dd32
22 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
22 months ago

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

21 months ago

#3 @jorbin
21 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
21 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
21 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.

20 months ago

#7 @desrosj
20 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.

20 months ago

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

19 months ago

#10 @stevenkword
19 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.