HTTPResponse: default content type to text/plain ( 4.x ) (#1076)

* HTTPResponse: default content type to text/plain * drop implicit detection of content type * drop support for returning HTML with ``response.setBody((title, body))`` * - add a change log entry and improve wording [ci skip] * - force GHA test run * Fix GHA: ubuntu-latest no longer contains Python 2.7 up to 3.6 Co-authored-by: Jens Vagelpohl <jens@plyp.com>
zopefoundation · Dec 16, 2022 · c43b3bb · c43b3bb
1 parent d653088
commit c43b3bb
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 75 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -18,8 +18,8 @@ jobs:
       fail-fast: false
       matrix:
         os:
-        - ubuntu
-        - windows
+        - ["ubuntu", "ubuntu-20.04"]
+        - ["windows", "windows-latest"]
         config:
         # [Python version, tox env]
         - ["3.8",   "lint"]
@@ -32,21 +32,22 @@ jobs:
         - ["3.8",   "docs"]
         - ["3.8",   "coverage"]
         exclude:
-          - { os: windows, config: ["3.8",   "lint"] }
-          - { os: windows, config: ["3.8",   "docs"] }
-          - { os: windows, config: ["3.8",   "coverage"] }
-          - { os: windows, config: ["2.7",   "py27-zserver"] }
+          - { os: ["windows", "windows-latest"], config: ["3.8",   "lint"] }
+          - { os: ["windows", "windows-latest"], config: ["3.8",   "docs"] }
+          - { os: ["windows", "windows-latest"], config: ["3.8",   "coverage"] }
+          - { os: ["windows", "windows-latest"], config: ["2.7",   "py27-zserver"] }
 
-    runs-on: ${{ matrix.os }}-latest
-    name: ${{ matrix.os }}-${{ matrix.config[1] }}
+    runs-on: ${{ matrix.os[1] }}
+    if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name
+    name: ${{ matrix.os[0] }}-${{ matrix.config[1] }}
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - name: Set up Python
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.config[0] }}
     - name: Pip cache
-      uses: actions/cache@v2
+      uses: actions/cache@v3
       with:
         path: ~/.cache/pip
         key: ${{ runner.os }}-pip-${{ matrix.config[0] }}-${{ hashFiles('setup.*', 'tox.ini') }}

diff --git a/CHANGES.rst b/CHANGES.rst
@@ -6,9 +6,15 @@ These are all the changes in Zope 4, starting with the alpha releases.
 The change log for the previous version, Zope 2.13, is at
 https://zope.readthedocs.io/en/2.13/CHANGES.html
 
+
 4.8.4 (unreleased)
 ------------------
 
+- Set the published default ``Content-Type`` header to ``text/plain``
+  if none has been set explicitly to prevent a cross-site scripting attack.
+  Also remove the old behavior of constructing an HTML page for published
+  methods returning a two-item tuple.
+
 - Update dependencies to the latest releases for each supported Python version.
 
 - Make Products.PageTemplate engine compatible with Chameleon 3.10.

diff --git a/docs/zdgbook/ObjectPublishing.rst b/docs/zdgbook/ObjectPublishing.rst
@@ -271,25 +271,6 @@ the chapter.
   Depending on the used client, it looks like nothing happens.
 
 
-Optionally, the published method can return a tuple with
-the title and the body of the response. In this case, the publisher
-returns a generated HTML page, with the first item of the tuple used
-for the value of the HTML ``title`` tag of the page, and the second
-item as the content of the HTML ``body`` tag.
-
-
-For example a response of::
-
-  ("my_title", "my_text")
-
-
-is turned into this HTML page::
-
-  <html>
-  <head><title>my_title</title></head>
-  <body>my_text</body>
-  </html>
-
 
 Controlling Base HREF
 ---------------------
@@ -358,10 +339,10 @@ base with a *base* tag in your ``index_html`` method output.
 Response Headers
 ----------------
 
-The publisher and the web server take care of setting response headers
-such as *Content-Length* and *Content-Type*. Later in the chapter
-you'll find out how to control these headers and also how exceptions
-are used to set the HTTP response code.
+The publisher and the web server take care of setting the *Content-Length*
+response header. Later in the chapter you'll find out how to control
+response headers and also how exceptions are used to set the HTTP
+response code.
 
 
 Pre-Traversal Hook
