OpenEuphoria

Euphoria Ticket #781: Implement http redirect support

• Reported by jimcbrown Sep 13, 2012
• 44 comments

Details

Type:	Bug Report	Severity:	Normal	Category:	Library Routine
Assigned To:	SDPringle	Status:	Fixed	Reported Release:
Fixed in SVN #:		View VCS:	none	Milestone:	4.0.6

1. Comment by SDPringle Jan 14, 2013

This is in the spec. It is a bug not a new feature.

2. Comment by SDPringle Jan 14, 2013

fixing some of the tags

3. Comment by jimcbrown Jan 14, 2013

Calling it a bug report is fine with me.

4. Comment by SDPringle Jan 14, 2013

See: hg:euphoria/rev/8b5cb3809cd6

changeset: 5924:8b5cb3809cd6 branch: 4.0 parent: 5911:01b6088812b8 user: Shawn Pringle <shawn.pringle@gmail.com> date: Mon Jan 14 12:36:38 2013 -0300 files: docs/release/4.0.6.txt include/std/net/http.e description:

• http_get and http_post now follow redirects
• fixes ticket 781

5. Comment by jimcbrown Jan 14, 2013

Um, is the code for HTTP POST redirects handling it correctly?

http://openeuphoria.org/ticket/834.wc#3243

6. Comment by CoJaBo2 Jan 14, 2013

This introduces two bugs;

• The status code is not checked; the location header should only be obeyed when the status code indicates a redirect.
• The code for HTTP POST is completely wrong, as noted in my other post.

7. Comment by mattlewis Jan 14, 2013

Removing the "Fixed" status in light of the comments.

8. Comment by jimcbrown Jan 14, 2013

See: hg:euphoria/rev/b7e563a517fd

changeset: 5929:b7e563a517fd branch: 4.0 tag: tip parent: 5924:8b5cb3809cd6 user: Jim C Brown date: Mon Jan 14 19:56:02 2013 -0500 files: include/std/net/http.e description:

• Fixes ticket:781
• Check HTTP Status Code before following the Location header
• For HTTP Post, do either a GET or a POST depending on the Status Code

9. Comment by jimcbrown Jan 14, 2013

See: hg:euphoria/rev/57752504a98b

changeset: 5930:57752504a98b tag: tip parent: 5928:bea043061178 user: Jim C Brown date: Mon Jan 14 20:00:04 2013 -0500 files: include/std/net/http.e description:

• Fixes ticket:781
• Check HTTP Status Code before following the Location header
• For HTTP Post, do either a GET or a POST depending on the Status Code

10. Comment by jimcbrown Jan 14, 2013

Updated code (now in two branches!) in an attempt to address these issues - but someone with a better understanding of the RFC needs to double check it.

