#16888 closed enhancement (wontfix)
WP HTTP Curl transport is only handling some but not all redirect status codes.
Reported by: | hakre | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.1 |
Component: | HTTP API | Keywords: | has-patch |
Focuses: | Cc: |
Description
That's violating the specs by namely processing 301 and 302 only:
HTTP status codes are extensible. HTTP applications are not required
to understand the meaning of all registered status codes, though such
understanding is obviously desirable. However, applications MUST
understand the class of any status code, as indicated by the first
digit, and treat any unrecognized response as being equivalent to the
x00 status code of that class, with the exception that an
unrecognized response MUST NOT be cached.
from: 6.1.1 Status Code and Reason Phrase
For 3xx responses, the location SHOULD indicate the
server's preferred URI for automatic redirection to the resource. The
field value consists of a single absolute URI.
from: 14.30 Location
Attachments (1)
Change History (15)
#2
@
14 years ago
- Summary changed from WP HTTP Layer with Curl is only handling some but not all redirects. to WP HTTP Curl transport is only handling some but not all redirect status codes.
#3
follow-up:
↓ 5
@
14 years ago
Not all 3xx codes indicate a redirection. For example 304 is a not modified and has nothing to do with redirection. Returning a WP_Error on a 304 would be incorrect behavior, as someone may be expecting to get additional information when a 304 is received.
#4
follow-up:
↓ 6
@
14 years ago
- Component changed from General to HTTP
What other status codes are used with a Redirect (other than 301/302)?
One option here is to redirect on any Location header, but in the case of the cURL library, IIRC, Redirections are handled internally by cURL, the modified code in the patch is mearly for error reporting purposes (If the final page is a 301/302, cURL gave up redirecting and returned a redirect response, If it falls past that, it should hit another error condition and return the raw error)
#5
in reply to:
↑ 3
@
14 years ago
Replying to sivel:
Not all 3xx codes indicate a redirection. For example 304 is a not modified and has nothing to do with redirection.
The RFC 2616 speaks another language (6.1.1 Status Code and Reason Phrase):
The first digit of the Status-Code defines the class of response. [...] - 3xx: Redirection - Further action must be taken in order to complete the request
And further on in specific of the 304 status (which is specified):
If the client has performed a conditional GET request and access is allowed, but the document has not been modified, the server SHOULD respond with this status code. The 304 response MUST NOT contain a message-body, and thus is always terminated by the first empty line after the header fields.
Returning a WP_Error on a 304 would be incorrect behavior, as someone may be expecting to get additional information when a 304 is received.
If redirects are limited and the request results in a going-over-that-limit, then I would expect the function to return an error. Per all specification documents related to the context of HTTP the classification of the 3xx codes as being all redirects per RFC make me think so.
If you can provide any document with some authority on the subject specifying the 304 status not being classified as redirect, please link it, I would be interested.
If you're setting the redirection parameter to control a follow-location-header behavior, an additional parameter might be more flexible to actually control something (and prevent error return values): #16855 (, #16889).
#6
in reply to:
↑ 4
;
follow-up:
↓ 7
@
14 years ago
Replying to dd32:
What other status codes are used with a Redirect (other than 301/302)?
All 3xx codes are classified as redirect per RFC (see my last comment)
One option here is to redirect on any Location header
I will make my mind about this, I'll check the location header for this.
, but in the case of the cURL library, IIRC, Redirections are handled internally by cURL, the modified code in the patch is mearly for error reporting purposes (If the final page is a 301/302, cURL gave up redirecting and returned a redirect response, If it falls past that, it should hit another error condition and return the raw error)
I have the feeling that cUrl is well developed (to say at least) and it's worth to test with it and look into it's detailed documentation to get a well-thought interface to http.
I'll review the 3xx codes in conjunction with location response headers as well.
#7
in reply to:
↑ 6
;
follow-up:
↓ 10
@
14 years ago
Replying to hakre:
I'll review the 3xx codes in conjunction with location response headers as well.
Done: HTTP Redirect Codes (3xx) and the Location Field - You'll find an overview-table for the use I could find specified with details linked.
#10
in reply to:
↑ 7
;
follow-up:
↓ 11
@
14 years ago
- Milestone changed from Awaiting Review to Future Release
- Type changed from defect (bug) to enhancement
Replying to hakre:
Looks like you've inverted all your logic there. By your logic we shouldn't redirect in every case we currently do.
Summarizing here for the purpose of keeping things in one place: http://tools.ietf.org/html/rfc2616
10.3.1 300 Multiple Choices The requested resource corresponds to any one of a set of representations, each with its own specific location, and agent- driven negotiation information. If the server has a preferred choice of representation, it SHOULD include the specific URI for that representation in the Location field; user agents MAY use the Location field value for automatic redirection. 10.3.4 303 See Other The response to the request can be found under a different URI and SHOULD be retrieved using a GET method on that resource. 10.3.6 305 Use Proxy The requested resource MUST be accessed through the proxy given by the Location field. The Location field gives the URI of the proxy. The recipient is expected to repeat this single request via the proxy. 10.3.8 307 Temporary Redirect The requested resource resides temporarily under a different URI. The temporary URI SHOULD be given by the Location field in the response. If the 307 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued.
Selectively copied segments which is of use to this ticket, both 303 and 307 specify that many HTTP 1.0 clients do not understand them, and a 302 is recommended for interoperability. 300 can be treated like a 301/302 (As we do not do user-agent negotiation, if the servers preferred method is there, take it)
I'm throwing this into Future Release as a enhancement, The WP_HTTP class currently supports the most common redirection methods in use on the web today (I'd challenge anyone to find anyone actively using them..)
#11
in reply to:
↑ 10
@
14 years ago
Replying to dd32:
I have no problem discussing the RFC 2626, but let's stick a bit on topic and not head that far away from this ticket.
It's as much not about removing redirects as it is about performing additional automatic follow-up-requests because of location headers.
The article linked on my blog was only a write-up to summarize the use of the location header in the 3xx class, it's not entirely related for this ticket here so I put it apart.
I opened this ticket to classify any status in the 3xx redirect class as being a redirect so they get counted for the error condition.
#12
follow-up:
↓ 13
@
14 years ago
on the back of #16889, it might be best to consider handling a location header being present instead of handling it by response code.
3xx instead of 301 and 302 only.