Skip to content

Commit

Permalink
Properly enforce max_request_header_size
Browse files Browse the repository at this point in the history
Previously this would not be properly enforced if the request was passed
to waitress in one go, and the request headers could be larger than the
administrator had allowed.
  • Loading branch information
digitalresistor committed Dec 19, 2019
1 parent 8eba394 commit fb08ecf
Showing 1 changed file with 31 additions and 17 deletions.
48 changes: 31 additions & 17 deletions waitress/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,41 @@ def received(self, data):
"""
if self.completed:
return 0 # Can't consume any more.

datalen = len(data)
br = self.body_rcv
if br is None:
# In header.
max_header = self.adj.max_request_header_size

s = self.header_plus + data
index = find_double_newline(s)
consumed = 0

if index >= 0:
# If the headers have ended, and we also have part of the body
# message in data we still want to validate we aren't going
# over our limit for received headers.
self.header_bytes_received += index
consumed = datalen - (len(s) - index)
else:
self.header_bytes_received += datalen
consumed = datalen

# If the first line + headers is over the max length, we return a
# RequestHeaderFieldsTooLarge error rather than continuing to
# attempt to parse the headers.
if self.header_bytes_received >= max_header:
self.parse_header(b"GET / HTTP/1.0\r\n")
self.error = RequestHeaderFieldsTooLarge(
"exceeds max_header of %s" % max_header
)
self.completed = True
return consumed

if index >= 0:
# Header finished.
header_plus = s[:index]
consumed = len(data) - (len(s) - index)

# Remove preceeding blank lines. This is suggested by
# https://tools.ietf.org/html/rfc7230#section-3.5 to support
Expand Down Expand Up @@ -117,22 +142,10 @@ def received(self, data):
self.completed = True
self.headers_finished = True
return consumed
else:
# Header not finished yet.
self.header_bytes_received += datalen
max_header = self.adj.max_request_header_size
if self.header_bytes_received >= max_header:
# malformed header, we need to construct some request
# on our own. we disregard the incoming(?) requests HTTP
# version and just use 1.0. IOW someone just sent garbage
# over the wire
self.parse_header(b"GET / HTTP/1.0\n")
self.error = RequestHeaderFieldsTooLarge(
"exceeds max_header of %s" % max_header
)
self.completed = True
self.header_plus = s
return datalen

# Header not finished yet.
self.header_plus = s
return datalen
else:
# In body.
consumed = br.received(data)
Expand All @@ -157,6 +170,7 @@ def received(self, data):
# appear to the client to be an entirely non-chunked HTTP
# request with a valid content-length.
self.headers["CONTENT_LENGTH"] = str(br.__len__())

return consumed

def parse_header(self, header_plus):
Expand Down

5 comments on commit fb08ecf

@D3V4N5H
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The status code in the response comes 200 instead of the expected 431

@digitalresistor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@D3V4N5H could you provide some sample code?

@digitalresistor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please open an issue.

@stevepiercy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #321

@digitalresistor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stevepiercy, didn't see the issue.

That being said, added test: c85f7e3 and it passes with flying colours. This commit is not the issue, and as far as I can tell waitress is not returning the 200 code.

Please sign in to comment.