TudbuT/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

[GITEA] Accept shorter commit IDs in web route

Browse source 
- 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)
(cherry picked from commit f96e55a7a9)
(cherry picked from commit bb6261f847)
(cherry picked from commit f6a4146161)
(cherry picked from commit ed02921379)

Conflicts:
	routers/web/web.go
	https://codeberg.org/forgejo/forgejo/pulls/2214
This commit is contained in:
Gusted 2023-11-14 19:24:20 +01:00 • committed by Earl Warren
parent a97de1d5bb
commit 768377cb02
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 50 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
routers/web/web.go
View file

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

40
tests/integration/repo_test.go
View file

@ -592,3 +592,43 @@ func TestViewCommit(t *testing.T) {
resp := MakeRequest(t, req, http.StatusNotFound)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "non-existing commit should render 404 page")
}
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")
})
}