From: Ben Darnell Date: Sun, 18 Nov 2012 17:25:13 +0000 (-0500) Subject: Make header_callback behavior consistent across both HTTP clients. X-Git-Tag: v3.0.0~221 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=65d27e5a045d7e096f56bbc711ebe0f716c39d0a;p=thirdparty%2Ftornado.git Make header_callback behavior consistent across both HTTP clients. SimpleAsyncHTTPClient now sends the first and last lines to the callback, matching CurlAsyncHTTPClient's behavior. All secondary callbacks are now wrapped for stack_context. Closes #637. --- diff --git a/tornado/httpclient.py b/tornado/httpclient.py index 5dbc9bfc0..7ab4b046c 100644 --- a/tornado/httpclient.py +++ b/tornado/httpclient.py @@ -38,9 +38,9 @@ import time import weakref from tornado.escape import utf8 -from tornado import httputil +from tornado import httputil, stack_context from tornado.ioloop import IOLoop -from tornado.util import import_object, bytes_type, Configurable +from tornado.util import import_object, Configurable class HTTPClient(object): @@ -232,8 +232,13 @@ class HTTPRequest(object): `~HTTPResponse.body` and `~HTTPResponse.buffer` will be empty in the final response. :arg callable header_callback: If set, `header_callback` will - be run with each header line as it is received, and - `~HTTPResponse.headers` will be empty in the final response. + be run with each header line as it is received (including the + first line, e.g. ``HTTP/1.0 200 OK\r\n``, and a final line + containing only ``\r\n``. All lines include the trailing newline + characters). `~HTTPResponse.headers` will be empty in the final + response. This is most useful in conjunction with + `streaming_callback`, because it's the only way to get access to + header data while the request is in progress. :arg callable prepare_curl_callback: If set, will be called with a `pycurl.Curl` object to allow the application to make additional `setopt` calls. @@ -281,9 +286,9 @@ class HTTPRequest(object): self.user_agent = user_agent self.use_gzip = use_gzip self.network_interface = network_interface - self.streaming_callback = streaming_callback - self.header_callback = header_callback - self.prepare_curl_callback = prepare_curl_callback + self.streaming_callback = stack_context.wrap(streaming_callback) + self.header_callback = stack_context.wrap(header_callback) + self.prepare_curl_callback = stack_context.wrap(prepare_curl_callback) self.allow_nonstandard_methods = allow_nonstandard_methods self.validate_cert = validate_cert self.ca_certs = ca_certs diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 4b0d76145..c42b81002 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -365,8 +365,11 @@ class _HTTPConnection(object): content_length = None if self.request.header_callback is not None: + # re-attach the newline we split on earlier + self.request.header_callback(first_line + _) for k, v in self.headers.get_all(): self.request.header_callback("%s: %s\r\n" % (k, v)) + self.request.header_callback('\r\n') if self.request.method == "HEAD": # HEAD requests never have content, even though they may have diff --git a/tornado/test/curl_httpclient_test.py b/tornado/test/curl_httpclient_test.py index bfda1817e..86ec3a8b5 100644 --- a/tornado/test/curl_httpclient_test.py +++ b/tornado/test/curl_httpclient_test.py @@ -1,6 +1,10 @@ from __future__ import absolute_import, division, with_statement +from tornado.httpclient import HTTPRequest +from tornado.stack_context import ExceptionStackContext +from tornado.testing import AsyncHTTPTestCase from tornado.test import httpclient_test from tornado.test.util import unittest +from tornado.web import Application try: import pycurl @@ -20,3 +24,30 @@ class CurlHTTPClientCommonTestCase(httpclient_test.HTTPClientCommonTestCase): CurlHTTPClientCommonTestCase = unittest.skipIf(pycurl is None, "pycurl module not present")( CurlHTTPClientCommonTestCase) + + +class CurlHTTPClientTestCase(AsyncHTTPTestCase): + def setUp(self): + super(CurlHTTPClientTestCase, self).setUp() + self.http_client = CurlAsyncHTTPClient(self.io_loop) + + def get_app(self): + return Application([]) + + def test_prepare_curl_callback_stack_context(self): + exc_info = [] + def error_handler(typ, value, tb): + exc_info.append((typ, value, tb)) + self.stop() + return True + + with ExceptionStackContext(error_handler): + request = HTTPRequest(self.get_url('/'), + prepare_curl_callback=lambda curl: 1 / 0) + self.http_client.fetch(request, callback=self.stop) + self.wait() + self.assertEqual(1, len(exc_info)) + self.assertIs(exc_info[0][0], ZeroDivisionError) +CurlHTTPClientTestCase = unittest.skipIf(pycurl is None, + "pycurl module not present")( + CurlHTTPClientTestCase) diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index d3e9e5516..5af168f97 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -6,10 +6,12 @@ import base64 import binascii from contextlib import closing import functools +import re from tornado.escape import utf8 from tornado.iostream import IOStream from tornado import netutil +from tornado.stack_context import ExceptionStackContext from tornado.testing import AsyncHTTPTestCase, bind_unused_port from tornado.util import b, bytes_type from tornado.web import Application, RequestHandler, url @@ -135,6 +137,25 @@ Transfer-Encoding: chunked self.assertEqual(resp.body, b("12")) self.io_loop.remove_handler(sock.fileno()) + def test_streaming_stack_context(self): + chunks = [] + exc_info = [] + def error_handler(typ, value, tb): + exc_info.append((typ, value, tb)) + return True + + def streaming_cb(chunk): + chunks.append(chunk) + if chunk == b('qwer'): + 1 / 0 + + with ExceptionStackContext(error_handler): + self.fetch('/chunk', streaming_callback=streaming_cb) + + self.assertEqual(chunks, [b('asdf'), b('qwer')]) + self.assertEqual(1, len(exc_info)) + self.assertIs(exc_info[0][0], ZeroDivisionError) + def test_basic_auth(self): self.assertEqual(self.fetch("/auth", auth_username="Aladdin", auth_password="open sesame").body, @@ -188,3 +209,43 @@ Transfer-Encoding: chunked self.assertEqual(type(response.headers["Content-Type"]), str) self.assertEqual(type(response.code), int) self.assertEqual(type(response.effective_url), str) + + def test_header_callback(self): + first_line = [] + headers = {} + chunks = [] + + def header_callback(header_line): + if header_line.startswith('HTTP/'): + first_line.append(header_line) + elif header_line != '\r\n': + k, v = header_line.split(':', 1) + headers[k] = v.strip() + + def streaming_callback(chunk): + # All header callbacks are run before any streaming callbacks, + # so the header data is available to process the data as it + # comes in. + self.assertEqual(headers['Content-Type'], 'text/html; charset=UTF-8') + chunks.append(chunk) + + self.fetch('/chunk', header_callback=header_callback, + streaming_callback=streaming_callback) + self.assertEqual(len(first_line), 1) + self.assertRegexpMatches(first_line[0], 'HTTP/1.[01] 200 OK\r\n') + self.assertEqual(chunks, [b('asdf'), b('qwer')]) + + def test_header_callback_stack_context(self): + exc_info = [] + def error_handler(typ, value, tb): + exc_info.append((typ, value, tb)) + return True + + def header_callback(header_line): + if header_line.startswith('Content-Type:'): + 1 / 0 + + with ExceptionStackContext(error_handler): + self.fetch('/chunk', header_callback=header_callback) + self.assertEqual(len(exc_info), 1) + self.assertIs(exc_info[0][0], ZeroDivisionError) diff --git a/website/sphinx/releases/next.rst b/website/sphinx/releases/next.rst index 30a8001d9..6c6e6c67c 100644 --- a/website/sphinx/releases/next.rst +++ b/website/sphinx/releases/next.rst @@ -165,3 +165,10 @@ In progress * `tornado.auth.TwitterMixin` now works on Python 3. * ``Etag``/``If-None-Match`` requests now work with `StaticFileHandler`. * `StaticFileHandler` no longer sets ``Cache-Control: public`` unnecessarily. +* The behavior of ``header_callback`` with `SimpleAsyncHTTPClient` has + changed and is now the same as that of `CurlAsyncHTTPClient`. The + header callback now receives the first line of the response (e.g. + ``HTTP/1.0 200 OK``) and the final empty line. +* Secondary `AsyncHTTPClient` callbacks (``streaming_callback``, + ``header_callback``, and ``prepare_curl_callback``) now respect + `StackContext`.