diff hgext/lfs/wireprotolfsserver.py @ 37692:10e5bb9678f4

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/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: