Mercurial > hg-stable

changeset 37692:10e5bb9678f4

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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".
author Matt Harbison <matt_harbison@yahoo.com>
date Sun, 25 Feb 2018 14:07:13 -0500
parents d241e6632669
children 31a0d47d69b3
files hgext/lfs/blobstore.py hgext/lfs/wireprotolfsserver.py tests/test-lfs-serve-access.t
diffstat 3 files changed, 50 insertions(+), 45 deletions(-) [+]
line wrap: on
line diff
--- 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)