This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5498

server: always close http socket if responding with an error (issue6033)
ClosedPublic

Authored by durin42 on Jan 6 2019, 3:28 PM.

Details

Reviewers
mharbison72
pulkit
Group Reviewers
hg-reviewers
Commits
rHG197f092b2cd9: server: always close http socket if responding with an error (issue6033)
Summary

It's possible for hgweb to respond _very_ early with an error if we're
catching certain types of errors. When we do, we need to tell the client
the socket is toast when there's a POST involved because otherwise there
can be lingering POST data on the socket that will confuse any future
requests on the socket. This manifested as a flaky failure on Linux in an
lfs extension test and a reliable failure on FreeBSD. With this patch
applied, test-lfs-serve-access.t now passes for me on FreeBSD.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Jan 6 2019, 3:28 PM
mharbison72 accepted this revision.Jan 6 2019, 11:16 PM
mharbison72 added a subscriber: mharbison72.

This works for me on Windows in that narrow window where this was constantly happening, thanks!

pulkit accepted this revision.Jan 7 2019, 5:24 AM
pulkit added a comment.Jan 7 2019, 5:27 AM

Is this meant for stable?

durin42 added a comment.Jan 7 2019, 10:16 AM
In D5498#81525, @pulkit wrote:

Is this meant for stable?

It'd be fine on stable if it applies cleanly there, but it's only /needed/ on default: the tests are exposing what is, frankly, a theoretical problem that extant versions of hg clients won't ever trigger in the wild.

Closed by commit rHG197f092b2cd9: server: always close http socket if responding with an error (issue6033) (authored by durin42). · Explain WhyJan 7 2019, 2:17 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/hgweb/server.py (7 lines)

Diff 13062

mercurial/hgweb/server.py

self.sent_headers = False self.sent_headers = False
path, query = _splitURI(self.path) path, query = _splitURI(self.path)
# Ensure the slicing of path below is valid # Ensure the slicing of path below is valid
if (path != self.server.prefix if (path != self.server.prefix
and not path.startswith(self.server.prefix + b'/')): and not path.startswith(self.server.prefix + b'/')):
self._start_response(pycompat.strurl(common.statusmessage(404)), self._start_response(pycompat.strurl(common.statusmessage(404)),
[]) [])
if self.command == 'POST':
# Paranoia: tell the client we're going to close the
# socket so they don't try and reuse a socket that
# might have a POST body waiting to confuse us. We do
# this by directly munging self.saved_headers because
# self._start_response ignores Connection headers.
self.saved_headers = [(r'Connection', r'Close')]
self._write(b"Not Found") self._write(b"Not Found")
self._done() self._done()
return return
env = {} env = {}
env[r'GATEWAY_INTERFACE'] = r'CGI/1.1' env[r'GATEWAY_INTERFACE'] = r'CGI/1.1'
env[r'REQUEST_METHOD'] = self.command env[r'REQUEST_METHOD'] = self.command
env[r'SERVER_NAME'] = self.server.server_name env[r'SERVER_NAME'] = self.server.server_name