[Zope-Checkins] SVN: Zope/trunk/lib/python/ZPublisher/ Actually land the CRLF response header fix.

[Tres Seaver]

Log message for revision 92625:
  Actually land the CRLF response header fix.

Changed:
  U   Zope/trunk/lib/python/ZPublisher/HTTPResponse.py
  U   Zope/trunk/lib/python/ZPublisher/tests/testHTTPResponse.py

-=-
Modified: Zope/trunk/lib/python/ZPublisher/HTTPResponse.py
===================================================================
--- Zope/trunk/lib/python/ZPublisher/HTTPResponse.py	2008-10-27 19:18:53 UTC (rev 92624)
+++ Zope/trunk/lib/python/ZPublisher/HTTPResponse.py	2008-10-27 19:19:42 UTC (rev 92625)
@@ -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.
@@ -242,15 +246,14 @@
         if lock:
              self._locked_status = 1
 
-    def setHeader(self, name, value, literal=0):
+    def setHeader(self, name, value, literal=0, scrubbed=False):
         '''\
         Sets an HTTP return header "name" with value "value", clearing
         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)
+        if not scrubbed:
+            name, value = _scrubHeader(name, value)
         key = name.lower()
         if accumulate_header(key):
             self.accumulated_headers = (
@@ -275,8 +278,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\r\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):
@@ -594,7 +596,7 @@
             h = "%s%s\r\n\t%s" % (h,delimiter,value)
         else:
             h = value
-        self.setHeader(name,h)
+        self.setHeader(name,h, scrubbed=True)
 
     def isHTML(self, s):
         s = s.lstrip()

Modified: Zope/trunk/lib/python/ZPublisher/tests/testHTTPResponse.py
===================================================================
--- Zope/trunk/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-27 19:18:53 UTC (rev 92624)
+++ Zope/trunk/lib/python/ZPublisher/tests/testHTTPResponse.py	2008-10-27 19:19:42 UTC (rev 92625)
@@ -145,7 +145,42 @@
         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\r\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"\r\n' +
+                'Set-Cookie: '
+                'violation="http://www.ietf.org/rfc/rfc2616.txt"\r\n')
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(HTTPResponseTests, 'test'))