Merge pull request twisted#1027 from chrisbarber/trunk

Author: chrisbarber Reviewer: glyph Fixes: ticket:9468 twisted.web.twcgi.CGIProcessProtocol.processEnded(...) now handles an already-finished request, for example when request.connectionLost(...) was called previously.
ryanc-me · Aug 7, 2020 · 78032af · 78032af
2 parents 1b439fb + ec1025f
commit 78032af
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 8 deletions.
diff --git a/src/twisted/newsfragments/9468.bugfix b/src/twisted/newsfragments/9468.bugfix
@@ -0,0 +1 @@
+twisted.web.twcgi.CGIProcessProtocol.processEnded(...) now handles an already-finished request, for example when request.connectionLost(...) was called previously.
diff --git a/src/twisted/web/test/test_cgi.py b/src/twisted/web/test/test_cgi.py
@@ -12,11 +12,12 @@
 
 from twisted.trial import unittest
 from twisted.internet import address, reactor, interfaces, error
+from twisted.internet.error import ConnectionLost
 from twisted.python import util, failure, log
 from twisted.web.http import NOT_FOUND, INTERNAL_SERVER_ERROR
-from twisted.web import client, twcgi, server, resource, http_headers
+from twisted.web import client, http, twcgi, server, resource, http_headers
 from twisted.web.test._util import _render
-from twisted.web.test.test_web import DummyRequest
+from twisted.web.test.requesthelper import DummyRequest, DummyChannel
 
 DUMMY_CGI = '''\
 print("Header: OK")
@@ -485,6 +486,20 @@ def test_prematureEndOfHeaders(self):
         self.assertEqual(request.responseCode, INTERNAL_SERVER_ERROR)
 
 
+    def test_connectionLost(self):
+        """
+        Ensure that the CGI process ends cleanly when the request connection
+        is lost.
+        """
+        d = DummyChannel()
+        request = http.Request(d, True)
+        protocol = twcgi.CGIProcessProtocol(request)
+        request.connectionLost(
+            failure.Failure(ConnectionLost("Connection done"))
+        )
+        protocol.processEnded(failure.Failure(error.ProcessTerminated()))
+
+
 
 def discardBody(response):
     """

diff --git a/src/twisted/web/twcgi.py b/src/twisted/web/twcgi.py
@@ -202,6 +202,7 @@ class CGIProcessProtocol(protocol.ProcessProtocol, pb.Viewable):
     headertext = b''
     errortext = b''
     _log = Logger()
+    _requestFinished = False
 
     # Remotely relay producer interface.
 
@@ -231,6 +232,7 @@ def stopProducing(self):
 
     def __init__(self, request):
         self.request = request
+        self.request.notifyFinish().addBoth(self._finished)
 
 
     def connectionMade(self):
@@ -311,12 +313,24 @@ def processEnded(self, reason):
         if self.errortext:
             self._log.error("Errors from CGI {uri}: {errorText}",
                 uri=self.request.uri, errorText=self.errortext)
+
         if self.handling_headers:
             self._log.error("Premature end of headers in {uri}: {headerText}",
                 uri=self.request.uri, headerText=self.headertext)
-            self.request.write(
-                resource.ErrorPage(http.INTERNAL_SERVER_ERROR,
-                    "CGI Script Error",
-                    "Premature end of script headers.").render(self.request))
-        self.request.unregisterProducer()
-        self.request.finish()
+            if not self._requestFinished:
+                self.request.write(
+                    resource.ErrorPage(http.INTERNAL_SERVER_ERROR,
+                        "CGI Script Error", "Premature end of script headers."
+                    ).render(self.request))
+
+        if not self._requestFinished:
+            self.request.unregisterProducer()
+            self.request.finish()
+
+
+    def _finished(self, ignored):
+        """
+        Record the end of the response generation for the request being
+        serviced.
+        """
+        self._requestFinished = True
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		twisted.web.twcgi.CGIProcessProtocol.processEnded(...) now handles an already-finished request, for example when request.connectionLost(...) was called previously.