changeset 18483:ce5f529deb36 stable

largefiles: don't allow corruption to propagate after detection basestore.get uses util.atomictempfile when checking and receiving a new largefile ... but the close/discard logic was too clever for largefiles. Largefiles relied on being able to discard the file and thus prevent it from being written to the store. That was however too brittle. lfutil.copyandhash closes the infile after writing to it ... with a 'blecch' comment. The discard was thus a silent noop, and as a result of that corruption would be detected ... and then the corrupted files would be used anyway. Instead we now use a tmp file and rename or unlink it after validating it. A better solution should be implemented ... but not now.
author Mads Kiilerich <madski@unity3d.com>
date Mon, 28 Jan 2013 15:19:44 +0100
parents 6f219eb83435
children d43823f928fe
files hgext/largefiles/basestore.py tests/test-largefiles.t
diffstat 2 files changed, 8 insertions(+), 9 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/largefiles/basestore.py	Mon Jan 28 15:19:44 2013 +0100
+++ b/hgext/largefiles/basestore.py	Mon Jan 28 15:19:44 2013 +0100
@@ -67,7 +67,7 @@
             ui.note(_('getting %s:%s\n') % (filename, hash))
 
             storefilename = lfutil.storepath(self.repo, hash)
-            tmpfile = util.atomictempfile(storefilename,
+            tmpfile = util.atomictempfile(storefilename + '.tmp',
                                           createmode=self.repo.store.createmode)
 
             try:
@@ -75,16 +75,17 @@
             except StoreError, err:
                 ui.warn(err.longmessage())
                 hhash = ""
+            tmpfile.close() # has probably already been closed!
 
             if hhash != hash:
                 if hhash != "":
                     ui.warn(_('%s: data corruption (expected %s, got %s)\n')
                             % (filename, hash, hhash))
-                tmpfile.discard() # no-op if it's already closed
+                util.unlink(storefilename + '.tmp')
                 missing.append(filename)
                 continue
 
-            tmpfile.close()
+            util.rename(storefilename + '.tmp', storefilename)
             lfutil.linktousercache(self.repo, hash)
             success.append((filename, hhash))
 
--- a/tests/test-largefiles.t	Mon Jan 28 15:19:44 2013 +0100
+++ b/tests/test-largefiles.t	Mon Jan 28 15:19:44 2013 +0100
@@ -1584,14 +1584,12 @@
   $ hg -R http-clone up --config largefiles.usercache=http-clone-usercache
   getting changed largefiles
   f1: data corruption (expected 02a439e5c31c526465ab1a0ca1f431f76b827b90, got 6a7bb2556144babe3899b25e5428123735bb1e27)
-  1 largefiles updated, 0 removed
+  0 largefiles updated, 0 removed
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg -R http-clone st
-  M f1
-  $ cat http-clone/.hg/largefiles/02a439e5c31c526465ab1a0ca1f431f76b827b90
-  corruption
-  $ cat http-clone/f1
-  corruption
+  ! f1
+  $ [ ! -f http-clone/.hg/largefiles/02a439e5c31c526465ab1a0ca1f431f76b827b90 ]
+  $ [ ! -f http-clone/f1 ]
   $ [ ! -f http-clone-usercache ]
   $ hg -R http-clone verify --large --lfc
   checking changesets