httppeer: use util.readexactly() to abort on incomplete responses
authorAnton Shestakov <av6@dwimlabs.net>
Sat, 08 Sep 2018 21:58:51 +0800
changeset 39484 98995b689e03
parent 39483 1fc39367eafd
child 39485 42bc1c70a6b8
httppeer: use util.readexactly() to abort on incomplete responses Plain resp.read(n) may not return exactly n bytes when we need, and to detect such cases before trying to interpret whatever has been read, we can use util.readexactly(), which raises an Abort when stream ends unexpectedly. In the first case here, readexactly() prevents a traceback with struct.error, in the second it avoids looking for invalid compression engines. In this test case, _wraphttpresponse doesn't catch the problem (presumably because it doesn't know transfer encoding), and the code continues reading the response until it gets to compression engine data. Maybe there should be checks before the execution gets there, but I'm not sure where (httplib?)
mercurial/httppeer.py
tests/test-http-bad-server.t
--- a/mercurial/httppeer.py	Sat Sep 08 23:57:07 2018 +0800
+++ b/mercurial/httppeer.py	Sat Sep 08 21:58:51 2018 +0800
@@ -401,8 +401,8 @@
     elif version_info == (0, 2):
         # application/mercurial-0.2 always identifies the compression
         # engine in the payload header.
-        elen = struct.unpack('B', resp.read(1))[0]
-        ename = resp.read(elen)
+        elen = struct.unpack('B', util.readexactly(resp, 1))[0]
+        ename = util.readexactly(resp, elen)
         engine = util.compengines.forwiretype(ename)
 
         resp = engine.decompressorreader(resp)
--- a/tests/test-http-bad-server.t	Sat Sep 08 23:57:07 2018 +0800
+++ b/tests/test-http-bad-server.t	Sat Sep 08 21:58:51 2018 +0800
@@ -464,6 +464,26 @@
 
   $ rm -f error.log
 
+Server stops before it sends transfer encoding
+
+  $ hg serve --config badserver.closeaftersendbytes=959 -p $HGPORT -d --pid-file=hg.pid -E error.log
+  $ cat hg.pid > $DAEMON_PIDS
+
+  $ hg clone http://localhost:$HGPORT/ clone
+  requesting all changes
+  abort: stream ended unexpectedly (got 0 bytes, expected 1)
+  [255]
+
+  $ killdaemons.py $DAEMON_PIDS
+
+  $ tail -4 error.log
+  write(41 from 41) -> (25) Content-Type: application/mercurial-0.2\r\n
+  write(25 from 28) -> (0) Transfer-Encoding: chunke
+  write limit reached; closing socket
+  write(36) -> HTTP/1.1 500 Internal Server Error\r\n
+
+  $ rm -f error.log
+
 Server sends empty HTTP body for getbundle
 
   $ hg serve --config badserver.closeaftersendbytes=964 -p $HGPORT -d --pid-file=hg.pid -E error.log