diff --git a/vcsserver/git_lfs/app.py b/vcsserver/git_lfs/app.py --- a/vcsserver/git_lfs/app.py +++ b/vcsserver/git_lfs/app.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software Foundation, # Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - +import hashlib import re import logging @@ -160,8 +160,14 @@ def lfs_objects_oid_upload(request): engine = store.get_engine(mode='wb') log.debug('LFS: starting chunked write of LFS oid: %s to storage', oid) + # validate if OID is not by any chance already in the store + if store.has_oid(): + log.debug('LFS: oid %s exists in store', oid) + return {'upload': 'ok', 'state': 'in-store'} + body = request.environ['wsgi.input'] + digest = hashlib.sha256() with engine as f: blksize = 64 * 1024 # 64kb while True: @@ -175,10 +181,17 @@ def lfs_objects_oid_upload(request): if not chunk: break + f.write(chunk) + digest.update(chunk) - f.write(chunk) + hex_digest = digest.hexdigest() + digest_check = hex_digest == oid + if not digest_check: + engine.cleanup() # trigger cleanup so we don't save mismatch OID into the store + return write_response_error( + HTTPBadRequest, f'oid {oid} does not match expected sha {hex_digest}') - return {'upload': 'ok'} + return {'upload': 'ok', 'state': 'written'} def lfs_objects_oid_download(request): @@ -223,12 +236,10 @@ def lfs_objects_verify(request): store_size = store.size_oid() if store_size != size: - msg = 'requested file size mismatch store size:{} requested:{}'.format( - store_size, size) - return write_response_error( - HTTPUnprocessableEntity, msg) + msg = f'requested file size mismatch store size:{store_size} requested:{size}' + return write_response_error(HTTPUnprocessableEntity, msg) - return {'message': {'size': 'ok', 'in_store': 'ok'}} + return {'message': {'size': store_size, 'oid': oid}} def lfs_objects_lock(request): diff --git a/vcsserver/git_lfs/lib.py b/vcsserver/git_lfs/lib.py --- a/vcsserver/git_lfs/lib.py +++ b/vcsserver/git_lfs/lib.py @@ -133,12 +133,16 @@ class LFSOidStore: """ class StoreEngine: + _cleanup = None def __init__(self, mode, store_path, oid_path, tmp_oid_path): self.mode = mode self.store_path = store_path self.oid_path = oid_path self.tmp_oid_path = tmp_oid_path + def cleanup(self): + self._cleanup = True + def __enter__(self): if not os.path.isdir(self.store_path): os.makedirs(self.store_path) @@ -149,9 +153,13 @@ class LFSOidStore: return fd def __exit__(self, exc_type, exc_value, traceback): - # close tmp file, and rename to final destination self.fd.close() - shutil.move(self.tmp_oid_path, self.oid_path) + + if self._cleanup is None: + # close tmp file, and rename to final destination + shutil.move(self.tmp_oid_path, self.oid_path) + else: + os.remove(self.tmp_oid_path) return StoreEngine( mode, self.store_path, self.oid_path, self.tmp_oid_path) diff --git a/vcsserver/git_lfs/tests/test_lfs_app.py b/vcsserver/git_lfs/tests/test_lfs_app.py --- a/vcsserver/git_lfs/tests/test_lfs_app.py +++ b/vcsserver/git_lfs/tests/test_lfs_app.py @@ -100,22 +100,27 @@ class TestLFSApplication: def test_app_batch_api_download_missing_object( self, git_lfs_app, http_auth): - params = {'operation': 'download', - 'objects': [{'oid': '123', 'size': '1024'}]} + params = { + 'operation': 'download', + 'objects': [{'oid': '123', 'size': '1024'}] + } response = git_lfs_app.post_json( '/repo/info/lfs/objects/batch', params=params, extra_environ=http_auth) expected_objects = [ - {'authenticated': True, - 'errors': {'error': { - 'code': 404, - 'message': 'object: 123 does not exist in store'}}, - 'oid': '123', - 'size': '1024'} + { + 'oid': '123', + 'size': '1024', + 'authenticated': True, + 'errors': {'error': {'code': 404, 'message': 'object: 123 does not exist in store'}}, + } ] + assert json.loads(response.text) == { - 'objects': expected_objects, 'transfer': 'basic'} + 'objects': expected_objects, + 'transfer': 'basic' + } def test_app_batch_api_download(self, git_lfs_app, http_auth): oid = '456' @@ -142,7 +147,9 @@ class TestLFSApplication: 'size': '1024'} ] assert json.loads(response.text) == { - 'objects': expected_objects, 'transfer': 'basic'} + 'objects': expected_objects, + 'transfer': 'basic' + } def test_app_batch_api_upload(self, git_lfs_app, http_auth): params = {'operation': 'upload', @@ -151,21 +158,31 @@ class TestLFSApplication: '/repo/info/lfs/objects/batch', params=params, extra_environ=http_auth) expected_objects = [ - {'authenticated': True, - 'actions': { - 'upload': { - 'header': {'Authorization': 'Basic XXXXX', - 'Transfer-Encoding': 'chunked'}, - 'href': 'http://localhost/repo/info/lfs/objects/123'}, - 'verify': { - 'header': {'Authorization': 'Basic XXXXX'}, - 'href': 'http://localhost/repo/info/lfs/verify'} - }, - 'oid': '123', - 'size': '1024'} + { + 'authenticated': True, + 'actions': { + 'upload': { + 'header': { + 'Authorization': 'Basic XXXXX', + 'Transfer-Encoding': 'chunked' + }, + 'href': 'http://localhost/repo/info/lfs/objects/123' + }, + 'verify': { + 'header': { + 'Authorization': 'Basic XXXXX' + }, + 'href': 'http://localhost/repo/info/lfs/verify' + } + }, + 'oid': '123', + 'size': '1024' + } ] assert json.loads(response.text) == { - 'objects': expected_objects, 'transfer': 'basic'} + 'objects': expected_objects, + 'transfer': 'basic' + } def test_app_batch_api_upload_for_https(self, git_lfs_https_app, http_auth): params = {'operation': 'upload', @@ -178,7 +195,7 @@ class TestLFSApplication: 'actions': { 'upload': { 'header': {'Authorization': 'Basic XXXXX', - 'Transfer-Encoding': 'chunked'}, + 'Transfer-Encoding': 'chunked'}, 'href': 'https://localhost/repo/info/lfs/objects/123'}, 'verify': { 'header': {'Authorization': 'Basic XXXXX'}, @@ -206,7 +223,8 @@ class TestLFSApplication: status=404) assert json.loads(response.text) == { - 'message': 'oid `missing` does not exists in store'} + 'message': 'oid `missing` does not exists in store' + } def test_app_verify_api_size_mismatch(self, git_lfs_app): oid = 'existing' @@ -221,8 +239,8 @@ class TestLFSApplication: '/repo/info/lfs/verify', params=params, status=422) assert json.loads(response.text) == { - 'message': 'requested file size mismatch ' - 'store size:11 requested:1024'} + 'message': 'requested file size mismatch store size:11 requested:1024' + } def test_app_verify_api(self, git_lfs_app): oid = 'existing' @@ -237,13 +255,13 @@ class TestLFSApplication: '/repo/info/lfs/verify', params=params) assert json.loads(response.text) == { - 'message': {'size': 'ok', 'in_store': 'ok'}} + 'message': {'size': 11, 'oid': oid} + } def test_app_download_api_oid_not_existing(self, git_lfs_app): oid = 'missing' - response = git_lfs_app.get( - '/repo/info/lfs/objects/{oid}'.format(oid=oid), status=404) + response = git_lfs_app.get(f'/repo/info/lfs/objects/{oid}', status=404) assert json.loads(response.text) == { 'message': 'requested file with oid `missing` not found in store'} @@ -256,19 +274,37 @@ class TestLFSApplication: with open(oid_path, 'wb') as f: f.write(safe_bytes('OID_CONTENT')) - response = git_lfs_app.get( - '/repo/info/lfs/objects/{oid}'.format(oid=oid)) + response = git_lfs_app.get(f'/repo/info/lfs/objects/{oid}') assert response def test_app_upload(self, git_lfs_app): - oid = 'uploaded' + oid = '65f23e22a9bfedda96929b3cfcb8b6d2fdd34a2e877ddb81f45d79ab05710e12' response = git_lfs_app.put( - '/repo/info/lfs/objects/{oid}'.format(oid=oid), params='CONTENT') + f'/repo/info/lfs/objects/{oid}', params='CONTENT') - assert json.loads(response.text) == {'upload': 'ok'} + assert json.loads(response.text) == {'upload': 'ok', 'state': 'written'} # verify that we actually wrote that OID oid_path = LFSOidStore(oid=oid, repo=None, store_location=git_lfs_app._store).oid_path assert os.path.isfile(oid_path) assert 'CONTENT' == open(oid_path).read() + + response = git_lfs_app.put( + f'/repo/info/lfs/objects/{oid}', params='CONTENT') + + assert json.loads(response.text) == {'upload': 'ok', 'state': 'in-store'} + + + def test_app_upload_wrong_sha(self, git_lfs_app): + oid = 'i-am-a-wrong-sha' + + response = git_lfs_app.put(f'/repo/info/lfs/objects/{oid}', params='CONTENT', status=400) + + assert json.loads(response.text) == { + 'message': 'oid i-am-a-wrong-sha does not match expected sha ' + '65f23e22a9bfedda96929b3cfcb8b6d2fdd34a2e877ddb81f45d79ab05710e12'} + + # check this OID wasn't written to store + response = git_lfs_app.get(f'/repo/info/lfs/objects/{oid}', status=404) + assert json.loads(response.text) == {'message': 'requested file with oid `i-am-a-wrong-sha` not found in store'} diff --git a/vcsserver/git_lfs/tests/test_lib.py b/vcsserver/git_lfs/tests/test_lib.py --- a/vcsserver/git_lfs/tests/test_lib.py +++ b/vcsserver/git_lfs/tests/test_lib.py @@ -24,7 +24,7 @@ from vcsserver.git_lfs.lib import OidHan @pytest.fixture() def lfs_store(tmpdir): repo = 'test' - oid = '123456789' + oid = '65f23e22a9bfedda96929b3cfcb8b6d2fdd34a2e877ddb81f45d79ab05710e12' store = LFSOidStore(oid=oid, repo=repo, store_location=str(tmpdir)) return store @@ -63,7 +63,8 @@ class TestOidHandler: assert response is None assert has_errors['error'] == { 'code': 404, - 'message': 'object: 123456789 does not exist in store'} + 'message': 'object: 65f23e22a9bfedda96929b3cfcb8b6d2fdd34a2e877ddb81f45d79ab05710e12 does not exist in store' + } def test_download_oid(self, oid_handler): store = oid_handler.get_store()