From 5a2f2229d205206a6c74345ac815267c23966bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Ri=C3=9Fe?= Date: Thu, 20 Feb 2025 21:50:46 +0000 Subject: [PATCH] fix: return 404 for empty repositories (#7003) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Some endpoints (`/api/v1/repos/*/*/raw`, `/api/v1/repos/*/*/media`, ...; anything that uses both `context.ReferencesGitRepo()` and `context.RepoRefForAPI` really) returned a 500 when the repository was completely empty. This resulted in some confusion in https://github.com/datalad/datalad-usage-dashboard/issues/47 because the same request for a non-existent file in a repository could sometimes generate a 404 and sometimes a 500, depending on if the git repository is initialized at all or not. Returning a 404 is more appropriate here, since this isn't an unexpected internal error, but just another way of not finding the requested data. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7003 Reviewed-by: Gusted Co-authored-by: Matthias Riße Co-committed-by: Matthias Riße --- services/context/api.go | 5 +++++ tests/integration/empty_repo_test.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/services/context/api.go b/services/context/api.go index 238313c62c..871a2f012d 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -364,6 +364,11 @@ func RepoRefForAPI(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { ctx := GetAPIContext(req) + if ctx.Repo.Repository.IsEmpty { + ctx.NotFound(fmt.Errorf("repository is empty")) + return + } + if ctx.Repo.GitRepo == nil { ctx.InternalServerError(fmt.Errorf("no open git repo")) return diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index 4122c78ec2..637de7d5c7 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -136,3 +136,24 @@ func TestEmptyRepoAddFileByAPI(t *testing.T) { DecodeJSON(t, resp, &apiRepo) assert.Equal(t, "new_branch", apiRepo.DefaultBranch) } + +func TestEmptyRepoAPIRequestsReturn404(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user30") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("Raw", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user30/empty/raw/main/something").AddTokenAuth(token) + _ = session.MakeRequest(t, req, http.StatusNotFound) + }) + + t.Run("Media", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user30/empty/media/main/something").AddTokenAuth(token) + _ = session.MakeRequest(t, req, http.StatusNotFound) + }) +}