Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Use real httpserver.HTTPRequest in WSGI apps instead of mock #358

Closed
wants to merge 1 commit into from

2 participants

@alekstorm

A large amount of parsing code that the two classes had in common (duplicated, generally in constructors) has been refactored into utility functions so that only minimal logic is implemented. Also, some attributes that were previously set based on multiple conditions in constructors have become properties.

@alekstorm

What's the status of this pull request? Does it need to be updated to merge cleanly?

@alekstorm

@bdarnell Tagging to notify you via email :)

@bdarnell
Owner

Yeah, it looks like it may need a bit of an update to merge. Overall it looks reasonable, but there's one issue that will have to be cleared up before a change like this can go in. The ioloop.py module is not importable on App Engine, and by extension neither is httpserver.py. We'll need to either move HTTPRequest somewhere neutral (httputil.py) or do some conditional imports in httpserver to make it work when the IOLoop is not available (at least enough for HTTPRequest - note that the SSLIOStream type check will probably need to move).

Why did you move some stuff from the constructor to properties?

@bdarnell
Owner

I just tried merging this and the tests failed on python 3 - looks like the path is getting utf8 encoded an extra time.

@bdarnell
Owner

I've just merged changes into the master branch that include replacing the WSGI HTTPRequest with a shared version.

@bdarnell bdarnell closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
33 tornado/httpserver.py
@@ -30,6 +30,7 @@ class except to start a server at the beginning of the process
import logging
import socket
import time
+import urlparse
from tornado.escape import native_str, parse_qs_bytes
from tornado import httputil
@@ -245,8 +246,10 @@ def _on_headers(self, data):
# Unix (or other) socket; fake the remote address
remote_ip = '0.0.0.0'
+ path, sep, query = uri.partition('?')
self._request = HTTPRequest(
- connection=self, method=method, uri=uri, version=version,
+ connection=self, method=method, path=path, query=query,
+ arguments=httputil.parse_query(query), version=version,
headers=headers, remote_ip=remote_ip)
content_length = headers.get("Content-Length")
@@ -345,13 +348,19 @@ class HTTPRequest(object):
An HTTP request is attached to a single HTTP connection, which can
be accessed through the "connection" attribute. Since connections
are typically kept open in HTTP/1.1, multiple requests can be handled
- sequentially on a single connection.
+ sequentially on a single connection. This attribute is set to ``None``
+ in WSGI applications, which do not support asynchronous requests.
"""
- def __init__(self, method, uri, version="HTTP/1.0", headers=None,
- body=None, remote_ip=None, protocol=None, host=None,
- files=None, connection=None):
+ def __init__(self, method, path, query="", arguments=None, version="HTTP/1.0",
+ headers=None, body="", remote_ip=None, protocol=None,
+ host=None, files=None, connection=None):
self.method = method
- self.uri = uri
+ self.uri = path
+ if query:
+ self.uri += '?' + query
+ self.path = path
+ self.query = query
+ self.arguments = arguments or {}
self.version = version
self.headers = headers or httputil.HTTPHeaders()
self.body = body or ""
@@ -378,16 +387,10 @@ def __init__(self, method, uri, version="HTTP/1.0", headers=None,
self.host = host or self.headers.get("Host") or "127.0.0.1"
self.files = files or {}
self.connection = connection
+
self._start_time = time.time()
self._finish_time = None
-
- self.path, sep, self.query = uri.partition('?')
- arguments = parse_qs_bytes(self.query)
- self.arguments = {}
- for name, values in arguments.iteritems():
- values = [v for v in values if v]
- if values:
- self.arguments[name] = values
+ self._cookies = None
def supports_http_1_1(self):
"""Returns True if this request supports HTTP/1.1 semantics"""
@@ -396,7 +399,7 @@ def supports_http_1_1(self):
@property
def cookies(self):
"""A dictionary of Cookie.Morsel objects."""
- if not hasattr(self, "_cookies"):
+ if self._cookies is None:
self._cookies = Cookie.SimpleCookie()
if "Cookie" in self.headers:
try:
View
8 tornado/httputil.py
@@ -224,6 +224,14 @@ def parse_body_arguments(content_type, body, arguments, files):
logging.warning("Invalid multipart/form-data")
+def parse_query(query):
+ arguments = {}
+ for name, values in parse_qs_bytes(query).iteritems():
+ values = [v for v in values if v]
+ if values: arguments[name] = values
+ return arguments
+
+
def parse_multipart_form_data(boundary, data, arguments, files):
"""Parses a multipart/form-data body.
View
4 tornado/web.py
@@ -123,7 +123,7 @@ def __init__(self, application, request, **kwargs):
self.ui["modules"] = self.ui["_modules"]
self.clear()
# Check since connection is not available in WSGI
- if getattr(self.request, "connection", None):
+ if self.request.connection is not None:
self.request.connection.stream.set_close_callback(
self.on_connection_close)
self.initialize(**kwargs)
@@ -689,7 +689,7 @@ def finish(self, chunk=None):
content_length = sum(len(part) for part in self._write_buffer)
self.set_header("Content-Length", content_length)
- if hasattr(self.request, "connection"):
+ if self.request.connection is not None:
# Now that the request is finished, clear the callback we
# set on the IOStream (which would otherwise prevent the
# garbage collection of the RequestHandler when there
View
109 tornado/wsgi.py
@@ -42,7 +42,8 @@
from tornado import escape
from tornado import httputil
from tornado import web
-from tornado.escape import native_str, utf8, parse_qs_bytes
+from tornado.escape import native_str, utf8
+from tornado.httpserver import HTTPRequest
from tornado.util import b, bytes_type
try:
@@ -112,7 +113,36 @@ def __init__(self, handlers=None, default_host="", **settings):
wsgi=True, **settings)
def __call__(self, environ, start_response):
- handler = web.Application.__call__(self, HTTPRequest(environ))
+ """Parses the given WSGI environ to construct the request."""
+ path = (urllib.quote(environ.get("SCRIPT_NAME", ""))
+ + urllib.quote(environ.get("PATH_INFO", "")))
+ query = environ.get("QUERY_STRING", "")
+
+ headers = httputil.HTTPHeaders()
+ if environ.get("CONTENT_TYPE"):
+ headers["Content-Type"] = environ["CONTENT_TYPE"]
+ if environ.get("CONTENT_LENGTH"):
+ headers["Content-Length"] = environ["CONTENT_LENGTH"]
+ for key, value in environ.iteritems():
+ if key.startswith("HTTP_"):
+ headers[key[5:].replace("_", "-")] = value
+
+ method = environ["REQUEST_METHOD"]
+ body = environ["wsgi.input"].read(int(headers["Content-Length"])) if "Content-Length" in headers else ""
+
+ arguments = httputil.parse_query(query)
+ files = {}
+
+ httputil.parse_body_arguments(headers.get("Content-Type", ""), body, arguments, files)
+
+ request = HTTPRequest(method=method, path=path, query=query,
+ arguments=arguments, files=files,
+ version="HTTP/1.1", headers=headers,
+ protocol=environ["wsgi.url_scheme"], body=body,
+ remote_ip=environ.get("REMOTE_ADDR", ""),
+ host=environ.get("HTTP_HOST", environ["SERVER_NAME"]))
+
+ handler = web.Application.__call__(self, request)
assert handler._finished
status = str(handler._status_code) + " " + \
httplib.responses[handler._status_code]
@@ -125,81 +155,6 @@ def __call__(self, environ, start_response):
return handler._write_buffer
-class HTTPRequest(object):
- """Mimics `tornado.httpserver.HTTPRequest` for WSGI applications."""
- def __init__(self, environ):
- """Parses the given WSGI environ to construct the request."""
- self.method = environ["REQUEST_METHOD"]
- self.path = urllib.quote(from_wsgi_str(environ.get("SCRIPT_NAME", "")))
- self.path += urllib.quote(from_wsgi_str(environ.get("PATH_INFO", "")))
- self.uri = self.path
- self.arguments = {}
- self.query = environ.get("QUERY_STRING", "")
- if self.query:
- self.uri += "?" + self.query
- arguments = parse_qs_bytes(native_str(self.query))
- for name, values in arguments.iteritems():
- values = [v for v in values if v]
- if values:
- self.arguments[name] = values
- self.version = "HTTP/1.1"
- self.headers = httputil.HTTPHeaders()
- if environ.get("CONTENT_TYPE"):
- self.headers["Content-Type"] = environ["CONTENT_TYPE"]
- if environ.get("CONTENT_LENGTH"):
- self.headers["Content-Length"] = environ["CONTENT_LENGTH"]
- for key in environ:
- if key.startswith("HTTP_"):
- self.headers[key[5:].replace("_", "-")] = environ[key]
- if self.headers.get("Content-Length"):
- self.body = environ["wsgi.input"].read(
- int(self.headers["Content-Length"]))
- else:
- self.body = ""
- self.protocol = environ["wsgi.url_scheme"]
- self.remote_ip = environ.get("REMOTE_ADDR", "")
- if environ.get("HTTP_HOST"):
- self.host = environ["HTTP_HOST"]
- else:
- self.host = environ["SERVER_NAME"]
-
- # Parse request body
- self.files = {}
- httputil.parse_body_arguments(self.headers.get("Content-Type", ""),
- self.body, self.arguments, self.files)
-
- self._start_time = time.time()
- self._finish_time = None
-
- def supports_http_1_1(self):
- """Returns True if this request supports HTTP/1.1 semantics"""
- return self.version == "HTTP/1.1"
-
- @property
- def cookies(self):
- """A dictionary of Cookie.Morsel objects."""
- if not hasattr(self, "_cookies"):
- self._cookies = Cookie.SimpleCookie()
- if "Cookie" in self.headers:
- try:
- self._cookies.load(
- native_str(self.headers["Cookie"]))
- except Exception:
- self._cookies = None
- return self._cookies
-
- def full_url(self):
- """Reconstructs the full URL for this request."""
- return self.protocol + "://" + self.host + self.uri
-
- def request_time(self):
- """Returns the amount of time it took for this request to execute."""
- if self._finish_time is None:
- return time.time() - self._start_time
- else:
- return self._finish_time - self._start_time
-
-
class WSGIContainer(object):
r"""Makes a WSGI-compatible function runnable on Tornado's HTTP server.
Something went wrong with that request. Please try again.