Skip to content

Commit c0d3e6d

Browse files
committed
http1connection: Improve error logging for invalid host headers
This was previously being logged as an uncaught exception in application code, which is wrong for a malformed request. HTTPInputError now passes through the app-error logging to be caught and reported as a 400 (which logs at the warning level to the access log and info to the general log). Fixes #3510
1 parent 4ff5594 commit c0d3e6d

File tree

4 files changed

+18
-3
lines changed

4 files changed

+18
-3
lines changed

tornado/http1connection.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ def __exit__(
6666
) -> None:
6767
if value is not None:
6868
assert typ is not None
69+
# Let HTTPInputError pass through to higher-level handler
70+
if isinstance(value, httputil.HTTPInputError):
71+
return None
6972
self.logger.error("Uncaught exception", exc_info=(typ, value, tb))
7073
raise _QuietException
7174

tornado/routing.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,8 @@ def finish(self) -> None:
279279
self.delegate.finish()
280280

281281
def on_connection_close(self) -> None:
282-
assert self.delegate is not None
283-
self.delegate.on_connection_close()
282+
if self.delegate is not None:
283+
self.delegate.on_connection_close()
284284

285285

286286
class _DefaultMessageDelegate(httputil.HTTPMessageDelegate):

tornado/test/httpclient_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ def test_invalid_gzip(self):
442442
# test if client hangs on tricky invalid gzip
443443
# curl/simple httpclient have different behavior (exception, logging)
444444
with ExpectLog(
445-
app_log, "(Uncaught exception|Exception in callback)", required=False
445+
gen_log, ".*Malformed HTTP message.*unconsumed gzip data", required=False
446446
):
447447
try:
448448
response = self.fetch("/invalid_gzip")

tornado/test/httpserver_test.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,18 @@ def test_malformed_headers(self):
462462
self.io_loop.add_timeout(datetime.timedelta(seconds=0.05), self.stop)
463463
self.wait()
464464

465+
def test_invalid_host_header_with_whitespace(self):
466+
with ExpectLog(
467+
gen_log, ".*Malformed HTTP message.*Invalid Host header", level=logging.INFO
468+
):
469+
self.stream.write(b"GET / HTTP/1.0\r\nHost: foo bar\r\n\r\n")
470+
start_line, headers, response = self.io_loop.run_sync(
471+
lambda: read_stream_body(self.stream)
472+
)
473+
self.assertEqual("HTTP/1.1", start_line.version)
474+
self.assertEqual(400, start_line.code)
475+
self.assertEqual("Bad Request", start_line.reason)
476+
465477
def test_chunked_request_body(self):
466478
# Chunked requests are not widely supported and we don't have a way
467479
# to generate them in AsyncHTTPClient, but HTTPServer will read them.

0 commit comments

Comments
 (0)