From 1667e0b7f800f4fe41ad49c17f7151c6a6058c92 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 28 Sep 2011 23:26:58 +1300 Subject: [PATCH 1/3] handle HTTP 303 redirects correctly see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.4 In response to a 303 the client SHOULD make a GET to the Location, was using the original method. --- tornado/simple_httpclient.py | 7 ++++++- tornado/test/simple_httpclient_test.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index a98eb544..79a5960c 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -350,12 +350,17 @@ class _HTTPConnection(object): self.request) if (self.request.follow_redirects and self.request.max_redirects > 0 and - self.code in (301, 302)): + self.code in (301, 302, 303)): new_request = copy.copy(self.request) new_request.url = urlparse.urljoin(self.request.url, self.headers["Location"]) 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: + new_request.method = "GET" + new_request.body = None new_request.original_request = original_request final_callback = self.final_callback self.final_callback = None diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index f6d7235a..55d5cf14 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -44,6 +44,16 @@ class ContentLengthHandler(RequestHandler): self.set_header("Content-Length", self.get_argument("value")) self.write("ok") +class PRGPostHandler(RequestHandler): + def post(self): + self.set_header("Location", "/prg_get") + self.set_status(303) + +class PRGGetHandler(RequestHandler): + def get(self): + self.write("ok") + + class SimpleHTTPClientTestCase(AsyncHTTPTestCase, LogTrapTestCase): def get_app(self): # callable objects to finish pending /trigger requests @@ -56,6 +66,8 @@ class SimpleHTTPClientTestCase(AsyncHTTPTestCase, LogTrapTestCase): url("/hang", HangHandler), url("/hello", HelloWorldHandler), url("/content_length", ContentLengthHandler), + url("/prg_post", PRGPostHandler), + url("/prg_get", PRGGetHandler), ], gzip=True) def test_singleton(self): @@ -134,6 +146,14 @@ class SimpleHTTPClientTestCase(AsyncHTTPTestCase, LogTrapTestCase): self.assertTrue(response.effective_url.endswith("/countdown/2")) self.assertTrue(response.headers["Location"].endswith("/countdown/1")) + def test_303_redirect(self): + response = self.fetch("/prg_post", method="POST", body="") + self.assertEqual(200, response.code) + self.assertTrue(response.request.url.endswith("/prg_post")) + self.assertTrue(response.effective_url.endswith("/prg_get")) + #request is the original request, is a POST still + self.assertEqual("POST", response.request.method) + def test_request_timeout(self): response = self.fetch('/hang', request_timeout=0.1) self.assertEqual(response.code, 599) From 6cd8e08fe3092b3ccfa167b0560c42c4348989bd Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Sun, 2 Oct 2011 21:07:02 +1300 Subject: [PATCH 2/3] remove content headers for 303 redirect when doing a GET request for a 303 redirect clear the Content-Length and Content-Type headers, they are set during the initial POST. --- tornado/simple_httpclient.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 79a5960c..5bf3d158 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -361,6 +361,11 @@ class _HTTPConnection(object): if self.code == 303: new_request.method = "GET" new_request.body = None + for h in ["Content-Length", "Content-Type"]: + try: + del self.request.headers[h] + except KeyError: + pass new_request.original_request = original_request final_callback = self.final_callback self.final_callback = None From 45752bfdf96724e88d616f626132461e08e02a33 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Mon, 24 Oct 2011 23:38:54 +1300 Subject: [PATCH 3/3] Clear Content-Encoding and Transfer-Encoding Also better test names. --- tornado/simple_httpclient.py | 3 ++- tornado/test/simple_httpclient_test.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 5bf3d158..00978a6c 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -361,7 +361,8 @@ class _HTTPConnection(object): if self.code == 303: new_request.method = "GET" new_request.body = None - for h in ["Content-Length", "Content-Type"]: + for h in ["Content-Length", "Content-Type", + "Content-Encoding", "Transfer-Encoding"]: try: del self.request.headers[h] except KeyError: diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 55d5cf14..a87fb03b 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -44,12 +44,12 @@ class ContentLengthHandler(RequestHandler): self.set_header("Content-Length", self.get_argument("value")) self.write("ok") -class PRGPostHandler(RequestHandler): +class SeeOther303PostHandler(RequestHandler): def post(self): - self.set_header("Location", "/prg_get") + self.set_header("Location", "/303_get") self.set_status(303) -class PRGGetHandler(RequestHandler): +class SeeOther303GetHandler(RequestHandler): def get(self): self.write("ok") @@ -66,8 +66,8 @@ class SimpleHTTPClientTestCase(AsyncHTTPTestCase, LogTrapTestCase): url("/hang", HangHandler), url("/hello", HelloWorldHandler), url("/content_length", ContentLengthHandler), - url("/prg_post", PRGPostHandler), - url("/prg_get", PRGGetHandler), + url("/303_post", SeeOther303PostHandler), + url("/303_get", SeeOther303GetHandler), ], gzip=True) def test_singleton(self): @@ -147,10 +147,10 @@ class SimpleHTTPClientTestCase(AsyncHTTPTestCase, LogTrapTestCase): self.assertTrue(response.headers["Location"].endswith("/countdown/1")) def test_303_redirect(self): - response = self.fetch("/prg_post", method="POST", body="") + response = self.fetch("/303_post", method="POST", body="") self.assertEqual(200, response.code) - self.assertTrue(response.request.url.endswith("/prg_post")) - self.assertTrue(response.effective_url.endswith("/prg_get")) + 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)