Patchwork D2064: wireprotoserver: document and improve the httplib workaround

login
register
mail settings
Submitter phabricator
Date Feb. 7, 2018, 10:41 p.m.
Message ID <e380d8164a59886b4e53fb0355bd70a5@localhost.localdomain>
Download mbox | patch
Permalink /patch/27446/
State Not Applicable
Headers show

Comments

phabricator - Feb. 7, 2018, 10:41 p.m.
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG6010fe1da619: wireprotoserver: document and improve the httplib workaround (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2064?vs=5261&id=5312

REVISION DETAIL
  https://phab.mercurial-scm.org/D2064

AFFECTED FILES
  mercurial/wireprotoserver.py

CHANGE DETAILS




To: indygreg, #hg-reviewers, durin42
Cc: durin42, mercurial-devel

Patch

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -292,8 +292,9 @@ 
         req.respond(HTTP_OK, HGTYPE, body=rsp)
         return []
     elif isinstance(rsp, wireproto.pusherr):
-        # drain the incoming bundle
+        # This is the httplib workaround documented in _handlehttperror().
         req.drain()
+
         proto.restore()
         rsp = '0\n%s\n' % rsp.res
         req.respond(HTTP_OK, HGTYPE, body=rsp)
@@ -306,16 +307,28 @@ 
 
 def _handlehttperror(e, req, cmd):
     """Called when an ErrorResponse is raised during HTTP request processing."""
-    # A client that sends unbundle without 100-continue will
-    # break if we respond early.
-    if (cmd == 'unbundle' and
+
+    # Clients using Python's httplib are stateful: the HTTP client
+    # won't process an HTTP response until all request data is
+    # sent to the server. The intent of this code is to ensure
+    # we always read HTTP request data from the client, thus
+    # ensuring httplib transitions to a state that allows it to read
+    # the HTTP response. In other words, it helps prevent deadlocks
+    # on clients using httplib.
+
+    if (req.env[r'REQUEST_METHOD'] == r'POST' and
+        # But not if Expect: 100-continue is being used.
         (req.env.get('HTTP_EXPECT',
                      '').lower() != '100-continue') or
+        # Or the non-httplib HTTP library is being advertised by
+        # the client.
         req.env.get('X-HgHttp2', '')):
         req.drain()
     else:
         req.headers.append((r'Connection', r'Close'))
 
+    # TODO This response body assumes the failed command was
+    # "unbundle." That assumption is not always valid.
     req.respond(e, HGTYPE, body='0\n%s\n' % e)
 
     return ''