fix: check valid commit ids for existence (#12963)

When downloading archives Forgejo uses git cat-file to resolve a ref name (e.g. branch or tag) to a commit id. There were a check if the input already was a valid full commit id, sadly without a check if the commit exists. Therefore, I removed this "early return".

I noticed this while trying to download archives from Codeberg and the there was just a timeout after 10min. Upon trying this on a self-hosted instance I saw the error in the logs, which led me to his fix.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12963
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
Marcel 2026-06-29 17:21:55 +02:00 committed by Gusted
commit 7e5ca3bdeb
2 changed files with 29 additions and 12 deletions

View file

@ -644,19 +644,9 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id ObjectID)
}
}
// ConvertToGitID returns a GitHash object from a potential ID string
// ConvertToGitID returns a ObjectID object from a potential ID string
// The resulting ObjectID is guaranteed to exist.
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
objectFormat, err := repo.GetObjectFormat()
if err != nil {
return nil, err
}
if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) {
ID, err := NewIDFromString(commitID)
if err == nil {
return ID, nil
}
}
wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx)
if err != nil {
return nil, err

View file

@ -275,6 +275,33 @@ func TestGetCommitsFromIDs(t *testing.T) {
})
}
func TestConvertToGitID(t *testing.T) {
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path)
require.NoError(t, err)
defer bareRepo1.Close()
// existing commit
existingID := "2839944139e0de9737a044f78b0e4b40d989a9e3"
id, err := bareRepo1.ConvertToGitID(existingID)
require.NoError(t, err)
assert.Equal(t, existingID, id.String())
// non-existing commit, but well-formatted git hash
_, err = bareRepo1.ConvertToGitID("2839944139e0de9737a044f78b0e4b40d989a9e4")
require.Error(t, err)
assert.True(t, IsErrNotExist(err))
// invalid branch name
_, err = bareRepo1.ConvertToGitID("invalid-branch")
require.Error(t, err)
// valid branch
id, err = bareRepo1.ConvertToGitID("master")
require.NoError(t, err)
assert.Equal(t, "ce064814f4a0d337b333e646ece456cd39fab612", id.String())
}
func TestGetLatestCommitTime(t *testing.T) {
t.Run("repo1", func(t *testing.T) {
repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare"))