Any recommendations for what the hardcoded maximum should be (or if we shouldn't have one at all) ? Any reason to change the default away from 10? Should the default for follow_redirects be 0 ?

11. Comment by CoJaBo2 Jan 14, 2013

"Any reason to change the default away from 10?"
I'd say 10, but 20 is also used (IE uses the former, Firefox and Webkit use the latter). The usual case for the parameter being changed would be to decrease it. Note that in the overwhelming majority of cases, there will be no more than 2 chained redirects; very poorly designed webapps may have up to 4.

"Any recommendations for what the hardcoded maximum should be (or if we shouldn't have one at all)?"
The hardcode limit should probably be 20, if implemented. There is no reason any functioning server would emit a redirect chain longer than this, as no existing browser or HTTP library would be able to load it.
The reason for having a limit is to prevent "accidental" Denial of Service attacks against servers by users setting the redirect limit to infinite "to make the error go away" (yes, I have seen this happen; the issue took literally months to track down).
If, however, we prefer to not do stuff like this to prevent the user from shooting themselves in the foot, this could be left out (however, there is no benefit to doing so).

"Should the default for follow_redirects be 0 ? "
It would make little sense to default to not following redirects, as following them is whats going to be needed in 95% of cases. The remaining times would be exceptions, when the programmer knows that following the redirect is not necessary- apps that prompt the user to confirm following a redirect, apps that debug HTTP/status information (for the latter case, Eu really ought to support HEAD as well..).

12. Comment by useless_ Jan 14, 2013

Years ago the official dynamically built Cyc site had badly constructed their redirects, the huge ontology directory linked to itself a few directories deeper. The server would happily serve up exactly the same files in new dirs right up to the max limit of HTTP path length.

I have also seen and heard of a site being hacked, and a redirect installed to an exact copy of the hacked site, with an 0 for a O in the url. This was also done years ago to pictures with html in them, which IE would happily execute and download malware.

Nope, i do not want to be responsible for a Eu program downloading malware.

It's so simple, i can't figure why you are making it difficult. You send a GET req to a server, it replies. You process the reply, you decide to stop GETting, or follow a redirect, or process the page otherwise. You don't take that control away from the programmer and blindly follow the redirect.

useless

13. Comment by CoJaBo2 Jan 14, 2013

"Years ago the official dynamically built Cyc site had badly constructed their redirects, the huge ontology directory linked to itself a few directories deeper. The server would happily serve up exactly the same files in new dirs right up to the max limit of HTTP path length. "
Like this? lol

"I have also seen and heard of a site being hacked, and a redirect installed to an exact copy of the hacked site, with an 0 for a O in the url. This was also done years ago to pictures with html in them, which IE would happily execute and download malware. "
In all cases I've ever seen, the malware was either hosted directly on the compromised site, or injected using an iframe and/or Javascript. Hijacking a site with an HTTP redirect would be feasible, but attackers don't do that because there are easier and more effective ways. It is no argument to not following a redirect.

"t's so simple, i can't figure why you are making it difficult. You send a GET req to a server, it replies. You process the reply, you decide to stop GETting, or follow a redirect, or process the page otherwise. You don't take that control away from the programmer and blindly follow the redirect. "
If you know you need to do this, its simple- pass 0 to that parameter. The majority of cases, the programmer is going to expect the library to just get the page, redirect or no redirect.

14. Comment by SDPringle Jan 16, 2013

See: hg:euphoria/rev/232d58b4ac75

changeset: 5935:232d58b4ac75 branch: 4.0 tag: tip parent: 5929:b7e563a517fd user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Jan 16 10:24:48 2013 -0300 files: include/std/net/http.e description:

• removed extra '(' in include/std/net/http.e
• combined several if statements into one.
• ticket 781

15. Comment by jimcbrown Jan 16, 2013

It looks like the only thing left to do is add a hard limit of 20 for the follow_redirect option.

16. Comment by SDPringle Jan 17, 2013

we should also prevent negative numbers from being passed for these parameters that are clearly for non-negative numbers. In std/get.e, there is a natural type that would work nicely.

17. Comment by SDPringle Jan 17, 2013

See: hg:euphoria/rev/6869e3185331

changeset: 5945:6869e3185331 branch: 4.0 parent: 5935:232d58b4ac75 user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Jan 16 11:07:40 2013 -0300 files: include/std/net/http.e description:

• removed a call to find and a variable in include/std/net/http.e
• ticket 781

18. Comment by SDPringle Jan 17, 2013

See: hg:euphoria/rev/20f603ba68be

changeset: 5946:20f603ba68be branch: 4.0 user: Shawn Pringle <shawn.pringle@gmail.com> date: Wed Jan 16 11:10:42 2013 -0300 files: tests/t_net_http.e description:

• made tests more descriptive
• ticket 781

19. Comment by SDPringle Jan 19, 2013

According to the RFC 2616, when a response comes from a request that is neither GET nor HEAD; the response codes 301, 302 and 307 should not cause a user-agent (here the library) to automatically redirect.

See: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.1

but the implementation explicitly does this in the http_post.

20. Comment by SDPringle Jan 19, 2013

It see that 308 is supposed to be like 307 but 'Permanent' like 301. We shouldn't redirect in the case of http_post anything but code 303.

21. Comment by SDPringle Jan 19, 2013

See: hg:euphoria/rev/85886b58c80d

changeset: 5961:85886b58c80d branch: 4.0 user: Shawn Pringle <shawn.pringle@gmail.com> date: Sat Jan 19 19:48:41 2013 -0300 files: include/std/net/http.e description:

• set to only redirect http_post when response 303
• ticket 781

22. Comment by SDPringle Jan 19, 2013

See: hg:euphoria/rev/cf49888f2883

changeset: 5962:cf49888f2883 branch: 4.0 tag: tip user: Shawn Pringle <shawn.pringle@gmail.com> date: Sat Jan 19 19:59:43 2013 -0300 files: include/std/net/http.e description:

• set to only redirect http_post when response code is 303
• added natural type to prevent misuse of negative numbers
• improved testing order to help performance
• ticket 781

23. Comment by jimcbrown Jan 19, 2013

was working off of http://en.wikipedia.org/wiki/List_of_HTTP_status_codes

So basically, you are right - but if I understand it correctly, historically 301 and 302 did automatic redirects when the redirect caused the HTTP method to change, even though this was against the RFC. RFCs are generally updated to describe widespread and mainstream behavior that appears "in the wild" even if this behavior contradicts the earlier versions of those RFCs.

My understanding is that the request method is not allowed to change for 307 or 308 - that is, if it starts out as HTTP POST, and we get a redirect with reson 307 or 308, then the new URL must also be HTTP POST. Changing to HTTP GET is simply not allowed at this point. I'm not sure how to handle the case when this rule is violated, though...

24. Comment by jimcbrown Jan 19, 2013

You're only updating the 4.0 branch? What about default?

Anyways, to quote the RFC

Should we really act differently from most existing user agent implementations?

25. Comment by jimcbrown Jan 19, 2013

Hmm. I'm not clear on how we're suppose to tell, from the redirect request and headers, when we get a (for example) 303 back, does the web server want use to use an HTTP GET or an HTTP POST or possibly some other method to follow that URL. Is there another header with that information?

26. Comment by SDPringle Jan 19, 2013

The RFC clearly state that a user agent "should" use GET after receiving a 303.

27. Comment by CoJaBo2 Jan 20, 2013

302 must be treated as if it were 303; this was an omission in the spec, which had not specified a way to do PRG forms submissions. Most apps that do PRG still use the older code, because 303 didn't exist back then, and tutorials/libraries still fail to be updated (this means that 302 is ambiguous, will likely be deprecated, and should no longer be generated by new applications).

As for 301 and 307 (and 308, which doesn't appear to yet be standardized), I've never seen either used, and they do not appear to work properly in most browsers (a 301 generally only happens when the server is misconfigured/broken); when the method is POST, it is probably best to either A) not follow them ever or B) follow them only when a (positive) redirect_limit is explicitly specified when calling the function.

