lfs: gracefully handle aborts on the server when corrupt blobs are detected
The aborts weren't killing the server, but this seems cleaner. I'm not sure if
it matters to handle the remaining IOError in the test like this, for
consistency.
The error code still feels wrong (especially if the client is trying to download
a corrupt blob) but I don't see anything better in the RFCs, and this is already
used elsewhere because the Batch API spec specifically mentioned this as a
"Validation Error".
--- a/hgext/lfs/blobstore.py Fri Apr 13 14:16:30 2018 -0400
+++ b/hgext/lfs/blobstore.py Sun Feb 25 14:07:13 2018 -0500
@@ -152,7 +152,8 @@
realoid = sha256.hexdigest()
if realoid != oid:
- raise error.Abort(_('corrupt remote lfs object: %s') % oid)
+ raise LfsCorruptionError(_('corrupt remote lfs object: %s')
+ % oid)
self._linktousercache(oid)
@@ -526,8 +527,8 @@
def _verify(oid, content):
realoid = hashlib.sha256(content).hexdigest()
if realoid != oid:
- raise error.Abort(_('detected corrupt lfs object: %s') % oid,
- hint=_('run hg verify'))
+ raise LfsCorruptionError(_('detected corrupt lfs object: %s') % oid,
+ hint=_('run hg verify'))
def remote(repo, remote=None):
"""remotestore factory. return a store in _storemap depending on config
@@ -573,3 +574,8 @@
class LfsRemoteError(error.RevlogError):
pass
+
+class LfsCorruptionError(error.Abort):
+ """Raised when a corrupt blob is detected, aborting an operation
+
+ It exists to allow specialized handling on the server side."""
--- a/hgext/lfs/wireprotolfsserver.py Fri Apr 13 14:16:30 2018 -0400
+++ b/hgext/lfs/wireprotolfsserver.py Sun Feb 25 14:07:13 2018 -0500
@@ -20,6 +20,8 @@
pycompat,
)
+from . import blobstore
+
HTTP_OK = hgwebcommon.HTTP_OK
HTTP_CREATED = hgwebcommon.HTTP_CREATED
HTTP_BAD_REQUEST = hgwebcommon.HTTP_BAD_REQUEST
@@ -276,13 +278,15 @@
# Content-Length, but what happens if a client sends less than it
# says it will?
- # TODO: download() will abort if the checksum fails. It should raise
- # something checksum specific that can be caught here, and turned
- # into an http code.
- localstore.download(oid, req.bodyfh)
+ statusmessage = hgwebcommon.statusmessage
+ try:
+ localstore.download(oid, req.bodyfh)
+ res.status = statusmessage(HTTP_OK if existed else HTTP_CREATED)
+ except blobstore.LfsCorruptionError:
+ _logexception(req)
- statusmessage = hgwebcommon.statusmessage
- res.status = statusmessage(HTTP_OK if existed else HTTP_CREATED)
+ # XXX: Is this the right code?
+ res.status = statusmessage(422, b'corrupt blob')
# There's no payload here, but this is the header that lfs-test-server
# sends back. This eliminates some gratuitous test output conditionals.
@@ -296,9 +300,18 @@
res.status = hgwebcommon.statusmessage(HTTP_OK)
res.headers[b'Content-Type'] = b'application/octet-stream'
- # TODO: figure out how to send back the file in chunks, instead of
- # reading the whole thing.
- res.setbodybytes(localstore.read(oid))
+ try:
+ # TODO: figure out how to send back the file in chunks, instead of
+ # reading the whole thing. (Also figure out how to send back
+ # an error status if an IOError occurs after a partial write
+ # in that case. Here, everything is read before starting.)
+ res.setbodybytes(localstore.read(oid))
+ except blobstore.LfsCorruptionError:
+ _logexception(req)
+
+ # XXX: Is this the right code?
+ res.status = hgwebcommon.statusmessage(422, b'corrupt blob')
+ res.setbodybytes(b'')
return True
else:
--- a/tests/test-lfs-serve-access.t Fri Apr 13 14:16:30 2018 -0400
+++ b/tests/test-lfs-serve-access.t Sun Feb 25 14:07:13 2018 -0500
@@ -236,7 +236,7 @@
$ hg -R client push http://localhost:$HGPORT1
pushing to http://localhost:$HGPORT1/
searching for changes
- abort: HTTP error: HTTP Error 500: Internal Server Error (oid=b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c, action=upload)!
+ abort: HTTP error: HTTP Error 422: corrupt blob (oid=b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c, action=upload)!
[255]
$ echo 'test lfs file' > server/lfs3.bin
@@ -255,7 +255,7 @@
$ hg --config lfs.url=http://localhost:$HGPORT1/.git/info/lfs \
> -R client update -r tip
- abort: HTTP error: HTTP Error 500: Internal Server Error (oid=276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d, action=download)!
+ abort: HTTP error: HTTP Error 422: corrupt blob (oid=276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d, action=download)!
[255]
$ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS
@@ -279,14 +279,14 @@
$LOCALIP - - [$LOGDATE$] "GET /?cmd=branchmap HTTP/1.1" 200 - x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull (glob)
$LOCALIP - - [$LOGDATE$] "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull (glob)
$LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 200 - (glob)
- $LOCALIP - - [$LOGDATE$] "PUT /.hg/lfs/objects/b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c HTTP/1.1" 500 - (glob)
+ $LOCALIP - - [$LOGDATE$] "PUT /.hg/lfs/objects/b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c HTTP/1.1" 422 - (glob)
$LOCALIP - - [$LOGDATE$] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
$LOCALIP - - [$LOGDATE$] "GET /?cmd=batch HTTP/1.1" 200 - x-hgarg-1:cmds=heads+%3Bknown+nodes%3D392c05922088bacf8e68a6939b480017afbf245d x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull (glob)
$LOCALIP - - [$LOGDATE$] "GET /?cmd=getbundle HTTP/1.1" 200 - x-hgarg-1:bookmarks=1&bundlecaps=HG20%2Cbundle2%3DHG20%250Abookmarks%250Achangegroup%253D01%252C02%252C03%250Adigests%253Dmd5%252Csha1%252Csha512%250Aerror%253Dabort%252Cunsupportedcontent%252Cpushraced%252Cpushkey%250Ahgtagsfnodes%250Alistkeys%250Aphases%253Dheads%250Apushkey%250Aremote-changegroup%253Dhttp%252Chttps%250Arev-branch-cache%250Astream%253Dv2&cg=1&common=525251863cad618e55d483555f3d00a2ca99597e&heads=506bf3d83f78c54b89e81c6411adee19fdf02156+525251863cad618e55d483555f3d00a2ca99597e&listkeys=bookmarks&phases=1 x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ partial-pull (glob)
$LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 200 - (glob)
$LOCALIP - - [$LOGDATE$] "GET /.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d HTTP/1.1" 500 - (glob)
$LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 200 - (glob)
- $LOCALIP - - [$LOGDATE$] "GET /.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d HTTP/1.1" 500 - (glob)
+ $LOCALIP - - [$LOGDATE$] "GET /.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d HTTP/1.1" 422 - (glob)
$ grep -v ' File "' $TESTTMP/errors.log
$LOCALIP - - [$ERRDATE$] HG error: Exception happened while processing request '/.git/info/lfs/objects/batch': (glob)
@@ -301,20 +301,13 @@
$LOCALIP - - [$ERRDATE$] HG error: raise IOError(errno.EIO, '%s: I/O error' % oid) (glob)
$LOCALIP - - [$ERRDATE$] HG error: IOError: [Errno 5] b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c: I/O error (glob)
$LOCALIP - - [$ERRDATE$] HG error: (glob)
- $LOCALIP - - [$ERRDATE$] Exception happened during processing request '/.hg/lfs/objects/b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c': (glob)
- Traceback (most recent call last):
- self.do_write()
- self.do_hgweb()
- for chunk in self.server.application(env, self._start_response):
- for r in self._runwsgi(req, res, repo):
- rctx, req, res, self.check_perm)
- return func(*(args + a), **kw)
- lambda perm:
- localstore.download(oid, req.bodyfh)
- super(badstore, self).download(oid, src)
- raise error.Abort(_('corrupt remote lfs object: %s') % oid)
- Abort: corrupt remote lfs object: b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c
-
+ $LOCALIP - - [$ERRDATE$] HG error: Exception happened while processing request '/.hg/lfs/objects/b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c': (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: Traceback (most recent call last): (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: localstore.download(oid, req.bodyfh) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: super(badstore, self).download(oid, src) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: % oid) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: LfsCorruptionError: corrupt remote lfs object: b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: (glob)
$LOCALIP - - [$ERRDATE$] Exception happened during processing request '/.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d': (glob)
Traceback (most recent call last):
self.do_write()
@@ -329,18 +322,11 @@
raise IOError(errno.EIO, '%s: I/O error' % oid)
IOError: [Errno 5] 276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d: I/O error
- $LOCALIP - - [$ERRDATE$] Exception happened during processing request '/.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d': (glob)
- Traceback (most recent call last):
- self.do_write()
- self.do_hgweb()
- for chunk in self.server.application(env, self._start_response):
- for r in self._runwsgi(req, res, repo):
- rctx, req, res, self.check_perm)
- return func(*(args + a), **kw)
- lambda perm:
- res.setbodybytes(localstore.read(oid))
- blob = self._read(self.vfs, oid, verify)
- blobstore._verify(oid, 'dummy content')
- hint=_('run hg verify'))
- Abort: detected corrupt lfs object: 276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d
-
+ $LOCALIP - - [$ERRDATE$] HG error: Exception happened while processing request '/.hg/lfs/objects/276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d': (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: Traceback (most recent call last): (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: res.setbodybytes(localstore.read(oid)) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: blob = self._read(self.vfs, oid, verify) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: blobstore._verify(oid, 'dummy content') (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: hint=_('run hg verify')) (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: LfsCorruptionError: detected corrupt lfs object: 276f73cfd75f9fb519810df5f5d96d6594ca2521abd86cbcd92122f7d51a1f3d (glob)
+ $LOCALIP - - [$ERRDATE$] HG error: (glob)