From ff343ae91c35eee0233c4eae182f40711e432dff Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Wed, 31 Oct 2012 22:53:08 -0700 Subject: [PATCH] simple_httpclient: Treat 302 like 303. Cherry-picked from https://github.com/bergundy/tornado/commit/e49b263de2f4aa7b9748531b2ccf6b50b28b1bd5 Closes #623. --- tornado/simple_httpclient.py | 9 ++++++-- tornado/test/simple_httpclient_test.py | 31 +++++++++++++------------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 0c8ffaa40a..4b0d761450 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -409,8 +409,13 @@ def _on_body(self, data): new_request.max_redirects -= 1 del new_request.headers["Host"] # http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.4 - # client SHOULD make a GET request - if self.code == 303: + # Client SHOULD make a GET request after a 303. + # According to the spec, 302 should be followed by the same + # method as the original request, but in practice browsers + # treat 302 the same as 303, and many servers use 302 for + # compatibility with pre-HTTP/1.1 user agents which don't + # understand the 303 status. + if self.code in (302, 303): new_request.method = "GET" new_request.body = None for h in ["Content-Length", "Content-Type", diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 1d6859c3db..e0e6d376b5 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -74,15 +74,15 @@ def get(self): self.set_status(204) -class SeeOther303PostHandler(RequestHandler): +class SeeOtherPostHandler(RequestHandler): def post(self): - if self.request.body != b("blah"): - raise Exception("unexpected body %r" % self.request.body) - self.set_header("Location", "/303_get") - self.set_status(303) + redirect_code = int(self.request.body) + assert redirect_code in (302, 303), "unexpected body %r" % self.request.body + self.set_header("Location", "/see_other_get") + self.set_status(redirect_code) -class SeeOther303GetHandler(RequestHandler): +class SeeOtherGetHandler(RequestHandler): def get(self): if self.request.body: raise Exception("unexpected body %r" % self.request.body) @@ -113,8 +113,8 @@ def get_app(self): url("/head", HeadHandler), url("/options", OptionsHandler), url("/no_content", NoContentHandler), - url("/303_post", SeeOther303PostHandler), - url("/303_get", SeeOther303GetHandler), + url("/see_other_post", SeeOtherPostHandler), + url("/see_other_get", SeeOtherGetHandler), url("/host_echo", HostEchoHandler), ], gzip=True) @@ -201,13 +201,14 @@ def test_header_reuse(self): self.fetch("/hello", headers=headers) self.assertEqual(list(headers.get_all()), [('User-Agent', 'Foo')]) - def test_303_redirect(self): - response = self.fetch("/303_post", method="POST", body="blah") - self.assertEqual(200, response.code) - self.assertTrue(response.request.url.endswith("/303_post")) - self.assertTrue(response.effective_url.endswith("/303_get")) - #request is the original request, is a POST still - self.assertEqual("POST", response.request.method) + def test_see_other_redirect(self): + for code in (302, 303): + response = self.fetch("/see_other_post", method="POST", body="%d" % code) + self.assertEqual(200, response.code) + self.assertTrue(response.request.url.endswith("/see_other_post")) + self.assertTrue(response.effective_url.endswith("/see_other_get")) + #request is the original request, is a POST still + self.assertEqual("POST", response.request.method) def test_request_timeout(self): with ExpectLog(gen_log, "uncaught exception"):