User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 The HTTP ACCEPT header is not preserved when after a GET redirect. That leads that the second GET is not using the original HTTP ACCEPT header, but is reset to the standard. This is a really important feature for the proper implementation of RESTful architectures. I'm currently developing some RESTful prototypes and this feature is a must when dealing with aliasing of resources, for instance in order to access resources by their ID or any other resource key field (as its unique name). The problem is that the http accept header in particular is very important to determine the proper format to de-reference to the resource to (as when using Ruby on Rails respond_to in order to determine the content-type to respond with). Reproducible: Always Steps to Reproduce: http://localhost:3034/elements/ApplicantDetails GET /elements/ApplicantDetails HTTP/1.1 Host: localhost:3034 User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 Accept: application/json Accept-Language: en-gb,es-es;q=0.7,en;q=0.3 Accept-Encoding: gzip,deflate Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7 Keep-Alive: 300 Connection: keep-alive X-Requested-With: XMLHttpRequest X-Prototype-Version: 1.5.0 Referer: http://localhost:3034/elements/ Cookie: _XMLDictionaryREST_session_id=BAh7CDoOcmV0dXJuX3RvIg8vZWxlbWVudHMvOgtsb2NhbGUiCmVuLWdiIgpm%250AbGFzaElDOidBY3Rpb25Db250cm9sbGVyOjpGbGFzaDo6Rmxhc2hIYXNoewAG%250AOgpAdXNlZHsA--3392c9915ea7c440fd48aca0423dedd0e26f5595 HTTP/1.x 302 Moved Temporarily Content-Length: 99 Connection: close Date: Mon, 29 Oct 2007 14:55:47 GMT Status: 302 Found Location: http://localhost:3034/elements/96 X-Runtime: 0.01500 Cache-Control: no-cache Server: Mongrel 0.3.13.3 Content-Type: text/html; charset=utf-8 ---------------------------------------------------------- http://localhost:3034/elements/96 GET /elements/96 HTTP/1.1 Host: localhost:3034 User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 Accept-Language: en-gb,es-es;q=0.7,en;q=0.3 Accept-Encoding: gzip,deflate Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7 Keep-Alive: 300 Connection: keep-alive Referer: http://localhost:3034/elements/ Cookie: _XMLDictionaryREST_session_id=BAh7CDoOcmV0dXJuX3RvIg8vZWxlbWVudHMvOgtsb2NhbGUiCmVuLWdiIgpm%250AbGFzaElDOidBY3Rpb25Db250cm9sbGVyOjpGbGFzaDo6Rmxhc2hIYXNoewAG%250AOgpAdXNlZHsA--3392c9915ea7c440fd48aca0423dedd0e26f5595

Another paste just to make clear that the Content-Type returned by the redirect does affect the HTTP ACCEPT header used by FF when redirecting :) http://localhost:3034/elements/ApplicantIdentifier?_dc=1193670352239 GET /elements/ApplicantIdentifier?_dc=1193670352239 HTTP/1.1 Host: localhost:3034 User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 Accept: application/json Accept-Language: en-gb,es-es;q=0.7,en;q=0.3 Accept-Encoding: gzip,deflate Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7 Keep-Alive: 300 Connection: keep-alive X-Requested-With: XMLHttpRequest X-Prototype-Version: 1.5.0 Referer: http://localhost:3034/elements/ Cookie: _XMLDictionaryREST_session_id=BAh7CDoOcmV0dXJuX3RvIhEvZWxlbWVudHMvOTY6C2xvY2FsZSIKZW4tZ2Ii%250ACmZsYXNoSUM6J0FjdGlvbkNvbnRyb2xsZXI6OkZsYXNoOjpGbGFzaEhhc2h7%250AAAY6CkB1c2VkewA%253D--945a7380007bab5ac06eceb4a235805531881f47 HTTP/1.x 302 Moved Temporarily Content-Length: 99 Connection: close Date: Mon, 29 Oct 2007 15:05:52 GMT Status: 302 Found Location: http://localhost:3034/elements/97 X-Runtime: 0.06200 Cache-Control: no-cache Server: Mongrel 0.3.13.3 Content-Type: application/json; charset=utf-8 ---------------------------------------------------------- http://localhost:3034/elements/97 GET /elements/97 HTTP/1.1 Host: localhost:3034 User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8 Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 Accept-Language: en-gb,es-es;q=0.7,en;q=0.3 Accept-Encoding: gzip,deflate Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7 Keep-Alive: 300 Connection: keep-alive Referer: http://localhost:3034/elements/ Cookie: _XMLDictionaryREST_session_id=BAh7CDoOcmV0dXJuX3RvIhEvZWxlbWVudHMvOTY6C2xvY2FsZSIKZW4tZ2Ii%250ACmZsYXNoSUM6J0FjdGlvbkNvbnRyb2xsZXI6OkZsYXNoOjpGbGFzaEhhc2h7%250AAAY6CkB1c2VkewA%253D--945a7380007bab5ac06eceb4a235805531881f47

