Merge pull request 'fix: git.ComputeHash did not write the content' (#3466) from oliverpool/forgejo:fix_compute_hash into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3466
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

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

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()
+	var obj Sha1Hash
-	_, _ = hasher.Write(t.Bytes())
+	computeHash(obj[:0], sha1.New(), t, content)
-	_, _ = hasher.Write([]byte(" "))
+	return &obj
-	_, _ = 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
 }
 
 /* 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()
+	var obj Sha256Hash
-	_, _ = hasher.Write(t.Bytes())
+	computeHash(obj[:0], sha256.New(), t, content)
-	_, _ = hasher.Write([]byte(" "))
+	return &obj
-	_, _ = 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 (

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())
 }

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 {