WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 3 years ago

#25747 new defect (bug)

The `http_api_debug` hook isn't called for all HTTP requests

Reported by: johnbillion Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.2
Component: HTTP API Keywords: needs-testing needs-unit-tests has-patch
Focuses: Cc:

Description (last modified by johnbillion)

In WP_Http::_dispatch_request(), the http_api_debug action isn't called if there are no available transports for the request. This means we miss a situation that is ripe for debugging.

Edit: There's more. See my comments below.

Attachments (2)

25747.diff (1.1 KB) - added by johnbillion 5 years ago.
25747.2.diff (4.7 KB) - added by johnbillion 3 years ago.

Download all attachments as: .zip

Change History (7)

@johnbillion
5 years ago

#1 @johnbillion
5 years ago

25747.diff (best viewed without whitespace changes) alters _dispatch_request() so it doesn't return the WP_Error immediately. It then gets passed to the http_api_debug hook as expected.

Anything that's hooking into this hook will need to ensure it handles boolean false as the $class parameter, as well as a string.

#2 @johnbillion
5 years ago

  • Keywords has-patch added
  • Version set to 3.2

#3 @johnbillion
5 years ago

  • Description modified (diff)
  • Keywords needs-patch added; has-patch removed
  • Summary changed from The `http_api_debug` hook isn't called when there are no available transports to The `http_api_debug` hook isn't called for all HTTP requests

There are actually several places where an HTTP request's return value doesn't trigger the http_api_debug action. None of the following responses trigger it:

  • Anything short-circuiting requests with the pre_http_request filter (here).
  • Invalid URLs (here).
  • Blocked requests (here).
  • Unwritable directories when streaming (here).

We might need a little refactoring here, or a helper method to trigger this action.

#4 @wonderboymusic
3 years ago

  • Milestone changed from Awaiting Review to Future Release

Bump

@johnbillion
3 years ago

#5 @johnbillion
3 years ago

  • Keywords needs-testing needs-unit-tests has-patch added; needs-patch removed

25747.2.diff is a partial patch that I've had sitting around for ages. Probably needs some more work. And tests.

Note: See TracTickets for help on using tickets.