I see this too, and not just with the Accept header; my If-Modified-Since custom header was dropped from a redirected request in a recent coding project. In my case I was able to work around the problem by updating the URL I was loading to the new one, but that's not always possible (hence the whole point of having a mechanism for automatically redirecting old URLs to new ones). XMLHttpRequest should really be preserving these custom headers across redirect requests.

Yeah, I think all custom headers are dropped. Looking at the redirect code the other day I didn't see code that copied any headers over, other than referrer since that is stored separately from other headers. What does the HTTP spec say on the subject?

RFC 2616 (Hypertext Transfer Protocol -- HTTP/1.1) <http://www.ietf.org/rfc/rfc2616.txt> section 10.3 (Redirection 3xx) doesn't say anything one way or the other about header preservation across redirects.

What do other browsers do? One way to try is with XMLHttpRequest, though checking with <script> and <img> might also be possible.

(In reply to comment #6) > What do other browsers do? One way to try is with XMLHttpRequest, though > checking with <script> and <img> might also be possible. I wrote a testcase <http://www.melez.com/mozilla/bug401564.html> that retrieves http://www.melez.com/foo (which redirects to http://www.melez.com/bar), setting an If-Modified-Since header that causes the server to return 304 (not modified) if the header is preserved. The testcase then outputs the response status code, which is either 304 (if the header is preserved) or 200 (if the header is not preserved). Here are the results for a few common browsers: Firefox 3: 200 Safari 3: 304 Chrome 1: 304 IE 7: 200 Opera 9.64: 200

RFC 2616 (Hypertext Transfer Protocol -- HTTP/1.1) <http://www.ietf.org/rfc/rfc2616.txt> section 10.3 says that "This class of status code indicates that further action needs to be taken by the user agent in order to fulfill the request." - I interpret that as if the second redirected request should be considered to be the same as the first request, only to another location, something that 10.3.4 also suggest when saying that the 303 See Other does not contain "a substitute reference for the originally requested resource". An XMLHttpRequest should contain the same headers for all redirected requests except those of 201 and 303 as far as I understand - because those two are not considered as substitute requests. Although there needs to be a way to specify headers for 201 and 303 redirects as well by perhaps being able to disable XMLHttpRequest automatically redirecting requests so that we manually can specify the correct headers. I would also like to point out that this applies to all HTTP methods - POST as well as GET and probably PUT and the others as well.

Although I don't think the HTTP spec says or implies anything about what to do in this situation, I do agree with Pelle that it would make sense for XMLHttpRequest to persist these headers or, if it's not safe to automatically persist headers, then also not automatically follow the redirect. It's not safe to simply automatically follow the redirect but without the headers that the caller expects the request to contain, as currently occurs.

A redirected IMG request also has similar problem too, even on Firefox 4.0.1. My simple experiment showed the first IMG request was sent with "Accept: image/png,image/*;q=0.8,*/*;q=0.5", but the redirected one was with "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", which is clearly wrong as Firefox cannot display text/html as an IMG tag.

Worth noting is also bug 553888, which is scoped to redirected XMLHttpRequests retaining custom headers.

Jonas, there's a contributor who's interested in working on this bug. Do we have a firm decision one way or the other as to whether it's desired?

I will help out the contributor that is interested in working on this. Who is he/she?

The one thing I'd be concerned about is that we don't cause scary headers to be forwarded across cross-origin redirects. Especially in requests that originate from plugins which generally means that the plugin doesn't get to participate in the redirect. Here's the type of plugin-initiated request redirects we currently block: http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginStreamListenerPeer.cpp#1394 I suspect that code need to block more redirects

It looks like FireFox redirects on HTTP 201 as well, which is "Really Bad" (tm). Please fix this soon!

(In reply to Gili from comment #16) > It looks like FireFox redirects on HTTP 201 as well, which is "Really Bad" > (tm). Please fix this soon! firefox does not generally redirect on 201 - test http://www.ducksong.com/cgi-bin/nph-redir-201 can you provide a test case that illustrates your comment?

Hi Patrick, My mistake. I am invoking http://api.jquery.com/jQuery.ajax/ against a URI that returns HTTP 201. I was mislead by the fact that "data" returned a Document instead of an empty object if the resource returns an empty response body. I can confirm that xhr.status and xhr.getResponseHeader("Location") return the values I was looking for. Sorry for the false alarm.

i would like to work on this bug.

Like the assignee field currently says, it's ok to simply take this bug. I.e. assign the bug to yourself and attach a patch once you have one :)

Aniss, I heard you were asking about how to create a test for this. You should use the xpcshell harness (https://developer.mozilla.org/en-US/docs/Writing_xpcshell-based_unit_tests); there are many examples in netwerk/test/unit to learn from. In particular, you can create an nsHttpServer with two handlers - one causes a redirect to the second, and the second checks for the presence of the header. The test_307_redirect.js test might be a good reference.

i was thinking of adding this line of code in the line 1563 of nsHttpChannel.cpp: newchannel.setrequestheader("accept",mRequestHead.peekheader(accept),0); what do yoou think about that??

I think it's in your interest to write a test that doesn't pass, then try your patch and see if it works.

I took a shot at this bug, following the advice in this thread. I'm a bit concerned that the solution is a bit too specialized, and it should try to replicate all the additional headers (are they easily found?).

This patch only copies the Accept header, and leaves all others. Is it preferable to handle all additional headers that were in the original request?

Woo, there's a test and everything! Scott, bug 216828 covers general headers for redirected requests, so this one can probably stay focused on just Accept. You'll want to set the review flag on your patch (click Details) to '?' and target it at :sicking, I think; this will kick off our code review process.

Josh, thanks for taking the time to respond (on your vacation no less!), much appreciated!

Is there anything else I should be doing to/for/with this bug?

Comment on attachment 8376849 [details] [diff] [review] Test and fix for losing Accept header information after redirect Review of attachment 8376849 [details] [diff] [review]: ----------------------------------------------------------------- Scott - thanks for doing this. Please make the following changes and request review again from me with the new patch. After that I'll give it a r+ and you're ready to check it in. You can do that with the checkin-needed keyword documented https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch ::: netwerk/protocol/http/HttpBaseChannel.cpp @@ +1811,5 @@ > httpChannel->SetRedirectionLimit(mRedirectionLimit - 1); > > + // convey the Accept header value > + { > + nsCString oldAcceptValue; nsAutoCString @@ +1813,5 @@ > + // convey the Accept header value > + { > + nsCString oldAcceptValue; > + nsresult hasHeader = mRequestHead.GetHeader(nsHttp::Accept, oldAcceptValue); > + if (hasHeader == NS_OK) { if (NS_SUCCEEDED(hasHeader)) ::: netwerk/test/unit/test_bug401564.js @@ +2,5 @@ > +"use strict"; > +Cu.import("resource://testing-common/httpd.js"); > + > +var httpserver = null; > +const URL = "http://localhost:8888"; you don't need URL @@ +3,5 @@ > +Cu.import("resource://testing-common/httpd.js"); > + > +var httpserver = null; > +const URL = "http://localhost:8888"; > +const uri = URL + "/redirect"; you don't need uri @@ +4,5 @@ > + > +var httpserver = null; > +const URL = "http://localhost:8888"; > +const uri = URL + "/redirect"; > +const noRedirectURI = URL + "/content"; you can just use "/content".. technically http/1 requires an absolute uri location, but gecko allows relative ones (all browsers do). this saves you the trouble of looking up the port number. @@ +29,5 @@ > +{ > + httpserver = new HttpServer(); > + httpserver.registerPathHandler("/redirect", redirectHandler); > + httpserver.registerPathHandler("/content", contentHandler); > + httpserver.start(8888); httpserver.start(-1)

Updates to the test and patch after review.

Comment on attachment 8381655 [details] [diff] [review] bug-401564.patch Review of attachment 8381655 [details] [diff] [review]: ----------------------------------------------------------------- thanks!

If you attach a patch with "r=mcmanus" at the end of the commit message summary, you can add the checkin-needed keyword (see the Keywords field up top) and it will be committed.

Same as before, just changing commit message to have r=mcmanus