comparison mercurial/wireprotoserver.py @ 35987:6010fe1da619

wireprotoserver: document and improve the httplib workaround This workaround dates all the way back to a42d27bc809d in 2008. The code is esoteric enough to warrant an inline explanation. So I've added one. At the time the code was written, the only wire protocol command that accepted an HTTP request body was "unbundle." In the years since, we've grown the ability to accept command arguments via HTTP POST requests. So, the code has been changed to apply the httplib workaround to all HTTP POST requests. While staring at this code, I realized that the HTTP response body in case of error is always the same. And, it appears to mimic the behavior of a failed call to the "unbundle" command. Since we can hit this code path on theoretically any protocol request (since self.check_perm accepts custom auth checking functions which may raise), I'm having a hard time believing that clients react well to an "unbundle" response payload on any wire protocol command. I wouldn't be surprised if our test coverage for this feature only covers HTTP POST calls to "unbundle." In other words, the experimental support for sending arguments via HTTP POST request bodies may result in badness on the client. Something to investigate another time perhaps... Differential Revision: https://phab.mercurial-scm.org/D2064
author Gregory Szorc <gregory.szorc@gmail.com>
date Thu, 01 Feb 2018 16:11:54 -0800
parents 98a00aa0288d
children 04231e893a12
comparison
equal deleted inserted replaced
35986:98a00aa0288d 35987:6010fe1da619
290 val = proto.restore() 290 val = proto.restore()
291 rsp = '%d\n%s' % (rsp.res, val) 291 rsp = '%d\n%s' % (rsp.res, val)
292 req.respond(HTTP_OK, HGTYPE, body=rsp) 292 req.respond(HTTP_OK, HGTYPE, body=rsp)
293 return [] 293 return []
294 elif isinstance(rsp, wireproto.pusherr): 294 elif isinstance(rsp, wireproto.pusherr):
295 # drain the incoming bundle 295 # This is the httplib workaround documented in _handlehttperror().
296 req.drain() 296 req.drain()
297
297 proto.restore() 298 proto.restore()
298 rsp = '0\n%s\n' % rsp.res 299 rsp = '0\n%s\n' % rsp.res
299 req.respond(HTTP_OK, HGTYPE, body=rsp) 300 req.respond(HTTP_OK, HGTYPE, body=rsp)
300 return [] 301 return []
301 elif isinstance(rsp, wireproto.ooberror): 302 elif isinstance(rsp, wireproto.ooberror):
304 return [] 305 return []
305 raise error.ProgrammingError('hgweb.protocol internal failure', rsp) 306 raise error.ProgrammingError('hgweb.protocol internal failure', rsp)
306 307
307 def _handlehttperror(e, req, cmd): 308 def _handlehttperror(e, req, cmd):
308 """Called when an ErrorResponse is raised during HTTP request processing.""" 309 """Called when an ErrorResponse is raised during HTTP request processing."""
309 # A client that sends unbundle without 100-continue will 310
310 # break if we respond early. 311 # Clients using Python's httplib are stateful: the HTTP client
311 if (cmd == 'unbundle' and 312 # won't process an HTTP response until all request data is
313 # sent to the server. The intent of this code is to ensure
314 # we always read HTTP request data from the client, thus
315 # ensuring httplib transitions to a state that allows it to read
316 # the HTTP response. In other words, it helps prevent deadlocks
317 # on clients using httplib.
318
319 if (req.env[r'REQUEST_METHOD'] == r'POST' and
320 # But not if Expect: 100-continue is being used.
312 (req.env.get('HTTP_EXPECT', 321 (req.env.get('HTTP_EXPECT',
313 '').lower() != '100-continue') or 322 '').lower() != '100-continue') or
323 # Or the non-httplib HTTP library is being advertised by
324 # the client.
314 req.env.get('X-HgHttp2', '')): 325 req.env.get('X-HgHttp2', '')):
315 req.drain() 326 req.drain()
316 else: 327 else:
317 req.headers.append((r'Connection', r'Close')) 328 req.headers.append((r'Connection', r'Close'))
318 329
330 # TODO This response body assumes the failed command was
331 # "unbundle." That assumption is not always valid.
319 req.respond(e, HGTYPE, body='0\n%s\n' % e) 332 req.respond(e, HGTYPE, body='0\n%s\n' % e)
320 333
321 return '' 334 return ''
322 335
323 class sshserver(abstractserverproto): 336 class sshserver(abstractserverproto):