[Zope-Checkins] SVN: Zope/branches/Zope-2_8-branch/ Ensure that response header values cannot embed CRLF pairs, which violate the

[Tres Seaver]

Log message for revision 92516:
  Ensure that response header values cannot embed CRLF pairs, which violate the
  HTTP spec (RFC 2616).
  

Changed:
  U   Zope/branches/Zope-2_8-branch/doc/CHANGES.txt
  U   Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/HTTPResponse.py
  U   Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/tests/testHTTPResponse.py

-=-
Modified: Zope/branches/Zope-2_8-branch/doc/CHANGES.txt
===================================================================
--- Zope/branches/Zope-2_8-branch/doc/CHANGES.txt	2008-10-23 18:49:44 UTC (rev 92515)
+++ Zope/branches/Zope-2_8-branch/doc/CHANGES.txt	2008-10-23 19:14:53 UTC (rev 92516)
@@ -8,6 +8,9 @@
 
     Bugs fixed
 
+      - Ensure that response header values cannot embed CRLF pairs, which
+        violate the HTTP spec (RFC 2616).
+
       - 'AccessControl.ZopeGuards.guarded_import' mapped some Unauthorized
         exceptions onto ImportErrors:  don't do that!  Also, removed
         mutable defaults from argument list, improved tests.

Modified: Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/HTTPResponse.py
===================================================================
--- Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/HTTPResponse.py	2008-10-23 18:49:44 UTC (rev 92515)
+++ Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/HTTPResponse.py	2008-10-23 19:14:53 UTC (rev 92516)
@@ -122,7 +122,11 @@
 if otherTypes:
     uncompressableMimeMajorTypes += tuple(otherTypes.split(','))
 
+_CRLF = re.compile(r'\r[\n]?')
 
+def _scrubHeader(name, value):
+    return ''.join(_CRLF.split(str(name))), ''.join(_CRLF.split(str(value)))
+
 class HTTPResponse(BaseResponse):
     """\
     An object representation of an HTTP response.
@@ -249,8 +253,7 @@
         literal flag is true, the case of the header name is preserved,
         otherwise word-capitalization will be performed on the header
         name on output.'''
-        name = str(name)
-        value = str(value)
+        name, value = _scrubHeader(name, value)
         key = name.lower()
         if accumulate_header(key):
             self.accumulated_headers = (
@@ -263,8 +266,7 @@
         '''\
         Set a new HTTP return header with the given value, while retaining
         any previously set headers with the same name.'''
-        name = str(name)
-        value = str(value)
+        name, value = _scrubHeader(name, value)
         self.accumulated_headers = (
             "%s%s: %s\n" % (self.accumulated_headers, name, value))
 
@@ -547,8 +549,8 @@
         Sets an HTTP return header "name" with value "value",
         appending it following a comma if there was a previous value
         set for the header. '''
-        name = str(name).lower()
-        value = str(value)
+        name, value = _scrubHeader(name, value)
+        name = name.lower()
 
         headers = self.headers
         if headers.has_key(name):

Modified: Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/tests/testHTTPResponse.py
===================================================================
--- Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-23 18:49:44 UTC (rev 92515)
+++ Zope/branches/Zope-2_8-branch/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-23 19:14:53 UTC (rev 92516)
@@ -80,7 +80,40 @@
         response.setStatus(ResourceLockedError)
         self.assertEqual(response.status, 423)
 
+    def test_addHeader_drops_CRLF(self):
+        # RFC2616 disallows CRLF in a header value.
+        response = self._makeOne()
+        response.addHeader('Location',
+                           'http://www.ietf.org/rfc/\r\nrfc2616.txt')
+        self.assertEqual(response.accumulated_headers,
+                         'Location: http://www.ietf.org/rfc/rfc2616.txt\n')
 
+    def test_appendHeader_drops_CRLF(self):
+        # RFC2616 disallows CRLF in a header value.
+        response = self._makeOne()
+        response.appendHeader('Location',
+                               'http://www.ietf.org/rfc/\r\nrfc2616.txt')
+        self.assertEqual(response.headers['location'],
+                         'http://www.ietf.org/rfc/rfc2616.txt')
+
+    def test_setHeader_drops_CRLF(self):
+        # RFC2616 disallows CRLF in a header value.
+        response = self._makeOne()
+        response.setHeader('Location',
+                           'http://www.ietf.org/rfc/\r\nrfc2616.txt')
+        self.assertEqual(response.headers['location'],
+                         'http://www.ietf.org/rfc/rfc2616.txt')
+
+    def test_setHeader_drops_CRLF_when_accumulating(self):
+        # RFC2616 disallows CRLF in a header value.
+        response = self._makeOne()
+        response.setHeader('Set-Cookie', 'allowed="OK"')
+        response.setHeader('Set-Cookie',
+                       'violation="http://www.ietf.org/rfc/\r\nrfc2616.txt"')
+        self.assertEqual(response.accumulated_headers,
+                'Set-Cookie: allowed="OK"\n' +
+                'Set-Cookie: violation="http://www.ietf.org/rfc/rfc2616.txt"\n')
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(HTTPResponseTests, 'test'))