Commit 17caae24 authored by Martijn Pieters's avatar Martijn Pieters

Fix Collector #1866: 304 responses should not have a content-length

parent 68deba35
...@@ -7,6 +7,8 @@ Zope Changes ...@@ -7,6 +7,8 @@ Zope Changes
Zope 2.9.8 (unreleased) Zope 2.9.8 (unreleased)
Bugs fixed Bugs fixed
- Collector #1866: a 304 HTTP status should not have a content length.
- Collector #2300: delimit *all* HTTP Response headers with CRLF. - Collector #2300: delimit *all* HTTP Response headers with CRLF.
......
...@@ -72,15 +72,16 @@ class ZServerHTTPResponse(HTTPResponse): ...@@ -72,15 +72,16 @@ class ZServerHTTPResponse(HTTPResponse):
self.status == 200: self.status == 200:
self.setStatus('nocontent') self.setStatus('nocontent')
# add content length if not streaming if self.status in (100, 101, 102, 204, 304):
if not headers.has_key('content-length') and \ # These responses should not have any body or Content-Length.
not self._streaming: # See RFC 2616 4.4 "Message Length".
self.setHeader('content-length',len(body)) body = ''
if 'content-length' in headers:
del headers['content-length']
content_length= headers.get('content-length', None) if 'content-type' in headers:
if content_length>0 : del headers['content-type']
self.setHeader('content-length', content_length) elif not headers.has_key('content-length') and not self._streaming:
self.setHeader('content-length', len(body))
headersl=[] headersl=[]
append=headersl.append append=headersl.append
...@@ -97,8 +98,7 @@ class ZServerHTTPResponse(HTTPResponse): ...@@ -97,8 +98,7 @@ class ZServerHTTPResponse(HTTPResponse):
append('Date: %s' % build_http_date(time.time())) append('Date: %s' % build_http_date(time.time()))
if self._http_version=='1.0': if self._http_version=='1.0':
if self._http_connection=='keep-alive' and \ if self._http_connection=='keep-alive':
self.headers.has_key('content-length'):
self.setHeader('Connection','Keep-Alive') self.setHeader('Connection','Keep-Alive')
else: else:
self.setHeader('Connection','close') self.setHeader('Connection','close')
...@@ -108,12 +108,10 @@ class ZServerHTTPResponse(HTTPResponse): ...@@ -108,12 +108,10 @@ class ZServerHTTPResponse(HTTPResponse):
if self._http_version=='1.1': if self._http_version=='1.1':
if self._http_connection=='close': if self._http_connection=='close':
self.setHeader('Connection','close') self.setHeader('Connection','close')
elif not self.headers.has_key('content-length'): elif (not self.headers.has_key('content-length') and
if self.http_chunk and self._streaming: self.http_chunk and self._streaming):
self.setHeader('Transfer-Encoding','chunked') self.setHeader('Transfer-Encoding','chunked')
self._chunking=1 self._chunking=1
else:
self.setHeader('Connection','close')
headers = headers.items() headers = headers.items()
for line in self.accumulated_headers.splitlines(): for line in self.accumulated_headers.splitlines():
......
...@@ -124,7 +124,149 @@ class ZServerHTTPResponseTestCase(unittest.TestCase): ...@@ -124,7 +124,149 @@ class ZServerHTTPResponseTestCase(unittest.TestCase):
self.assertTrue('Multilined: eggs\r\n\tham\r\n' in headers) self.assertTrue('Multilined: eggs\r\n\tham\r\n' in headers)
self.assertTrue('Foo-Bar: bar\r\n\tbaz\r\n' in headers) self.assertTrue('Foo-Bar: bar\r\n\tbaz\r\n' in headers)
def _assertResponsesAreEqual(self, got, expected):
got = got.split('\r\n')
# Sort the headers into alphabetical order.
headers = got[1:got.index('')]
headers.sort()
got[1:len(headers)+1] = headers
# Compare line by line.
for n in range(len(expected)):
if expected[n].endswith('...'):
m = len(expected[n]) - 3
self.assertEqual(got[n][:m], expected[n][:m])
else:
self.assertEqual(got[n], expected[n])
self.assertEqual(len(got), len(expected))
def test_emptyResponse(self):
# Empty repsonses have no Content-Length.
response = self._makeOne()
self._assertResponsesAreEqual(str(response),
('HTTP/1.0 204 No Content',
'Connection: close',
'Date: ...',
'Server: ...',
'',
''))
def test_304(self):
# Now we set the status to 304. 304 responses, according to RFC 2616,
# should not have a content-length header. __str__ should not add it
# back in if it is missing.
response = self._makeOne()
response.setStatus(304)
self._assertResponsesAreEqual(str(response),
('HTTP/1.0 304 Not Modified',
'Connection: close',
'Date: ...',
'Server: ...',
'',
''))
def test_304ContentLength(self):
# __str__ should strip out Content-Length
response = self._makeOne()
response.setStatus(304)
response.setHeader('content-length', '123')
self._assertResponsesAreEqual(str(response),
('HTTP/1.0 304 Not Modified',
'Connection: close',
'Date: ...',
'Server: ...',
'',
''))
def test_304ContentType(self):
# __str__ should strip out Content-Type
response = self._makeOne()
response.setStatus(304)
response.setHeader('content-type', 'text/plain')
self._assertResponsesAreEqual(str(response),
('HTTP/1.0 304 Not Modified',
'Connection: close',
'Date: ...',
'Server: ...',
'',
''))
def test_304ExplicitKeepAlive(self):
# Explicit keep-alive connection header for HTTP 1.0.
response = self._makeOne()
response._http_connection = 'keep-alive'
response.setStatus(304)
self._assertResponsesAreEqual(str(response),
('HTTP/1.0 304 Not Modified',
'Connection: Keep-Alive',
'Date: ...',
'Server: ...',
'',
''))
def test_304ImplicitKeepAlive(self):
# Keep-alive is implicit for HTTP 1.1.
response = self._makeOne()
response._http_version = '1.1'
response._http_connection = 'keep-alive'
response.setStatus(304)
self._assertResponsesAreEqual(str(response),
('HTTP/1.1 304 Not Modified',
'Date: ...',
'Server: ...',
'',
''))
def test_contentLength(self):
# Check that __str__ adds in the correct Content-Length header.
response = self._makeOne()
response._http_version = '1.1'
response._http_connection = 'keep-alive'
response.body = '123456789'
response.setHeader('Content-Type', 'text/plain')
self._assertResponsesAreEqual(str(response),
('HTTP/1.1 200 OK',
'Content-Length: 9',
'Content-Type: text/plain',
'Date: ...',
'Server: ...',
'',
'123456789'))
def test_emptyBody(self):
# Check that a response with an empty message body returns a
# Content-Length of 0. A common example of this is a 302 redirect.
response = self._makeOne()
response._http_version = '1.1'
response._http_connection = 'keep-alive'
response.redirect('somewhere')
self._assertResponsesAreEqual(str(response),
('HTTP/1.1 302 Moved Temporarily',
'Content-Length: 0',
'Date: ...',
'Location: somewhere',
'Server: ...',
'',
''))
def test_HEAD(self):
# A response to a HEAD request will have a non zero content
# length and an empty body.
response = self._makeOne()
response._http_version = '1.1'
response._http_connection = 'keep-alive'
response.setHeader('Content-Type', 'text/plain')
response.setHeader('Content-Length', 123)
self._assertResponsesAreEqual(str(response),
('HTTP/1.1 200 OK',
'Content-Length: 123',
'Content-Type: text/plain',
'Date: ...',
'Server: ...',
'',
''))
def test_suite(): def test_suite():
suite = unittest.TestSuite() suite = unittest.TestSuite()
suite.addTests(( suite.addTests((
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment