WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#10231 closed defect (bug) (fixed)

missing hook in WP_HTTP

Reported by: Denis-de-Bernardy Owned by: dd32
Milestone: 2.8.1 Priority: normal
Severity: normal Version: 2.8
Component: HTTP API Keywords: has-patch tested commit
Focuses: Cc:

Description

see attached patch.

the missing hook allows to stop a nascent request (e.g. due to a coming 403 related to an expired membership) before it occurs.

Attachments (1)

10231.diff (773 bytes) - added by Denis-de-Bernardy 6 years ago.

Download all attachments as: .zip

Change History (11)

@Denis-de-Bernardy6 years ago

comment:1 follow-up: @dd326 years ago

What is the point of the added WP_Error check?

Is there a reason to advertise the possibility of returning a non-array via it?

comment:2 @azaozz6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11652]) Add missing localization in WP_HTTP, props Denis-de-Bernardy, fixes #10231 for 2.8

comment:3 @azaozz6 years ago

(In [11653]) Add missing localization in WP_HTTP, props Denis-de-Bernardy, fixes #10231 for trunk

comment:4 in reply to: ↑ 1 @Denis-de-Bernardy6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to dd32:

What is the point of the added WP_Error check?

Is there a reason to advertise the possibility of returning a non-array via it?

If you know, based on transients, that the url will return junk (a 302 to a login form, for instance), there currently isn't any means to make WP stop then and there.

comment:5 @Denis-de-Bernardy6 years ago

  • Summary changed from missing localization + missing hook in WP_HTTP to missing hook in WP_HTTP

comment:6 @dd326 years ago

there currently isn't any means to make WP stop then and there.

No.. No there isnt.. Remind me again, Whats the point of that? Why would something be requesting it if it knew it was a dud url? Put your check in your code.. not in the API.

comment:7 @Denis-de-Bernardy6 years ago

Duh, I certainly would, if such a hook existed in the first place. ;-)

The http request in question is for a core update...

comment:8 @Denis-de-Bernardy6 years ago

It's like... I would offer a patch that places them where I'd like to see them, but it makes a lot more sense to add a generic one right in the http request args. It makes things a *lot* more generic.

comment:9 @Denis-de-Bernardy6 years ago

Some IRC logs:

jacobsantos
Dude, HTTP has such a large overhead already.
3:23 AM
Which is partly way I didn't do it in the first place.
3:24 AM
Well, given the ratio of plugin API verses HTTP request, there really isn't a comparsion.
3:24 AM
Be like 1:10000000000 or something.
3:24 AM
Well, localization does have a higher overhead than the Plugin API.
3:25 AM ddebernardy
right
3:25 AM
and this hook actually improves the overhead
3:25 AM
since a plugin can then prevent the HTTP request before it's even triggered
3:26 AM
I mean, heck, consider it for a moment
3:26 AM
http request to a protected zip file
3:26 AM
which redirects, using a 302, to a login form
3:26 AM
WP diligently downloads the form, and tries to unzip it
3:26 AM
sure I could manage this on my server
3:26 AM
and check the user agent
3:26 AM
but it's ridiculous
3:27 AM
one hook in there, and users get the correct error, i.e. "time to renew your membership"
3:27 AM
without a request in the first place
3:27 AM
whereas, without it, they get an error (bad zip)
3:27 AM
and I get a support request

comment:10 @Denis-de-Bernardy6 years ago

  • Milestone changed from 2.8.2 to 2.8.1
  • Resolution set to fixed
  • Status changed from reopened to closed

see #10413

Note: See TracTickets for help on using tickets.