Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 13 years ago

#11557 closed defect (bug) (fixed)

HTTP Streams transport doesnt abide max redirection

Reported by: dd32's profile dd32 Owned by: dd32's profile dd32
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9
Component: HTTP API Keywords: has-patch needs-testing
Focuses: Cc:

Description

The Streams HTTP transport doesnt abide by the maximum redirections properly.

It appears, that the redirection limit is placed on a per-access count, rather than a per-redirection count. So the first request is counted in the number of redirections.

If you request a page with 'redirection = 1' on a document which redirects exactly once, It fails with a too-many redirects error.

My suggestion for fixing this is to simply increase the redirections by one on the stream wrapper creation..

Attachments (1)

11557.diff (525 bytes) - added by dd32 14 years ago.

Download all attachments as: .zip

Change History (9)

@dd32
14 years ago

#1 @dd32
14 years ago

Can someone confirm this for me on another server?

Tested on PHP Version 5.3.0 Windows NT DD32 6.1 build 7600 ((null)) i586

#2 @hakre
14 years ago

I think that's properly compared to the documented behaviour:

HTTP context options:

max_redirects integer

The max number of redirects to follow. Value 1 or less means that no redirects are followed.

Defaults to 20.

It (the 1) could be read as: The first redirect is max, so it won't be done.

Is there a documentation of the request() $args array members available? the parameter is named 'redirection' and the default value is 5 (at least in WP_Http_Streams). I can not find any more specs about that parameter next to that it is in sync with the max_redirects documented on the php page.

what about setting the default value to 20?

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

#3 follow-up: @dd32
14 years ago

It (the 1) could be read as: The first redirect is max, so it won't be done.

Another way to look at it, Like in my OP, Is that the max_redirections is actually The maximum number of requests it will make. So 1 request, and result is a 30x, Has hit the maximum limit count.

Is there a documentation of the request() $args array members available?

Its in the file. If its not there, Then probably not. Look at WP_HTTP::request() perhaps. But reading the code is the best method.

what about setting the default value to 20?

Thats completely irrelivant to this ticket.

As a test, Try requesting http://tools.dd32.id.au/redirect/?rt=5 which makes 5 redirects with the various transports. All of them except streams will suceed in requesting the document. Increase redirection to 6 and all will suceed. Decrease redirection to 4 and all will fail.

You can use my Core Control plugin with the HTTP Access manager to disable the transports in their preffered orders.

#4 @jacobsantos
14 years ago

Um, how about we just do default + 1 as a hacked fix.

#5 @jacobsantos
14 years ago

Oh wait, the patch already does that.

#6 @dd32
14 years ago

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

(In [12745]) Bring WP_Http_Streams maximum redirection into line with the rest of the transports. Fixes #11557

#7 in reply to: ↑ 3 @hakre
13 years ago

Replying to dd32:

It (the 1) could be read as: The first redirect is max, so it won't be done.

Another way to look at it, Like in my OP, Is that the max_redirections is actually The maximum number of requests it will make. So 1 request, and result is a 30x, Has hit the maximum limit count.

But how would you explain the 0 value then? To do no request at all? Additionally it's called "Redirect" and not "Request" in that switches name, so seeing redirects as request looks highly subjective to me.

Is there a documentation of the request() $args array members available?

Its in the file. If its not there, Then probably not. Look at WP_HTTP::request() perhaps. But reading the code is the best method.

Right now on top of the file in the base class there is much more text, but it does not contains much more info.

what about setting the default value to 20?

Thats completely irrelivant to this ticket.

Right, see Related: #16887

As a test, Try requesting http://tools.dd32.id.au/redirect/?rt=5 which makes 5 redirects with the various transports. All of them except streams will suceed in requesting the document. Increase redirection to 6 and all will suceed. Decrease redirection to 4 and all will fail.

Works, but you should filter the input to not-too-large, positive integer values.

#8 @hakre
13 years ago

ignore, mixed with curl.

Last edited 13 years ago by hakre (previous) (diff)
Note: See TracTickets for help on using tickets.