| Submitter | Gregory Szorc |
|---|---|
| Date | Nov. 28, 2014, 7:05 p.m. |
| Message ID | <87760fa6c70b51035f9b.1417201552@gps-mbp.local> |
| Download | mbox | patch |
| Permalink | /patch/6883/ |
| State | Accepted |
| Headers | show |
Comments
On Nov 28, 2014, at 2:05 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1417201142 28800 > # Fri Nov 28 10:59:02 2014 -0800 > # Branch stable > # Node ID 87760fa6c70b51035f9b504393791fd86cde6e22 > # Parent edf29f9c15f0f171847f4c7a8184cca4e95c8b31 > hgweb: send proper HTTP response after uncaught exception Nice work! Queued. > > This patch fixes a bug where hgweb would send an incomplete HTTP > response. > > If an uncaught exception is raised when hgweb is processing a request, > hgweb attempts to send a generic error response and log that exception. > > The server defaults to chunked transfer coding. If an uncaught exception > occurred, it was sending the error response string / chunk properly. > However, RFC 7230 Section 4.1 mandates a 0 size last chunk be sent to > indicate end of the entity body. hgweb was failing to send this last > chunk. As a result, properly written HTTP clients would assume more data > was coming and they would likely time out waiting for another chunk to > arrive. > > Mercurial's own test harness was paving over the improper HTTP behavior > by not attempting to read the response body if the status code was 500. > This incorrect workaround was added in ba6577a19656 and has been removed > with this patch. > > diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py > --- a/mercurial/hgweb/server.py > +++ b/mercurial/hgweb/server.py > @@ -80,8 +80,9 @@ class _httprequesthandler(BaseHTTPServer > self.do_write() > except Exception: > self._start_response("500 Internal Server Error", []) > self._write("Internal Server Error") > + self._done() > tb = "".join(traceback.format_exception(*sys.exc_info())) > self.log_error("Exception happened during processing " > "request '%s':\n%s", self.path, tb) > > diff --git a/tests/get-with-headers.py b/tests/get-with-headers.py > --- a/tests/get-with-headers.py > +++ b/tests/get-with-headers.py > @@ -42,11 +42,10 @@ def request(host, path, show): > if response.getheader(h, None) is not None: > print "%s: %s" % (h, response.getheader(h)) > if not headeronly: > print > - if response.status != 500: > - data = response.read() > - sys.stdout.write(data) > + data = response.read() > + sys.stdout.write(data) > > if twice and response.getheader('ETag', None): > tag = response.getheader('ETag') > > diff --git a/tests/hgweberror.py b/tests/hgweberror.py > new file mode 100644 > --- /dev/null > +++ b/tests/hgweberror.py > @@ -0,0 +1,17 @@ > +# A dummy extension that installs an hgweb command that throws an Exception. > + > +from mercurial.hgweb import webcommands > + > +def raiseerror(web, req, tmpl): > + '''Dummy web command that raises an uncaught Exception.''' > + > + # Simulate an error after partial response. > + if 'partialresponse' in req.form: > + req.respond(200, 'text/plain') > + req.write('partial content\n') > + > + raise Exception('I am an uncaught error!') > + > +def extsetup(ui): > + setattr(webcommands, 'raiseerror', raiseerror) > + webcommands.__all__.append('raiseerror') > diff --git a/tests/test-hgweb.t b/tests/test-hgweb.t > --- a/tests/test-hgweb.t > +++ b/tests/test-hgweb.t > @@ -578,5 +578,31 @@ phase changes are refreshed (issue4061) > errors > > $ cat errors.log > > +Uncaught exceptions result in a logged error and canned HTTP response > + > + $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS > + $ hg --config extensions.hgweberror=$TESTDIR/hgweberror.py serve -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log > + $ cat hg.pid >> $DAEMON_PIDS > + > + $ $TESTDIR/get-with-headers.py localhost:$HGPORT 'raiseerror' transfer-encoding content-type > + 500 Internal Server Error > + transfer-encoding: chunked > + > + Internal Server Error (no-eol) > + [1] > + > + $ head -1 errors.log > + .* Exception happened during processing request '/raiseerror': (re) > + > +Uncaught exception after partial content sent > + > + $ $TESTDIR/get-with-headers.py localhost:$HGPORT 'raiseerror?partialresponse=1' transfer-encoding content-type > + 200 Script output follows > + transfer-encoding: chunked > + content-type: text/plain > + > + partial content > + Internal Server Error (no-eol) > + > $ cd .. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Nov 28, 2014, at 2:05 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> + raise Exception('I am an uncaught error!')
Fixing in flight: check-code sends you season's greetings.
Patch
diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py --- a/mercurial/hgweb/server.py +++ b/mercurial/hgweb/server.py @@ -80,8 +80,9 @@ class _httprequesthandler(BaseHTTPServer self.do_write() except Exception: self._start_response("500 Internal Server Error", []) self._write("Internal Server Error") + self._done() tb = "".join(traceback.format_exception(*sys.exc_info())) self.log_error("Exception happened during processing " "request '%s':\n%s", self.path, tb) diff --git a/tests/get-with-headers.py b/tests/get-with-headers.py --- a/tests/get-with-headers.py +++ b/tests/get-with-headers.py @@ -42,11 +42,10 @@ def request(host, path, show): if response.getheader(h, None) is not None: print "%s: %s" % (h, response.getheader(h)) if not headeronly: print - if response.status != 500: - data = response.read() - sys.stdout.write(data) + data = response.read() + sys.stdout.write(data) if twice and response.getheader('ETag', None): tag = response.getheader('ETag') diff --git a/tests/hgweberror.py b/tests/hgweberror.py new file mode 100644 --- /dev/null +++ b/tests/hgweberror.py @@ -0,0 +1,17 @@ +# A dummy extension that installs an hgweb command that throws an Exception. + +from mercurial.hgweb import webcommands + +def raiseerror(web, req, tmpl): + '''Dummy web command that raises an uncaught Exception.''' + + # Simulate an error after partial response. + if 'partialresponse' in req.form: + req.respond(200, 'text/plain') + req.write('partial content\n') + + raise Exception('I am an uncaught error!') + +def extsetup(ui): + setattr(webcommands, 'raiseerror', raiseerror) + webcommands.__all__.append('raiseerror') diff --git a/tests/test-hgweb.t b/tests/test-hgweb.t --- a/tests/test-hgweb.t +++ b/tests/test-hgweb.t @@ -578,5 +578,31 @@ phase changes are refreshed (issue4061) errors $ cat errors.log +Uncaught exceptions result in a logged error and canned HTTP response + + $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS + $ hg --config extensions.hgweberror=$TESTDIR/hgweberror.py serve -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log + $ cat hg.pid >> $DAEMON_PIDS + + $ $TESTDIR/get-with-headers.py localhost:$HGPORT 'raiseerror' transfer-encoding content-type + 500 Internal Server Error + transfer-encoding: chunked + + Internal Server Error (no-eol) + [1] + + $ head -1 errors.log + .* Exception happened during processing request '/raiseerror': (re) + +Uncaught exception after partial content sent + + $ $TESTDIR/get-with-headers.py localhost:$HGPORT 'raiseerror?partialresponse=1' transfer-encoding content-type + 200 Script output follows + transfer-encoding: chunked + content-type: text/plain + + partial content + Internal Server Error (no-eol) + $ cd ..