Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore response body if status code doesn't allow a body #202

Merged
merged 6 commits into from Sep 6, 2018
Prev

Rewrite removal of item from response_headers

We don't want to delete items from a list while iterating over the list.
  • Loading branch information...
bertjwregeer committed Sep 6, 2018
commit 508570ddd1cfda05500e17dccf3f52c0fe8b0ad2
@@ -200,39 +200,41 @@ def build_response_header(self):
version = self.version
# Figure out whether the connection should be closed.
connection = self.request.headers.get('CONNECTION', '').lower()
response_headers = self.response_headers
response_headers = []
content_length_header = None
date_header = None
server_header = None
connection_close_header = None

for i, (headername, headerval) in enumerate(response_headers):
for (headername, headerval) in self.response_headers:
headername = '-'.join(
[x.capitalize() for x in headername.split('-')]
)

if headername == 'Content-Length':
if self.has_body:
content_length_header = headerval
else:
del response_headers[i]
continue # pragma: no cover

This comment has been minimized.

Copy link
@mmerickel

mmerickel Sep 1, 2018

Member

deletion while iteration - sure this is ok?

This comment has been minimized.

Copy link
@bertjwregeer

This comment has been minimized.

Copy link
@bertjwregeer

bertjwregeer Sep 4, 2018

Author Member

I guess I need to find out if enumerate creates a copy of the list or if it iterates over the list internally.

This comment has been minimized.

Copy link
@bertjwregeer

bertjwregeer Sep 4, 2018

Author Member

This will break in subtle ways if there is more than one Content-Length header set (which is illegal, but possible)

This comment has been minimized.

Copy link
@tseaver

tseaver Sep 4, 2018

Member

FWIW, enumerate has generator semantics (see PEP 279).

This comment has been minimized.

Copy link
@bertjwregeer

bertjwregeer Sep 4, 2018

Author Member

Then the other code and this code are both subtly broken... will fix


if headername == 'Date':
date_header = headerval

if headername == 'Server':
server_header = headerval

if headername == 'Connection':
connection_close_header = headerval.lower()
# replace with properly capitalized version
response_headers[i] = (headername, headerval)
response_headers.append((headername, headerval))

if (
content_length_header is None and
self.content_length is not None and
self.has_body
):
content_length_header = str(self.content_length)
self.response_headers.append(
response_headers.append(
('Content-Length', content_length_header)
)

@@ -272,6 +274,7 @@ def close_on_finish():
# Set the Server and Date field, if not yet specified. This is needed
# if the server is used as a proxy.
ident = self.channel.server.adj.ident

if not server_header:
if ident:
response_headers.append(('Server', ident))
@@ -281,20 +284,28 @@ def close_on_finish():
if not date_header:
response_headers.append(('Date', build_http_date(self.start_time)))

self.response_headers = response_headers

This comment has been minimized.

Copy link
@bertjwregeer

bertjwregeer Sep 6, 2018

Author Member

Not strictly required in this function, but required for testing.


first_line = 'HTTP/%s %s' % (self.version, self.status)
# NB: sorting headers needs to preserve same-named-header order
# as per RFC 2616 section 4.2; thus the key=lambda x: x[0] here;
# rely on stable sort to keep relative position of same-named headers
next_lines = ['%s: %s' % hv for hv in sorted(
self.response_headers, key=lambda x: x[0])]
self.response_headers, key=lambda x: x[0])]
lines = [first_line] + next_lines
res = '%s\r\n\r\n' % '\r\n'.join(lines)

return tobytes(res)

def remove_content_length_header(self):
for i, (header_name, header_value) in enumerate(self.response_headers):
response_headers = []

for header_name, header_value in self.response_headers:
if header_name.lower() == 'content-length':
del self.response_headers[i]
continue # pragma: nocover
response_headers.append((header_name, header_value))

self.response_headers = response_headers

def start(self):
self.start_time = time.time()
@@ -308,6 +308,12 @@ def test_remove_content_length_header(self):
inst.remove_content_length_header()
self.assertEqual(inst.response_headers, [])

def test_remove_content_length_header_with_other(self):
inst = self._makeOne()
inst.response_headers = [('Content-Length', '70'), ('Content-Type', 'text/html')]
inst.remove_content_length_header()
self.assertEqual(inst.response_headers, [('Content-Type', 'text/html')])

def test_start(self):
inst = self._makeOne()
inst.start()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.