launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-head-requests into launchpad:master.

Commit message:
Fix setting of empty responses on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/402586

In a few situations (particularly HEAD requests) we use `HTTPResponse.setResult` to set an empty response.  However, if this is passed a Unicode string, then `HTTPResponse._implicitResult` requires the Content-Type to be one of a limited subset, which not all our responses are.  Explicitly pass a byte string instead.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-head-requests into launchpad:master.

diff --git a/lib/lp/code/stories/feeds/xx-revision-atom.txt b/lib/lp/code/stories/feeds/xx-revision-atom.txt
index 65a6397..f74132b 100644
--- a/lib/lp/code/stories/feeds/xx-revision-atom.txt
+++ b/lib/lp/code/stories/feeds/xx-revision-atom.txt
@@ -112,6 +112,19 @@ of that team.
     <id>tag:launchpad.net,...:/revision/rev2</id>
     <id>tag:launchpad.net,...:/revision/rev1</id>
 
+A HEAD request works too.
+
+    >>> response = http(r"""
+    ... HEAD /~mike/revisions.atom HTTP/1.1
+    ... Host: feeds.launchpad.test
+    ... """)
+    >>> print(str(response).split('\n')[0])
+    HTTP/1.1 200 Ok
+    >>> print(response.getHeader('Content-Length'))
+    0
+    >>> print(six.ensure_text(response.getBody()))
+    <BLANKLINE>
+
 
 == Feed for a product's revisions ==
 
diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 37bf494..8517e7f 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -351,7 +351,7 @@ class LaunchpadBrowserPublication(
         if non_restricted_url is not None:
             location += '?production=%s' % quote(non_restricted_url)
 
-        request.response.setResult('')
+        request.response.setResult(b'')
         request.response.redirect(location, temporary_if_possible=True)
         # Quash further traversal.
         request.setTraversalStack([])
@@ -538,7 +538,7 @@ class LaunchpadBrowserPublication(
         # Don't render any content for a HEAD.  This was done
         # by zope.app.publication.browser.BrowserPublication
         if request.method == 'HEAD':
-            request.response.setResult('')
+            request.response.setResult(b'')
 
         try:
             getUtility(IStoreSelector).pop()
@@ -759,7 +759,7 @@ class LaunchpadBrowserPublication(
         #           or is it only required because of our customisations?
         #        - Andrew Bennetts, 2005-03-08
         if request.method == 'HEAD':
-            request.response.setResult('')
+            request.response.setResult(b'')
 
     def beginErrorHandlingTransaction(self, request, ob, note):
         """Hook for when a new view is started to handle an exception.