Mercurial > hg
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): |