From ebbac258a290fdd8e7fe04e7e5312ea28bd7b1ab Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 19 Dec 2023 06:03:56 +0100 Subject: [PATCH] Add check for storage where repo/org name is empty (#2968) I just discovered that there is an organization created with name being empty. we should at least catch it for now in the storage - and later trace down why we get it in the first place --- server/store/datastore/org.go | 4 ++++ server/store/datastore/org_test.go | 1 + server/store/datastore/repo.go | 9 +++++++++ server/store/datastore/repo_test.go | 21 +++++++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/server/store/datastore/org.go b/server/store/datastore/org.go index a6a1b37cd..05cdbb434 100644 --- a/server/store/datastore/org.go +++ b/server/store/datastore/org.go @@ -15,6 +15,7 @@ package datastore import ( + "fmt" "strings" "xorm.io/xorm" @@ -29,6 +30,9 @@ func (s storage) OrgCreate(org *model.Org) error { func (s storage) orgCreate(org *model.Org, sess *xorm.Session) error { // sanitize org.Name = strings.ToLower(org.Name) + if org.Name == "" { + return fmt.Errorf("org name is empty") + } // insert _, err := sess.Insert(org) return err diff --git a/server/store/datastore/org_test.go b/server/store/datastore/org_test.go index 8e44bee5a..a2a06f868 100644 --- a/server/store/datastore/org_test.go +++ b/server/store/datastore/org_test.go @@ -63,6 +63,7 @@ func TestOrgCRUD(t *testing.T) { assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "some_other_u", Name: "abc", FullName: "some_other_u/abc", OrgID: someUser.ID})) assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "some_other_u", Name: "xyz", FullName: "some_other_u/xyz", OrgID: someUser.ID})) assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "renamedorg", Name: "567", FullName: "renamedorg/567", OrgID: orgOne.ID})) + assert.Error(t, store.OrgCreate(&model.Org{Name: ""}), "expect to fail if name is empty") // get all repos for a specific org repos, err := store.OrgRepoList(someUser, &model.ListOptions{All: true}) diff --git a/server/store/datastore/repo.go b/server/store/datastore/repo.go index 4b0a62498..bb33f15d5 100644 --- a/server/store/datastore/repo.go +++ b/server/store/datastore/repo.go @@ -16,6 +16,7 @@ package datastore import ( "errors" + "fmt" "strings" "xorm.io/builder" @@ -80,6 +81,14 @@ func (s storage) GetRepoCount() (int64, error) { } func (s storage) CreateRepo(repo *model.Repo) error { + switch { + case repo.Name == "": + return fmt.Errorf("repo name is empty") + case repo.Owner == "": + return fmt.Errorf("repo owner is empty") + case repo.FullName == "": + return fmt.Errorf("repo full name is empty") + } // only Insert set auto created ID back to object _, err := s.engine.Insert(repo) return err diff --git a/server/store/datastore/repo_test.go b/server/store/datastore/repo_test.go index 02dd66ea8..1a05613b4 100644 --- a/server/store/datastore/repo_test.go +++ b/server/store/datastore/repo_test.go @@ -70,6 +70,27 @@ func TestRepos(t *testing.T) { g.Assert(repo.ID != 0).IsTrue() }) + g.It("Should fail if repo has no name / owner / fullname", func() { + g.Assert(store.CreateRepo(&model.Repo{ + UserID: 1, + FullName: "bradrydzewski/", + Owner: "bradrydzewski", + Name: "", + })).IsNotNil() + g.Assert(store.CreateRepo(&model.Repo{ + UserID: 1, + FullName: "/test", + Owner: "", + Name: "test", + })).IsNotNil() + g.Assert(store.CreateRepo(&model.Repo{ + UserID: 1, + FullName: "", + Owner: "bradrydzewski", + Name: "test", + })).IsNotNil() + }) + g.It("Should Get a Repo by ID", func() { repo := model.Repo{ UserID: 1,