Merge 98fbc88 into deb8474

CHANGES.rst

@@ -54,6 +54,10 @@ Fixes
 - Update cookie expiration method in a way Firefox 63+ understands.
   (`#405 <https://github.com/zopefoundation/Zope/pull/405>`_)
 
+- Fix closing newly created request before processing it after a retryable
+  error has occurred.
+  (`#413 <https://github.com/zopefoundation/Zope/issues/413>`_)
+
 
 4.0b7 (2018-10-30)
 ------------------

src/ZPublisher/WSGIPublisher.py

@@ -252,15 +252,23 @@ def publish_module(environ, start_response,
 
         environ['PATH_INFO'] = path_info
     with closing(BytesIO()) as stdout, closing(BytesIO()) as stderr:
-        response = (_response if _response is not None else
-                    _response_factory(stdout=stdout, stderr=stderr))
-        response._http_version = environ['SERVER_PROTOCOL'].split('/')[1]
-        response._server_version = environ.get('SERVER_SOFTWARE')
-
-        request = (_request if _request is not None else
-                   _request_factory(environ['wsgi.input'], environ, response))
-
-        for i in range(getattr(request, 'retry_max_count', 3) + 1):
+        new_response = (
+            _response
+            if _response is not None
+            else _response_factory(stdout=stdout, stderr=stderr))
+        new_response._http_version = environ['SERVER_PROTOCOL'].split('/')[1]
+        new_response._server_version = environ.get('SERVER_SOFTWARE')
+
+        new_request = (
+            _request
+            if _request is not None
+            else _request_factory(environ['wsgi.input'],
+                                  environ,
+                                  new_response))
+
+        for i in range(getattr(new_request, 'retry_max_count', 3) + 1):
+            request = new_request
+            response = new_response
             setRequest(request)
             try:
                 with load_app(module_info) as new_mod_info:
@@ -270,9 +278,7 @@ def publish_module(environ, start_response,
             except (ConflictError, TransientError) as exc:
                 if request.supports_retry():
                     new_request = request.retry()
-                    request.close()
-                    request = new_request
-                    response = new_request.response
+                    new_response = new_request.response
                 else:
                     raise
             finally:

src/ZPublisher/tests/test_WSGIPublisher.py

@@ -15,6 +15,7 @@
 import unittest
 
 import transaction
+from ZODB.POSException import ConflictError
 from zope.interface.common.interfaces import IException
 from zope.publisher.interfaces import INotFound
 from zope.security.interfaces import IUnauthorized
@@ -447,6 +448,27 @@ def _request_factory(stdin, environ, response):
                       _request_factory=_request_factory)
         self.assertTrue(_request._closed)
 
+    def test_handle_ConflictError(self):
+        environ = self._makeEnviron()
+        start_response = DummyCallable()
+        def _publish(request, module_info):
+            if request.retry_count < 1:
+                raise ConflictError
+            response = DummyResponse()
+            response.setBody(request.other.get('method'))
+            return response
+
+        try:
+            from ZPublisher.HTTPRequest import HTTPRequest
+            original_retry_max_count =  HTTPRequest.retry_max_count
+            HTTPRequest.retry_max_count = 1
+            # After the retry the request has a filled `other` dict, thus the
+            # new request is not closed before processing it:
+            self.assertEqual(
+                self._callFUT(environ, start_response, _publish), (b'', 'GET'))
+        finally:
+            HTTPRequest.retry_max_count = original_retry_max_count
+
     def testCustomExceptionViewUnauthorized(self):
         from AccessControl import Unauthorized
         registerExceptionView(IUnauthorized)
@@ -506,6 +528,7 @@ def testCustomExceptionViewBadRequest(self):
         body = b''.join(app_iter)
         self.assertEqual(start_response._called_with[0][0], '400 Bad Request')
         self.assertTrue(b'Exception View: BadRequest' in body)
+        unregisterExceptionView(IException)
 
     def testCustomExceptionViewInternalError(self):
         from zExceptions import InternalError
@@ -519,6 +542,7 @@ def testCustomExceptionViewInternalError(self):
         self.assertEqual(
             start_response._called_with[0][0], '500 Internal Server Error')
         self.assertTrue(b'Exception View: InternalError' in body)
+        unregisterExceptionView(IException)
 
     def testRedirectExceptionView(self):
         from zExceptions import Redirect
@@ -534,6 +558,7 @@ def testRedirectExceptionView(self):
         self.assertTrue(b'Exception View: Redirect' in body)
         headers = dict(headers)
         self.assertEqual(headers['Location'], 'http://localhost:9/')
+        unregisterExceptionView(IException)
 
     def testHandleErrorsFalseBypassesExceptionResponse(self):
         from AccessControl import Unauthorized
@@ -644,6 +669,18 @@ def registerExceptionView(for_):
         name=u'index.html',
     )
 
+def unregisterExceptionView(for_):
+    from zope.interface import Interface
+    from zope.component import getGlobalSiteManager
+    from zope.publisher.interfaces.browser import IDefaultBrowserLayer
+    gsm = getGlobalSiteManager()
+    gsm.unregisterAdapter(
+        CustomExceptionView,
+        required=(for_, IDefaultBrowserLayer),
+        provided=Interface,
+        name=u'index.html',
+    )
+
 
 class DummyRequest(dict):
     _processedInputs = False