Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Inline the validate() function
It somewhat obscures the control flow and adds some non-trivial
overhead.

Inline it in favor of the direct match/if combination.

On the bench/ microbenchmark:

Before: 10700.2 requests/sec
After : 11334.0 requests/sec
  • Loading branch information
bluetech committed Nov 18, 2020
commit 40df2b6dbbf3f8a3bb847d3fa61cf20a94f0acba
5 changes: 3 additions & 2 deletions h11/_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from . import _headers
from ._abnf import request_target
from ._util import bytesify, LocalProtocolError, validate
from ._util import bytesify, LocalProtocolError

# Everything in __all__ gets re-exported as part of the h11 public API.
__all__ = [
Expand Down Expand Up @@ -85,7 +85,8 @@ def __init__(self, method, target, headers, http_version=b"1.1", _parsed=False):
if host_count > 1:
raise LocalProtocolError("Found multiple Host: headers")

validate(request_target_re, self.target, "Illegal target characters")
if request_target_re.fullmatch(self.target) is None:
raise LocalProtocolError("Illegal target characters")

def __repr__(self):
return "{}(method={}, target={}, headers={}, http_version={})".format(
Expand Down
11 changes: 7 additions & 4 deletions h11/_headers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re

from ._abnf import field_name, field_value
from ._util import bytesify, LocalProtocolError, validate
from ._util import bytesify, LocalProtocolError

# Facts
# -----
Expand Down Expand Up @@ -127,16 +127,19 @@ def normalize_and_validate(headers, _parsed=False):
if not _parsed:
name = bytesify(name)
value = bytesify(value)
validate(_field_name_re, name, "Illegal header name {!r}", name)
validate(_field_value_re, value, "Illegal header value {!r}", value)
if _field_name_re.fullmatch(name) is None:
raise LocalProtocolError("Illegal header name {!r}".format(name))
if _field_value_re.fullmatch(value) is None:
raise LocalProtocolError("Illegal header value {!r}".format(value))
raw_name = name
name = name.lower()
if name == b"content-length":
lengths = {length.strip() for length in value.split(b",")}
if len(lengths) != 1:
raise LocalProtocolError("conflicting Content-Length headers")
value = lengths.pop()
validate(_content_length_re, value, "bad Content-Length")
if _content_length_re.fullmatch(value) is None:
raise LocalProtocolError("bad Content-Length")
if seen_content_length is None:
seen_content_length = value
new_headers.append((raw_name, name, value))
Expand Down
50 changes: 28 additions & 22 deletions h11/_readers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
# Strategy: each reader is a callable which takes a ReceiveBuffer object, and
# either:
# 1) consumes some of it and returns an Event
# 2) raises a LocalProtocolError (for consistency -- e.g. we call validate()
# and it might raise a LocalProtocolError, so simpler just to always use
# this)
# 2) raises a LocalProtocolError
# 3) returns None, meaning "I need more data"
#
# If they have a .read_eof attribute, then this will be called if an EOF is
Expand All @@ -21,7 +19,7 @@
from ._abnf import chunk_header, header_field, request_line, status_line
from ._events import *
from ._state import *
from ._util import LocalProtocolError, RemoteProtocolError, validate
from ._util import LocalProtocolError, RemoteProtocolError

__all__ = ["READERS"]

Expand Down Expand Up @@ -54,8 +52,10 @@ def _obsolete_line_fold(lines):

def _decode_header_lines(lines):
for line in _obsolete_line_fold(lines):
matches = validate(header_field_re, line, "illegal header line: {!r}", line)
yield (matches["field_name"], matches["field_value"])
match = header_field_re.fullmatch(line)
if match is None:
raise LocalProtocolError("illegal header line: {!r}", line)
yield match.group("field_name", "field_value")


request_line_re = re.compile(request_line.encode("ascii"))
Expand All @@ -67,11 +67,15 @@ def maybe_read_from_IDLE_client(buf):
return None
if not lines:
raise LocalProtocolError("no request line received")
matches = validate(
request_line_re, lines[0], "illegal request line: {!r}", lines[0]
)
match = request_line_re.fullmatch(lines[0])
if match is None:
raise LocalProtocolError("illegal request line: {!r}", lines[0])
return Request(
headers=list(_decode_header_lines(lines[1:])), _parsed=True, **matches
headers=list(_decode_header_lines(lines[1:])),
method=match.group("method"),
target=match.group("target"),
http_version=match.group("http_version"),
_parsed=True,
)


Expand All @@ -84,14 +88,19 @@ def maybe_read_from_SEND_RESPONSE_server(buf):
return None
if not lines:
raise LocalProtocolError("no response line received")
matches = validate(status_line_re, lines[0], "illegal status line: {!r}", lines[0])
match = status_line_re.fullmatch(lines[0])
if match is None:
raise LocalProtocolError("illegal status line: {!r}", lines[0])
# Tolerate missing reason phrases
if matches["reason"] is None:
matches["reason"] = b""
status_code = matches["status_code"] = int(matches["status_code"])
reason = match.group("reason") or b""
status_code = int(match.group("status_code"))
class_ = InformationalResponse if status_code < 200 else Response
return class_(
headers=list(_decode_header_lines(lines[1:])), _parsed=True, **matches
status_code=status_code,
headers=list(_decode_header_lines(lines[1:])),
http_version=match.group("http_version"),
reason=reason,
_parsed=True,
)


Expand Down Expand Up @@ -150,14 +159,11 @@ def __call__(self, buf):
chunk_header = buf.maybe_extract_until_next(b"\r\n")
if chunk_header is None:
return None
matches = validate(
chunk_header_re,
chunk_header,
"illegal chunk header: {!r}",
chunk_header,
)
match = chunk_header_re.fullmatch(chunk_header)
if match is None:
raise LocalProtocolError("illegal chunk header: {!r}", chunk_header)
# XX FIXME: we discard chunk extensions. Does anyone care?
self._bytes_in_chunk = int(matches["chunk_size"], base=16)
self._bytes_in_chunk = int(match.group("chunk_size"), base=16)
if self._bytes_in_chunk == 0:
self._reading_trailer = True
return self(buf)
Expand Down
10 changes: 0 additions & 10 deletions h11/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"ProtocolError",
"LocalProtocolError",
"RemoteProtocolError",
"validate",
"make_sentinel",
"bytesify",
]
Expand Down Expand Up @@ -80,15 +79,6 @@ class RemoteProtocolError(ProtocolError):
pass


def validate(regex, data, msg="malformed data", *format_args):
match = regex.fullmatch(data)
if not match:
if format_args:
msg = msg.format(*format_args)
raise LocalProtocolError(msg)
return match.groupdict()


# Sentinel values
#
# - Inherit identity-based comparison and hashing from object
Expand Down
31 changes: 0 additions & 31 deletions h11/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,37 +42,6 @@ def thunk():
assert new_traceback.endswith(orig_traceback)


def test_validate():
my_re = re.compile(br"(?P<group1>[0-9]+)\.(?P<group2>[0-9]+)")
with pytest.raises(LocalProtocolError):
validate(my_re, b"0.")

groups = validate(my_re, b"0.1")
assert groups == {"group1": b"0", "group2": b"1"}

# successful partial matches are an error - must match whole string
with pytest.raises(LocalProtocolError):
validate(my_re, b"0.1xx")
with pytest.raises(LocalProtocolError):
validate(my_re, b"0.1\n")


def test_validate_formatting():
my_re = re.compile(br"foo")

with pytest.raises(LocalProtocolError) as excinfo:
validate(my_re, b"", "oops")
assert "oops" in str(excinfo.value)

with pytest.raises(LocalProtocolError) as excinfo:
validate(my_re, b"", "oops {}")
assert "oops {}" in str(excinfo.value)

with pytest.raises(LocalProtocolError) as excinfo:
validate(my_re, b"", "oops {} xx", 10)
assert "oops 10 xx" in str(excinfo.value)


def test_make_sentinel():
S = make_sentinel("S")
assert repr(S) == "S"
Expand Down