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

[Tres Seaver]

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

Changed:
  U   Zope/branches/2.10/doc/CHANGES.txt
  U   Zope/branches/2.10/lib/python/ZPublisher/HTTPResponse.py
  U   Zope/branches/2.10/lib/python/ZPublisher/tests/testHTTPResponse.py

-=-
Modified: Zope/branches/2.10/doc/CHANGES.txt
===================================================================
--- Zope/branches/2.10/doc/CHANGES.txt	2008-10-23 19:15:24 UTC (rev 92517)
+++ Zope/branches/2.10/doc/CHANGES.txt	2008-10-23 19:20:01 UTC (rev 92518)
@@ -8,6 +8,9 @@
 
     Bugs fixed
 
+      - Ensure that response header values cannot embed CRLF pairs, which
+        violate the HTTP spec (RFC 2616).
+
       - Launchpad #282677: fixed implementation of guarded_map and
         provided tests and implementation for guarded_zip
         (RestrictedPython).

Modified: Zope/branches/2.10/lib/python/ZPublisher/HTTPResponse.py
===================================================================
--- Zope/branches/2.10/lib/python/ZPublisher/HTTPResponse.py	2008-10-23 19:15:24 UTC (rev 92517)
+++ Zope/branches/2.10/lib/python/ZPublisher/HTTPResponse.py	2008-10-23 19:20:01 UTC (rev 92518)
@@ -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.
@@ -248,8 +252,7 @@
         the previous value set for the header, if one exists. If the
         literal flag is true, the case of the header name is preserved,
         otherwise the header name will be lowercased.'''
-        name = str(name)
-        value = str(value)
+        name, value = _scrubHeader(name, value)
         key = name.lower()
         if accumulate_header(key):
             self.accumulated_headers = (
@@ -274,8 +277,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))
 
@@ -585,8 +587,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/2.10/lib/python/ZPublisher/tests/testHTTPResponse.py
===================================================================
--- Zope/branches/2.10/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-23 19:15:24 UTC (rev 92517)
+++ Zope/branches/2.10/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-23 19:20:01 UTC (rev 92518)
@@ -145,7 +145,41 @@
         response = self._makeOne(body=xml, headers={'content-type': 'text/xml; charset=iso-8859-15'})
         self.assertEqual(xml==response.body, True)
 
+    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'))