Log message for revision 73801:
  Fix Collector #1866: 304 responses should not have a content-length

Changed:
  U   Zope/branches/2.9/doc/CHANGES.txt
  U   Zope/branches/2.9/lib/python/ZServer/HTTPResponse.py
  U   Zope/branches/2.9/lib/python/ZServer/tests/test_responses.py

-=-
Modified: Zope/branches/2.9/doc/CHANGES.txt
===================================================================
--- Zope/branches/2.9/doc/CHANGES.txt   2007-03-27 16:16:42 UTC (rev 73800)
+++ Zope/branches/2.9/doc/CHANGES.txt   2007-03-27 18:45:47 UTC (rev 73801)
@@ -7,6 +7,8 @@
   Zope 2.9.8 (unreleased)
 
    Bugs fixed
+
+      - Collector #1866: a 304 HTTP status should not have a content length.
   
       - Collector #2300: delimit *all* HTTP Response headers with CRLF.
 

Modified: Zope/branches/2.9/lib/python/ZServer/HTTPResponse.py
===================================================================
--- Zope/branches/2.9/lib/python/ZServer/HTTPResponse.py        2007-03-27 
16:16:42 UTC (rev 73800)
+++ Zope/branches/2.9/lib/python/ZServer/HTTPResponse.py        2007-03-27 
18:45:47 UTC (rev 73801)
@@ -72,16 +72,17 @@
                 self.status == 200:
             self.setStatus('nocontent')
 
-        # add content length if not streaming
-        if not headers.has_key('content-length') and \
-                not self._streaming:
-            self.setHeader('content-length',len(body))
+        if self.status in (100, 101, 102, 204, 304):
+            # These responses should not have any body or Content-Length.
+            # See RFC 2616 4.4 "Message Length".
+            body = ''
+            if 'content-length' in headers:
+                del headers['content-length']
+            if 'content-type' in headers:
+                del headers['content-type']
+        elif not headers.has_key('content-length') and not self._streaming:
+            self.setHeader('content-length', len(body))
 
-
-        content_length= headers.get('content-length', None)
-        if content_length>0 :
-            self.setHeader('content-length', content_length)
-
         headersl=[]
         append=headersl.append
 
@@ -97,8 +98,7 @@
         append('Date: %s' % build_http_date(time.time()))
 
         if self._http_version=='1.0':
-            if self._http_connection=='keep-alive' and \
-                    self.headers.has_key('content-length'):
+            if self._http_connection=='keep-alive':
                 self.setHeader('Connection','Keep-Alive')
             else:
                 self.setHeader('Connection','close')
@@ -108,12 +108,10 @@
         if self._http_version=='1.1':
             if self._http_connection=='close':
                 self.setHeader('Connection','close')
-            elif not self.headers.has_key('content-length'):
-                if self.http_chunk and self._streaming:
-                    self.setHeader('Transfer-Encoding','chunked')
-                    self._chunking=1
-                else:
-                    self.setHeader('Connection','close')
+            elif (not self.headers.has_key('content-length') and 
+                  self.http_chunk and self._streaming):
+                self.setHeader('Transfer-Encoding','chunked')
+                self._chunking=1
                     
         headers = headers.items()
         for line in self.accumulated_headers.splitlines():

Modified: Zope/branches/2.9/lib/python/ZServer/tests/test_responses.py
===================================================================
--- Zope/branches/2.9/lib/python/ZServer/tests/test_responses.py        
2007-03-27 16:16:42 UTC (rev 73800)
+++ Zope/branches/2.9/lib/python/ZServer/tests/test_responses.py        
2007-03-27 18:45:47 UTC (rev 73801)
@@ -124,7 +124,149 @@
 
         self.assertTrue('Multilined: eggs\r\n\tham\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():
     suite = unittest.TestSuite()
     suite.addTests((

_______________________________________________
Zope-Checkins maillist  -  Zope-Checkins@zope.org
http://mail.zope.org/mailman/listinfo/zope-checkins

Reply via email to