[GITEA] Accept shorter commit IDs in web route

- Be more liberal in what Forgejo accepts, by reducing the minimum
amount of characters for SHA to 4 characters, which is the minimum
amount that  Git needs in order to figure out which commit was meant.
- It's safe to reduce this requirements, as commits are passed to Git
which will error if the given commit ID results in more than one Git
object. Forgejo will catch this error as that the Commit doesn't exist,
which is a error that's already being handled in most places gracefully.
- Added integration testing.
- Resolves https://codeberg.org/forgejo/forgejo/issues/1760

(cherry picked from commit 0d655c7384)
(cherry picked from commit 9b9aca2a02)
(cherry picked from commit 0d0ab1af1f)
(cherry picked from commit d3b352c854)
(cherry picked from commit d6af2094df)
This commit is contained in:
Gusted 2023-11-14 19:24:20 +01:00 committed by Earl Warren
parent 58a9c2ebe9
commit f96e55a7a9
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 50 additions and 10 deletions

View file

@ -1217,7 +1217,7 @@ func registerRoutes(m *web.Route) {
Post(web.Bind(forms.UploadRepoFileForm{}), repo.UploadFilePost) Post(web.Bind(forms.UploadRepoFileForm{}), repo.UploadFilePost)
m.Combo("/_diffpatch/*").Get(repo.NewDiffPatch). m.Combo("/_diffpatch/*").Get(repo.NewDiffPatch).
Post(web.Bind(forms.EditRepoFileForm{}), repo.NewDiffPatchPost) Post(web.Bind(forms.EditRepoFileForm{}), repo.NewDiffPatchPost)
m.Combo("/_cherrypick/{sha:([a-f0-9]{7,40})}/*").Get(repo.CherryPick). m.Combo("/_cherrypick/{sha:([a-f0-9]{4,40})}/*").Get(repo.CherryPick).
Post(web.Bind(forms.CherryPickForm{}), repo.CherryPickPost) Post(web.Bind(forms.CherryPickForm{}), repo.CherryPickPost)
}, repo.MustBeEditable) }, repo.MustBeEditable)
m.Group("", func() { m.Group("", func() {
@ -1359,8 +1359,8 @@ func registerRoutes(m *web.Route) {
m.Combo("/*"). m.Combo("/*").
Get(repo.Wiki). Get(repo.Wiki).
Post(context.RepoMustNotBeArchived(), reqSignIn, reqRepoWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost) Post(context.RepoMustNotBeArchived(), reqSignIn, reqRepoWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost)
m.Get("/commit/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:[a-f0-9]{4,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff)
m.Get("/commit/{sha:[a-f0-9]{7,40}}.{ext:patch|diff}", repo.RawDiff) m.Get("/commit/{sha:[a-f0-9]{4,40}}.{ext:patch|diff}", repo.RawDiff)
}, repo.MustEnableWiki, func(ctx *context.Context) { }, repo.MustEnableWiki, func(ctx *context.Context) {
ctx.Data["PageIsWiki"] = true ctx.Data["PageIsWiki"] = true
ctx.Data["CloneButtonOriginLink"] = ctx.Repo.Repository.WikiCloneLink() ctx.Data["CloneButtonOriginLink"] = ctx.Repo.Repository.WikiCloneLink()
@ -1420,7 +1420,7 @@ func registerRoutes(m *web.Route) {
m.Group("/commits", func() { m.Group("/commits", func() {
m.Get("", context.RepoRef(), repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits) m.Get("", context.RepoRef(), repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits)
m.Get("/list", context.RepoRef(), repo.GetPullCommits) m.Get("/list", context.RepoRef(), repo.GetPullCommits)
m.Get("/{sha:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) m.Get("/{sha:[a-f0-9]{4,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit)
}) })
m.Post("/merge", context.RepoMustNotBeArchived(), web.Bind(forms.MergePullRequestForm{}), repo.MergePullRequest) m.Post("/merge", context.RepoMustNotBeArchived(), web.Bind(forms.MergePullRequestForm{}), repo.MergePullRequest)
m.Post("/cancel_auto_merge", context.RepoMustNotBeArchived(), repo.CancelAutoMergePullRequest) m.Post("/cancel_auto_merge", context.RepoMustNotBeArchived(), repo.CancelAutoMergePullRequest)
@ -1429,8 +1429,8 @@ func registerRoutes(m *web.Route) {
m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest) m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest)
m.Group("/files", func() { m.Group("/files", func() {
m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr) m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForAllCommitsOfPr)
m.Get("/{sha:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit) m.Get("/{sha:[a-f0-9]{4,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesStartingFromCommit)
m.Get("/{shaFrom:[a-f0-9]{7,40}}..{shaTo:[a-f0-9]{7,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange) m.Get("/{shaFrom:[a-f0-9]{4,40}}..{shaTo:[a-f0-9]{4,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForRange)
m.Group("/reviews", func() { m.Group("/reviews", func() {
m.Get("/new_comment", repo.RenderNewCodeCommentForm) m.Get("/new_comment", repo.RenderNewCodeCommentForm)
m.Post("/comments", web.Bind(forms.CodeCommentForm{}), repo.SetShowOutdatedComments, repo.CreateCodeComment) m.Post("/comments", web.Bind(forms.CodeCommentForm{}), repo.SetShowOutdatedComments, repo.CreateCodeComment)
@ -1480,9 +1480,9 @@ func registerRoutes(m *web.Route) {
m.Group("", func() { m.Group("", func() {
m.Get("/graph", repo.Graph) m.Get("/graph", repo.Graph)
m.Get("/commit/{sha:([a-f0-9]{7,40})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:([a-f0-9]{4,40})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff)
m.Get("/commit/{sha:([a-f0-9]{7,40})$}/load-branches-and-tags", repo.LoadBranchesAndTags) m.Get("/commit/{sha:([a-f0-9]{4,40})$}/load-branches-and-tags", repo.LoadBranchesAndTags)
m.Get("/cherry-pick/{sha:([a-f0-9]{7,40})$}", repo.SetEditorconfigIfExists, repo.CherryPick) m.Get("/cherry-pick/{sha:([a-f0-9]{4,40})$}", repo.SetEditorconfigIfExists, repo.CherryPick)
}, repo.MustBeNotEmpty, context.RepoRef(), reqRepoCodeReader) }, repo.MustBeNotEmpty, context.RepoRef(), reqRepoCodeReader)
m.Get("/rss/branch/*", repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed("rss")) m.Get("/rss/branch/*", repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed("rss"))
@ -1499,7 +1499,7 @@ func registerRoutes(m *web.Route) {
m.Group("", func() { m.Group("", func() {
m.Get("/forks", repo.Forks) m.Get("/forks", repo.Forks)
}, context.RepoRef(), reqRepoCodeReader) }, context.RepoRef(), reqRepoCodeReader)
m.Get("/commit/{sha:([a-f0-9]{7,40})}.{ext:patch|diff}", repo.MustBeNotEmpty, reqRepoCodeReader, repo.RawDiff) m.Get("/commit/{sha:([a-f0-9]{4,40})}.{ext:patch|diff}", repo.MustBeNotEmpty, reqRepoCodeReader, repo.RawDiff)
}, ignSignIn, context.RepoAssignment, context.UnitTypes()) }, ignSignIn, context.RepoAssignment, context.UnitTypes())
m.Post("/{username}/{reponame}/lastcommit/*", ignSignInAndCsrf, context.RepoAssignment, context.UnitTypes(), context.RepoRefByType(context.RepoRefCommit), reqRepoCodeReader, repo.LastCommit) m.Post("/{username}/{reponame}/lastcommit/*", ignSignInAndCsrf, context.RepoAssignment, context.UnitTypes(), context.RepoRefByType(context.RepoRefCommit), reqRepoCodeReader, repo.LastCommit)

View file

@ -582,3 +582,43 @@ func TestRenamedFileHistory(t *testing.T) {
htmlDoc.AssertElement(t, ".ui.bottom.attached.header", false) htmlDoc.AssertElement(t, ".ui.bottom.attached.header", false)
}) })
} }
func TestCommitView(t *testing.T) {
defer tests.PrepareTestEnv(t)()
t.Run("Non-existent commit", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
req := NewRequest(t, "GET", "/user2/repo1/commit/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
MakeRequest(t, req, http.StatusNotFound)
})
t.Run("Too short commit ID", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
req := NewRequest(t, "GET", "/user2/repo1/commit/65f")
MakeRequest(t, req, http.StatusNotFound)
})
t.Run("Short commit ID", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
req := NewRequest(t, "GET", "/user2/repo1/commit/65f1")
resp := MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitTitle := doc.Find(".commit-summary").Text()
assert.Contains(t, commitTitle, "Initial commit")
})
t.Run("Full commit ID", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
req := NewRequest(t, "GET", "/user2/repo1/commit/65f1bf27bc3bf70f64657658635e66094edbcb4d")
resp := MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitTitle := doc.Find(".commit-summary").Text()
assert.Contains(t, commitTitle, "Initial commit")
})
}