From 3dfa5ba43a907959f279273032a4448cd7e40924 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Thu, 25 Apr 2024 23:03:53 +0200 Subject: [PATCH 1/2] test: LFS gc should not delete all metadata objects and ComputeBlobHash should depend on the blob content (not only the length) --- models/fixtures/lfs_meta_object.yml | 2 +- modules/git/object_id_test.go | 4 ++++ services/repository/lfs_test.go | 13 +++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/models/fixtures/lfs_meta_object.yml b/models/fixtures/lfs_meta_object.yml index 1c29e02d44..cef4824d64 100644 --- a/models/fixtures/lfs_meta_object.yml +++ b/models/fixtures/lfs_meta_object.yml @@ -9,7 +9,7 @@ - - id: 2 + id: 2 # this is an LFS orphan object oid: 2eccdb43825d2a49d99d542daa20075cff1d97d9d2349a8977efe9c03661737c size: 107 repository_id: 54 diff --git a/modules/git/object_id_test.go b/modules/git/object_id_test.go index 1ad40096a0..6f365d6b19 100644 --- a/modules/git/object_id_test.go +++ b/modules/git/object_id_test.go @@ -18,4 +18,8 @@ func TestIsValidSHAPattern(t *testing.T) { assert.False(t, h.IsValid("abc")) assert.False(t, h.IsValid("123g")) assert.False(t, h.IsValid("some random text")) + + assert.Equal(t, "79ee38a6416c1ede423ec7ee0a8639ceea4aad22", ComputeBlobHash(Sha1ObjectFormat, []byte("some random blob")).String()) + assert.Equal(t, "d5c6407415d85df49592672aa421aed39b9db5e3", ComputeBlobHash(Sha1ObjectFormat, []byte("same length blob")).String()) + assert.Equal(t, "df0b5174ed06ae65aea40d43316bcbc21d82c9e3158ce2661df2ad28d7931dd6", ComputeBlobHash(Sha256ObjectFormat, []byte("some random blob")).String()) } diff --git a/services/repository/lfs_test.go b/services/repository/lfs_test.go index ee0b8f6b89..52ee05a147 100644 --- a/services/repository/lfs_test.go +++ b/services/repository/lfs_test.go @@ -28,9 +28,13 @@ func TestGarbageCollectLFSMetaObjects(t *testing.T) { err := storage.Init() assert.NoError(t, err) - repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user2", "repo1") + repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user2", "lfs") assert.NoError(t, err) + validLFSObjects, err := db.GetEngine(db.DefaultContext).Count(git_model.LFSMetaObject{RepositoryID: repo.ID}) + assert.NoError(t, err) + assert.Greater(t, validLFSObjects, int64(1)) + // add lfs object lfsContent := []byte("gitea1") lfsOid := storeObjectInRepo(t, repo.ID, &lfsContent) @@ -39,13 +43,18 @@ func TestGarbageCollectLFSMetaObjects(t *testing.T) { err = repo_service.GarbageCollectLFSMetaObjects(context.Background(), repo_service.GarbageCollectLFSMetaObjectsOptions{ AutoFix: true, OlderThan: time.Now().Add(7 * 24 * time.Hour).Add(5 * 24 * time.Hour), - UpdatedLessRecentlyThan: time.Now().Add(7 * 24 * time.Hour).Add(3 * 24 * time.Hour), + UpdatedLessRecentlyThan: time.Time{}, // ensure that the models/fixtures/lfs_meta_object.yml objects are considered as well + LogDetail: t.Logf, }) assert.NoError(t, err) // lfs meta has been deleted _, err = git_model.GetLFSMetaObjectByOid(db.DefaultContext, repo.ID, lfsOid) assert.ErrorIs(t, err, git_model.ErrLFSObjectNotExist) + + remainingLFSObjects, err := db.GetEngine(db.DefaultContext).Count(git_model.LFSMetaObject{RepositoryID: repo.ID}) + assert.NoError(t, err) + assert.Equal(t, validLFSObjects-1, remainingLFSObjects) } func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string { From 5247fd50dbd94de30c96feca844ff65286935404 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Fri, 26 Apr 2024 10:02:47 +0200 Subject: [PATCH 2/2] fix: git.ComputeHash did not write the content --- modules/git/object_format.go | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/modules/git/object_format.go b/modules/git/object_format.go index a056b20e8a..c2fcf4c063 100644 --- a/modules/git/object_format.go +++ b/modules/git/object_format.go @@ -6,6 +6,7 @@ package git import ( "crypto/sha1" "crypto/sha256" + "hash" "regexp" "strconv" ) @@ -33,6 +34,15 @@ type ObjectFormat interface { ComputeHash(t ObjectType, content []byte) ObjectID } +func computeHash(dst []byte, hasher hash.Hash, t ObjectType, content []byte) []byte { + _, _ = hasher.Write(t.Bytes()) + _, _ = hasher.Write([]byte(" ")) + _, _ = hasher.Write([]byte(strconv.Itoa(len(content)))) + _, _ = hasher.Write([]byte{0}) + _, _ = hasher.Write(content) + return hasher.Sum(dst) +} + /* SHA1 Type */ type Sha1ObjectFormatImpl struct{} @@ -65,16 +75,9 @@ func (Sha1ObjectFormatImpl) MustID(b []byte) ObjectID { // ComputeHash compute the hash for a given ObjectType and content func (h Sha1ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID { - hasher := sha1.New() - _, _ = hasher.Write(t.Bytes()) - _, _ = hasher.Write([]byte(" ")) - _, _ = hasher.Write([]byte(strconv.FormatInt(int64(len(content)), 10))) - _, _ = hasher.Write([]byte{0}) - - // HashSum generates a SHA1 for the provided hash - var sha1 Sha1Hash - copy(sha1[:], hasher.Sum(nil)) - return &sha1 + var obj Sha1Hash + computeHash(obj[:0], sha1.New(), t, content) + return &obj } /* SHA256 Type */ @@ -111,16 +114,9 @@ func (Sha256ObjectFormatImpl) MustID(b []byte) ObjectID { // ComputeHash compute the hash for a given ObjectType and content func (h Sha256ObjectFormatImpl) ComputeHash(t ObjectType, content []byte) ObjectID { - hasher := sha256.New() - _, _ = hasher.Write(t.Bytes()) - _, _ = hasher.Write([]byte(" ")) - _, _ = hasher.Write([]byte(strconv.FormatInt(int64(len(content)), 10))) - _, _ = hasher.Write([]byte{0}) - - // HashSum generates a SHA256 for the provided hash - var sha256 Sha1Hash - copy(sha256[:], hasher.Sum(nil)) - return &sha256 + var obj Sha256Hash + computeHash(obj[:0], sha256.New(), t, content) + return &obj } var (