28. Comment by jimcbrown Jan 20, 2013

We can't tell if value in follow_redirects was the defaulted parameter or if the user manually specified 10. So we have no way to know if the user wanted to follow 301 or not (unless you are suggested that we overload the parameter and use negative redirects to mean "follow the absolute value of this value but only do so for 302 and 303" and positive redirects to mean "follow the value of this value for all 3xx types" or something).

29. Comment by jimcbrown Jan 20, 2013

Do you have any information on how e.g. Firefox handles a 301, 307, or 308 ?

30. Comment by CoJaBo2 Jan 20, 2013

Redirecting on POST-
301: Firefox converts to GET; spec requirements are ambiguous; this generally indicates a server configuration problem (it won't work in any case)
302: Firefox converts to GET; spec requirements are ambiguous, but consensus is that it should be treated as 303
303: Firefox converts to GET; this matches spec
307: Firefox falls back to displaying the response body (i.e., not implemented); spec requires maintaining as POST "after user confirmation"
308: Firefox falls back to displaying the response body (i.e., not implemented); not in current version of spec, proposed to require maintaining as POST "after user confirmation"

Keep forgetting Eu doesn't have nulls... I think a cleaner way would be to have the default (for http_post only) be -1, meaning "the user doesn't care/didn't specify anything", and then just treat -1 as 20 for 302 and 303, and as 0 for 307 and 308 where the spec requires "user confirmation".
There is no apparent consensus on "correct" behavior for 301 on POST; treating it as 303 like Firefox does would cause breakage, and there are no browsers that treat it as 308; it might be best to just not follow it in any case. Alternatively, making 301 on POST a hard error would help with debugging POST issues on misconfigured servers.

31. Comment by jimcbrown Jan 20, 2013

Do you have a source for these Firefox results, or did you run a series of tests yourself?

When you say a 301 in reponse to a POST request is broken, how does it break? What exactly happens (with Firefox) in this case, when it does a GET request for the URL in the Location header and why is that breaking behavior?

32. Comment by CoJaBo2 Jan 20, 2013

I ran into this issue writing a web API on a misconfigured server; the tests are my own, and are for version 17 (18 has since been released, but there is nothing in the changelog to suggest the new codes were added).

A 301 on POST would generally happen if the server is configured to redirect all traffic (e.g., after a site move). If a POST is made to the old URL, most/all user-agents would convert it to a GET, which would cause it to completely break (as the POST data would be lost before reaching the forms processor). It would "work" if it was treated instead as a 308, but no current user-agents do this; moreover, doing so would just silently hide the problem that the (potentially large) POST request is now being submitted twice (and this would happen every single time, unless caching of permanent redirects is also implemented).

33. Comment by jimcbrown Jan 20, 2013

Which server was that? Was it a public one? If not, what software was it running?

Sorry for asking so many questions, I'm just trying to get enough information so that someone in the future (like Shawn) will have the option of reproducing your tests alongside running their own

Anyways, it seems like - with the new codes - the options are this:

302 - generally POST temporarily and automatically redirects to a GET - possibly POST temporarily and automatically redirects to a POST, but this behavior may be obsolete.

303 - POST temporarily and automatically redirects to a GET

307 - POST temporarily redirects to a POST, but not automatic due to the need to get user confirmation before re-POST'ing data.

301 - obsolete, do not use - possibly POST permanently and automatically redirects to a POST - possibly POST permanently and automatically redirects to a GET

308 - POST permanently redirects to a POST, but not automatic due to the need to get user confirmation before re-POST'ing data.

So it sounds like 302 was intended to act like 307, and 303 was tacked on to behave the way most user agents treat 302 - but getting them to change back to the RFC behavior of 302 was impractical, so 307 was tacked on afterwards as a way to implement the original behavior of 302 - this sort of makes 302 redundant now, since either 303 or 307 will get the job done - and do so unambiguously.

The common thread seems to be, automatic redirects from a POST to a GET are ok, but automatic redirects from a POST to a POST (which requires resending data) should require user confirmation. Any behavior that requires redirecting from a POST to a POST automatically (original 302 and original 301) is obsolete and should not be implemented - or, at least, not by default.

There's no non-breaking way to implement 301, but the best two ways are to default to a GET redirect or to default to not redirecting at all. Since Firefox does the former, and 308 makes the latter redundant behavior (in the same way that having 302 and 303 is redundant), my preference is to go with the former. Fortunately permanent redirects seem to be rare enough that this isn't going to be a common issue anyways.

What about the handling of those redirect codes from a GET to a GET? I guess 307 and 308 still should not be automatic in those cases?

34. Comment by CoJaBo2 Jan 21, 2013

The testing was done on NGINX, however the issue that led to the testing had come up on an Apache server.

Not sure I understand any of that. To clarify, this is what the lib should do-

On all methods-
If the redirect_limit is 0, don't follow the redirect in any case.

On GET and HEAD-
301/2/3/7/8: Follow the redirect, keeping the method the same.

On POST-
301: Never follow the redirect, or generate an error/warning. The latter would make debugging easier, as I've never witnessed this to not be an error condition.
302/303: Follow the redirect, but convert the request to GET.
307/308: Follow the redirect, keeping the method the same, but only if the user specified a redirect_limit.

The distinction between "permanent" and "temporary" redirects doesn't matter to the library, it only matters to the application using it; there should be a way for it to pass that information (including the final target of the redirect) to the caller (which may need such info).

301 redirects are common- almost every site out there has them in the config somewhere. For example, 301-redirecting www.site.web to site.web is a common paradigm. If this configuration reversed on a web API (that is, going from adding the www to removing it), users of the API would suddenly break, generally with a confusing error such as "invalid API key" or "invalid format". It is difficult to debug this scenario without access to the server logs, particularly if the change came without notice.

35. Comment by jimcbrown Jan 21, 2013

Ok, based on that information along with discussion on the dev list, here's what I've come up with

follow_redirects = 0, never follow a redirect, just return the headers

GET - follow redirects as get requests, unless explicitly told not to follow them

POST 303 - follow redirects, turn into a get request

POST 307/308 - follow redirects if and only if REDIRECT_ALL is passed in, otherwise never redirect, just return the headers

POST 301/302 - follow redirects, but turn into a get request if a enum other than REDIRECT_STRICT_RFC2616 is passed, but for REDIRECT_STRICT_RFC2616 we'll keep as a post request

I did think of

POST 301 - follow redirects but turn into a get request if and only if REDIRECT_ALL is passed in, however if REDIRECT_STRICT_RFC2616 is passed in, then still follow redirects and keep as a post request. If the default value, REDIRECT_BROWSER, is passed in, then don't follow the redirect, just return the headers.

But, seeing how most browsers will follow the request and turn it into a GET request, and how the existence of 308 (which keeps the POST method the same) implies that 301 should be turned into a GET request, I'm not sure I agree. Certainly, I disagree that 301 should cause a hard crash - if it's common then a lot of web sites will cause our HTTP library to hard crash, drawing ire from newbie users. We should have a reasonable default behavior here and a way for an end user to override and customize it per their own needs.

Anyways, I think we're all agreed on how to handle this, except for a 301 response to a POST request.

36. Comment by CoJaBo2 Jan 21, 2013

This seems way over-complex and confusing; it is clearly noted in the RFC that the majority of user-agents implement 302 as 303, due to the historical omission of a 303 code. I can see no reason this needs to have an override, let alone two- just treat 302 the same as 303 in all cases. There is no case under which that behavior is likely to fail.

A 301 on POST is a rare case. They only work with GET. The common scenario here would be the application is POSTing to a web API, and at some point the API either moves (unlikly, but it happens) or the user used the wrong URL in the first place.

There are 3 possible options on how to handle the resulting 301-

• Follow it, but turn it into a GET: The result body would be an error from the forms processor, complaining that it did not receive the POST data. This is hard to debug, as from the application side, the data clearly is there. In short, the user's application will fail.
• Don't follow it, just return the body as is: The result body will be empty on NGINX, and will be a browser redirect fallback page on Apache. In short, the user's application will fail.
• Follow it, and keep it as a POST: In the former case (the API moved), the user probably won't have specified a redirect_limit to begin with, as it would have worked without it; so, the previous bullet will apply. If the user did specify a limit, the data will be wastefully submitted twice, every time the API is called. If the application is ported to another language, it will fail mysteriously, as no other user-agent keeps it as POST. This also creates a failure mode where an API written in Eu could end up working only with a Eu client and nowhere else. In short, the user's application will fail.

There simply isn't a case where this could work; so it seems to make sense to me that a hard error may be helpful.

37. Comment by jimcbrown Jan 21, 2013

You forgot again, Euphoria doesn't have nulls. Hence we can't distinguish if the user wanted to follow redirects or if it's just the default setting. The enums were a way around this.

This was discussed on the dev list - browsers generally treat the two the same, but HTTP libraries have user configurable behavior here. Most still default to treating 302 the same as 303, but they provide options for implementing strict RFC behavior.

In fact, Python's library will also deal with a 301 redirect on POST by using a GET request on the new url.

http://docs.python.org/library/urllib2.html#urllib2.HTTPRedirectHandler.redirect%5Frequest

I think by default, we should follow the same behavior as urllib2. If the application breaks for some reason, the author can turn on debugging and see what happened.

38. Comment by jimcbrown Jan 22, 2013

One thing which concerned me about CoJaBo's approach was that I wasn't able to find a http library or browser that defaulted to a hard crash on a 301 redirect in response to a POST request. If no one else does it....

39. Comment by CoJaBo2 Jan 22, 2013

Then just take -1 and call it null.. REDIRECT_ALL, REDIRECT_STRICT_RFC2616, REDIRECT_BROWSER seems needlessly convoluted; under what circumstances would it be needed? As the RFC explicitly notes that 302 is implemented as 303, so I don't see the need for special modes to "follow it strictly" or "emulate a browser" that would affect the handling of just one status code. If a constant is wanted, only one would be needed, "FOLLOW_REDIRECT" (or similar), which would be set to whatever the default limit is, and used only to override the default redirect_limit of "null"/-1 in http_post.

Yes; its mostly a case of "do it the same way as everyone else" versus "detect something isn't right here and bail out". The former would trigger an error from the forms processor (which is likely to be unclear, as it wont point to the 301 as the cause), the latter from Eu (which could be more clear in some circumstances).

40. Comment by jimcbrown Jan 22, 2013

Other libraries, such as Perl's LWP and Python's urllib2, provide ways to handle 302 strictly according to the original RFC defined behavior - even if doing so is not the default behavior.

I think we should provide the option, for advanced users.

Regarding 301, I'm not seeing a compeling reason to do this. Euphoria doesn't have exceptions yet, so a hard crash is really undesirable for a lot of reasons. Since it's so easy for an Euphoria programmer to debug (just turn off all redirects and print the headers), and most users who'd get this hardc crash likely are end users of some product that just happens to be written in Euphoria (i.e. not Eu coders), I see doing this as a REALLY BAD IDEA (tm).

41. Comment by CoJaBo2 Jan 22, 2013

There probably should be a general interface to set options for the HTTP functions, if one doesn't already exist (including other options, cookie-handling, etc); it could be set there if it must (though, again, its difficult to see a scenario where this would be necessary).

Maybe this was unclear; I meant return an error, not specifically crash the program. Yes, that would be a bad idea.

As the error would come from the web application, it would not immediately be obvious that a redirect is the cause. There would be no apparent reason to turn off redirects or print out headers; debugging in this case would involve hours of checking the API server config, printing out the parameters being passed, trying to figure out whats "wrong" about them, etc, before finally being emailed the server logs and eventually seeing the GETs and causative 301s there...

42. Comment by jimcbrown Jan 22, 2013

I don't think we have a generic framework (like Python's HTTPRedirectHandler) - if we had one, I'd agree that this would be the right place to put these options in, instead of having an RFC enum.

I can't find a single source that recommends treating a 301-in-response-to-POST as an error. Your reasoning makes a kind of sense, but the fact that no one else recommends it suggests to me that there are probably good reasons why no one else has dared yet.

Of course, maybe it makes sense for Euphoria to just be different here - I think you should push for a vote to be held on this issue.

43. Comment by SDPringle Mar 01, 2014

marking this as fixed as it implements the spec.

44. Comment by useless_ Mar 01, 2014

I hack http.e to use task.e, i can also hack it to let me decide if i want to follow a redirect.

Include:

     News
     Tickets
     Forum
     Wiki
     Manual