and document our redirection behaviour.
Created attachment 60459 [details] Patch
Brady, can you have a look at this?
Comment on attachment 60459 [details] Patch I know how all this stuff is supposed to work, but I'm too tired right now to remember. If you don't get a review soon, feel free to ping me and I'll look in more detail. In general, what the RFC says about this stuff is bogus. :(
Comment on attachment 60459 [details] Patch Your comment is making me think you don't know this history of this topic. How to handle these redirects is a somewhat sad tale. The requirements for these status codes are changing in the next generation of the HTTP spec to more closely align with reality. I need to look up the proper behavior, but POST shouldn't be special-cased. I think HEAD (and possibly OPTIONS) are the only ones with special cases.
(In reply to comment #4) > (From update of attachment 60459 [details]) > Your comment is making me think you don't know this history of this topic. How to handle these redirects is a somewhat sad tale. The requirements for these status codes are changing in the next generation of the HTTP spec to more closely align with reality. > > I need to look up the proper behavior, but POST shouldn't be special-cased. I think HEAD (and possibly OPTIONS) are the only ones with special cases. So the code is wrong as well as the comments? I thought http/tests/loading/redirect-methods.html and others were all about handling cases where POST was used and multiple redirects followed. I'm confused now.
Created attachment 66668 [details] Patch
(In reply to comment #4) > (From update of attachment 60459 [details]) > Your comment is making me think you don't know this history of this topic. How to handle these redirects is a somewhat sad tale. The requirements for these status codes are changing in the next generation of the HTTP spec to more closely align with reality. > > I need to look up the proper behavior, but POST shouldn't be special-cased. I think HEAD (and possibly OPTIONS) are the only ones with special cases. I've resubmitted and removed the comments about the RFC. I just describe what we do in order to pass the layout tests.
Comment on attachment 66668 [details] Patch I don't think this is quite right. You shouldn't need m_receivedPermanentRedirect. What happens is that the first 301 (or whatever) changes the method to GET. By the time you get to the 307, you shouldn't have a POST method anymore. 307 should always preserve the method of the request that received the 307. That's the whole point of 307 existing.
(In reply to comment #8) > (From update of attachment 66668 [details]) > I don't think this is quite right. You shouldn't need m_receivedPermanentRedirect. What happens is that the first 301 (or whatever) changes the method to GET. Yes. >By the time you get to the 307, you shouldn't have a POST method anymore. We do. We have it in the original request (m_resourceHandle->firstRequest()), which is what QNetworkReplyHandler inspects here. We don't currently keep a record of the most recent method sent in response to a redirect. >307 should always preserve the method of the request that received the 307. That's the whole point of 307 existing. OK. The patch does that though - by reinstating the GET used for the previous 301-303, if that was what was used. The code in sendResponseIfNeeded() isn't very clear so I'll refactor it.
Created attachment 68949 [details] Patch
Attachment 68949 [details] did not build on qt: Build output: http://queues.webkit.org/results/4057156
Created attachment 68951 [details] Patch
Comment on attachment 68951 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68951&action=review Looks great! Thanks. > WebCore/platform/network/qt/QNetworkReplyHandler.cpp:393 > + if ((statusCode >= 301 && statusCode <= 303) && m_resourceHandle->firstRequest().httpMethod() == "POST") You've got an extra space in "&& m_resourceHandle"
Created attachment 69379 [details] Patch
Comment on attachment 69379 [details] Patch Clearing flags on attachment: 69379 Committed r68906: <http://trac.webkit.org/changeset/68906>
All reviewed patches have been landed. Closing bug.