@@ -1111,7 +1092,7 @@ Known issues and caveats
 
    - unrecognized directives are silently ignored
 
-   - if a request paramater contains several converter directives, the
+   - if a request parameter contains several converter directives, the
      leftmost wins
 
    - if a request paramter contains several encoding directives, the
@@ -1153,7 +1134,7 @@ Exceptions
 ----------
 
 When the object publisher catches an unhandled exception, it tries to
-match it with a set of predifined exceptions coming from the
+match it with a set of predefined exceptions coming from the
 **zExceptions** package, such as **HTTPNoContent**, **HTTPNotFound**,
 **HTTPUnauthorized**.
 
@@ -1272,6 +1253,7 @@ being called from the web. Consider this function::
       ...
       result = ...
       if REQUEST is not None:
+          REQUEST.RESPONSE.setHeader("Content-Type", "text/html")
           return "<html><p>Result: %s </p></html>" % result
       return result
 

diff --git a/src/App/Management.py b/src/App/Management.py
@@ -198,6 +198,7 @@ def manage_zmi_logout(self, REQUEST, RESPONSE):
         realm = RESPONSE.realm
         RESPONSE.setStatus(401)
         RESPONSE.setHeader('WWW-Authenticate', 'basic realm="%s"' % realm, 1)
+        RESPONSE.setHeader('Content-Type', 'text/html')
         RESPONSE.setBody("""<html>
 <head><title>Logout</title></head>
 <body>

diff --git a/src/ZPublisher/HTTPResponse.py b/src/ZPublisher/HTTPResponse.py
@@ -520,9 +520,6 @@ def setBody(self, body, title='', is_error=False, lock=None):
         You can also specify a title, in which case the title and body
         will be wrapped up in html, head, title, and body tags.
 
-        If the body is a 2-element tuple, then it will be treated
-        as (title,body)
-
         If body is unicode, encode it.
 
         If body is not a string or unicode, but has an 'asHTML' method, use
@@ -543,11 +540,11 @@ def setBody(self, body, title='', is_error=False, lock=None):
         if not body:
             return self
 
-        if isinstance(body, tuple) and len(body) == 2:
-            title, body = body
+        content_type = self.headers.get('content-type')
 
         if hasattr(body, 'asHTML'):
             body = body.asHTML()
+            content_type = 'text/html'
 
         if isinstance(body, text_type):
             body = self._encode_unicode(body)
@@ -569,6 +566,7 @@ def setBody(self, body, title='', is_error=False, lock=None):
         else:
             if title:
                 title = text_type(title)
+                content_type = 'text/html'
                 if not is_error:
                     self.body = body = self._html(
                         title, body.decode(self.charset)).encode(self.charset)
@@ -578,13 +576,8 @@ def setBody(self, body, title='', is_error=False, lock=None):
             else:
                 self.body = body
 
-        content_type = self.headers.get('content-type')
-
         if content_type is None:
-            if self.isHTML(body):
-                content_type = 'text/html; charset=%s' % self.charset
-            else:
-                content_type = 'text/plain; charset=%s' % self.charset
+            content_type = 'text/plain; charset=%s' % self.charset
             self.setHeader('content-type', content_type)
         else:
             if content_type.startswith('text/') and \
@@ -917,10 +910,11 @@ def exception(self, fatal=0, info=None, abort=1):
                 b = '<unprintable %s object>' % type(b).__name__
 
         if fatal and t is SystemExit and v.code == 0:
+            self.setHeader('content-type', 'text/html')
             body = self.setBody(
-                (text_type(t),
-                 'Zope has exited normally.<p>'
-                 + self._traceback(t, v, tb) + '</p>'),
+                'Zope has exited normally.<p>'
+                + self._traceback(t, v, tb) + '</p>',
+                title=t,
                 is_error=True)
         else:
             try:
@@ -929,10 +923,11 @@ def exception(self, fatal=0, info=None, abort=1):
                 match = None
 
             if match is None:
+                self.setHeader('content-type', 'text/html')
                 body = self.setBody(
-                    (text_type(t),
-                     'Sorry, a site error occurred.<p>'
-                     + self._traceback(t, v, tb) + '</p>'),
+                    'Sorry, a site error occurred.<p>'
+                    + self._traceback(t, v, tb) + '</p>',
+                    title=t,
                     is_error=True)
             elif self.isHTML(b):
                 # error is an HTML document, not just a snippet of html
@@ -943,8 +938,8 @@ def exception(self, fatal=0, info=None, abort=1):
                     body = self.setBody(b, is_error=True)
             else:
                 body = self.setBody(
-                    (text_type(t),
-                     b + self._traceback(t, '(see above)', tb, 0)),
+                    b + self._traceback(t, '(see above)', tb, 0),
+                    title=t,
                     is_error=True)
         del tb
         return body

diff --git a/src/ZPublisher/tests/testHTTPResponse.py b/src/ZPublisher/tests/testHTTPResponse.py
@@ -562,25 +562,10 @@ def test_setBody_empty_unchanged(self):
         self.assertEqual(response.getHeader('Content-Type'), None)
         self.assertEqual(response.getHeader('Content-Length'), None)
 
-    def test_setBody_2_tuple_wo_is_error_converted_to_HTML(self):
-        EXPECTED = (b"<html>\n"
-                    b"<head>\n<title>TITLE</title>\n</head>\n"
-                    b"<body>\nBODY\n</body>\n"
-                    b"</html>\n")
+    def test_setBody_with_is_error_converted_to_Site_Error(self):
         response = self._makeOne()
         response.body = b'BEFORE'
-        result = response.setBody(('TITLE', b'BODY'))
-        self.assertTrue(result)
-        self.assertEqual(response.body, EXPECTED)
-        self.assertEqual(response.getHeader('Content-Type'),
-                         'text/html; charset=utf-8')
-        self.assertEqual(response.getHeader('Content-Length'),
-                         str(len(EXPECTED)))
-
-    def test_setBody_2_tuple_w_is_error_converted_to_Site_Error(self):
-        response = self._makeOne()
-        response.body = b'BEFORE'
-        result = response.setBody(('TITLE', b'BODY'), is_error=True)
+        result = response.setBody(b'BODY', 'TITLE', is_error=True)
         self.assertTrue(result)
         self.assertFalse(b'BEFORE' in response.body)
         self.assertTrue(b'<h2>Site Error</h2>' in response.body)
@@ -598,14 +583,16 @@ def test_setBody_string_not_HTML(self):
                          'text/plain; charset=utf-8')
         self.assertEqual(response.getHeader('Content-Length'), '4')
 
-    def test_setBody_string_HTML(self):
+    def test_setBody_string_HTML_uses_text_plain(self):
         HTML = '<html><head></head><body></body></html>'
         response = self._makeOne()
         result = response.setBody(HTML)
         self.assertTrue(result)
         self.assertEqual(response.body, HTML.encode('utf-8'))
+        # content type is set as text/plain, even though body
+        # could be guessed as html
         self.assertEqual(response.getHeader('Content-Type'),
-                         'text/html; charset=utf-8')
+                         'text/plain; charset=utf-8')
         self.assertEqual(response.getHeader('Content-Length'), str(len(HTML)))
 
     def test_setBody_object_with_asHTML(self):
@@ -631,7 +618,7 @@ def test_setBody_object_with_unicode(self):
         self.assertTrue(result)
         self.assertEqual(response.body, ENCODED)
         self.assertEqual(response.getHeader('Content-Type'),
-                         'text/html; charset=utf-8')
+                         'text/plain; charset=utf-8')
         self.assertEqual(response.getHeader('Content-Length'),
                          str(len(ENCODED)))
 
@@ -648,6 +635,10 @@ def test_setBody_tuple(self):
         response = self._makeOne()
         response.setBody(('a',))
         self.assertEqual(b"('a',)", response.body)
+        response.setBody(('a', 'b'))
+        self.assertEqual(b"('a', 'b')", response.body)
+        response.setBody(('a', 'b', 'c'))
+        self.assertEqual(b"('a', 'b', 'c')", response.body)
 
     def test_setBody_calls_insertBase(self):
         response = self._makeOne()