From 3b72f8e65d78faa21ce4a9d78b26a7a27de334a6 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Tue, 28 Jan 2025 09:38:54 -0600 Subject: [PATCH 1/7] Link to tree views of submodules if possible When linking a submodule at a commit in either the repo view, or a diff when adding a new submodule, link to the tree view of that submodules intead of the individual commit. This shows the user the full tree, instead of the diff of the commit. This makes the assumption that the tree for a given SHA is at `/tree/`. This URL format is supported by both Github & Gitlab, but not Gitea. To fix this, add a redirect from `//tree/` to `//src/`. --- modules/git/commit_submodule_file.go | 4 ++-- modules/git/commit_submodule_file_test.go | 4 ++-- routers/web/web.go | 13 +++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/modules/git/commit_submodule_file.go b/modules/git/commit_submodule_file.go index 2ac744fbf61fd..729401f752112 100644 --- a/modules/git/commit_submodule_file.go +++ b/modules/git/commit_submodule_file.go @@ -46,9 +46,9 @@ func (sf *CommitSubmoduleFile) SubmoduleWebLink(ctx context.Context, optCommitID if len(optCommitID) == 2 { commitLink = sf.repoLink + "/compare/" + optCommitID[0] + "..." + optCommitID[1] } else if len(optCommitID) == 1 { - commitLink = sf.repoLink + "/commit/" + optCommitID[0] + commitLink = sf.repoLink + "/tree/" + optCommitID[0] } else { - commitLink = sf.repoLink + "/commit/" + sf.refID + commitLink = sf.repoLink + "/tree/" + sf.refID } return &SubmoduleWebLink{RepoWebLink: sf.repoLink, CommitWebLink: commitLink} } diff --git a/modules/git/commit_submodule_file_test.go b/modules/git/commit_submodule_file_test.go index 4b5b7676126b8..98342aa9e97b2 100644 --- a/modules/git/commit_submodule_file_test.go +++ b/modules/git/commit_submodule_file_test.go @@ -15,11 +15,11 @@ func TestCommitSubmoduleLink(t *testing.T) { wl := sf.SubmoduleWebLink(context.Background()) assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) - assert.Equal(t, "https://github.com/user/repo/commit/aaaa", wl.CommitWebLink) + assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink) wl = sf.SubmoduleWebLink(context.Background(), "1111") assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) - assert.Equal(t, "https://github.com/user/repo/commit/1111", wl.CommitWebLink) + assert.Equal(t, "https://github.com/user/repo/tree/1111", wl.CommitWebLink) wl = sf.SubmoduleWebLink(context.Background(), "1111", "2222") assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) diff --git a/routers/web/web.go b/routers/web/web.go index 5330b0f3c1b79..4e8e9b6bcfd2b 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -5,6 +5,7 @@ package web import ( "net/http" + "path" "strings" auth_model "code.gitea.io/gitea/models/auth" @@ -1584,6 +1585,18 @@ func registerRoutes(m *web.Router) { m.Get("/*", context.RepoRefByType(""), repo.Home) // "/*" route is deprecated, and kept for backward compatibility }, repo.SetEditorconfigIfExists) + // Add a /tree/* path to redirect to the deprecated /src/* + // path. This emulates both Github & Gitlab's URL structure. + m.Get("/tree/*", func(ctx *context.Context) { + prefix := "/" + if setting.AppSubURL != "" { + prefix = setting.AppSubURL + } + + url := path.Join(prefix, ctx.PathParam("username"), ctx.PathParam("reponame"), "src", ctx.PathParam("*")) + ctx.Redirect(url) + }) + m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) m.Post("/lastcommit/*", context.RepoRefByType(git.RefTypeCommit), repo.LastCommit) From aa242fc7c20b3b2f0e33f05e07b8d0d2b935deb8 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Tue, 28 Jan 2025 14:39:43 -0600 Subject: [PATCH 2/7] fix test --- services/gitdiff/submodule_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/gitdiff/submodule_test.go b/services/gitdiff/submodule_test.go index 89f32c0e0c849..f0eab5557cfa8 100644 --- a/services/gitdiff/submodule_test.go +++ b/services/gitdiff/submodule_test.go @@ -230,7 +230,7 @@ func TestSubmoduleInfo(t *testing.T) { assert.EqualValues(t, "name", sdi.SubmoduleRepoLinkHTML(ctx)) sdi.SubmoduleFile = git.NewCommitSubmoduleFile("https://github.com/owner/repo", "1234") - assert.EqualValues(t, `1111`, sdi.CommitRefIDLinkHTML(ctx, "1111")) + assert.EqualValues(t, `1111`, sdi.CommitRefIDLinkHTML(ctx, "1111")) assert.EqualValues(t, `aaaa...bbbb`, sdi.CompareRefIDLinkHTML(ctx)) assert.EqualValues(t, `name`, sdi.SubmoduleRepoLinkHTML(ctx)) } From dcf59838f6842f6bb4980ea500398894e2596452 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 29 Jan 2025 10:21:09 -0600 Subject: [PATCH 3/7] move redirect handler to repo, escape params, and add unit tests --- routers/web/repo/view_home.go | 30 ++++++++++++++++++ routers/web/repo/view_home_test.go | 50 ++++++++++++++++++++++++++++++ routers/web/web.go | 11 +------ 3 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 routers/web/repo/view_home_test.go diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index 456efb96f6e1a..ee43e82bd84b4 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -8,6 +8,7 @@ import ( "fmt" "html/template" "net/http" + "net/url" "path" "strings" "time" @@ -412,3 +413,32 @@ func Home(ctx *context.Context) { ctx.HTML(http.StatusOK, tplRepoHome) } + +// HomeRedirect redirects from /tree/* to /src/* in order to maintain a similar URL structure. +func HomeRedirect(ctx *context.Context) { + prefix := "/" + if setting.AppSubURL != "" { + prefix = setting.AppSubURL + } + + url := treeRedirectURL( + prefix, + ctx.PathParam("username"), + ctx.PathParam("reponame"), + ctx.PathParam("*"), + ) + ctx.Redirect(url) +} + +// treeRedirectURL constructs a "src" URL from the given "tree" +// prefixed path params. This is a private function for testing +// purposes. +func treeRedirectURL(prefix, username, reponame, remainder string) string { + return path.Join( + prefix, + url.PathEscape(username), + url.PathEscape(reponame), + "src", + util.PathEscapeSegments(remainder), + ) +} diff --git a/routers/web/repo/view_home_test.go b/routers/web/repo/view_home_test.go new file mode 100644 index 0000000000000..10871c174d2d1 --- /dev/null +++ b/routers/web/repo/view_home_test.go @@ -0,0 +1,50 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTreeRedirectURL(t *testing.T) { + type testcase struct { + name string + prefix, username, reponame, remainder string + expected string + } + + cases := []testcase{ + { + name: "ref", + prefix: "/", username: "user2", reponame: "readme-test", remainder: "main", + expected: "/user2/readme-test/src/main", + }, + { + name: "sha", + prefix: "/", username: "user2", reponame: "readme-test", remainder: "fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", + expected: "/user2/readme-test/src/fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", + }, + { + name: "escape", + prefix: "/", username: "user/2%", reponame: "readme/test?", remainder: "$/&", + expected: "/user%2F2%25/readme%2Ftest%3F/src/$/&", + }, + { + name: "appPath", + prefix: "/app-path", username: "user2", reponame: "readme-test", remainder: "main", + expected: "/app-path/user2/readme-test/src/main", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + actual := treeRedirectURL(c.prefix, c.username, c.reponame, c.remainder) + assert.Equal(t, c.expected, actual) + }) + } + +} diff --git a/routers/web/web.go b/routers/web/web.go index 4e8e9b6bcfd2b..1d17db3909a7b 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -5,7 +5,6 @@ package web import ( "net/http" - "path" "strings" auth_model "code.gitea.io/gitea/models/auth" @@ -1587,15 +1586,7 @@ func registerRoutes(m *web.Router) { // Add a /tree/* path to redirect to the deprecated /src/* // path. This emulates both Github & Gitlab's URL structure. - m.Get("/tree/*", func(ctx *context.Context) { - prefix := "/" - if setting.AppSubURL != "" { - prefix = setting.AppSubURL - } - - url := path.Join(prefix, ctx.PathParam("username"), ctx.PathParam("reponame"), "src", ctx.PathParam("*")) - ctx.Redirect(url) - }) + m.Get("/tree/*", repo.HomeRedirect) m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) From 917d322fc94216e2ab21630a35682f61b3b419b5 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 29 Jan 2025 10:31:25 -0600 Subject: [PATCH 4/7] simplify AppSubURL handling --- routers/web/repo/view_home.go | 8 ++------ routers/web/repo/view_home_test.go | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index ee43e82bd84b4..d47737ce1da70 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -416,13 +416,8 @@ func Home(ctx *context.Context) { // HomeRedirect redirects from /tree/* to /src/* in order to maintain a similar URL structure. func HomeRedirect(ctx *context.Context) { - prefix := "/" - if setting.AppSubURL != "" { - prefix = setting.AppSubURL - } - url := treeRedirectURL( - prefix, + setting.AppSubURL, ctx.PathParam("username"), ctx.PathParam("reponame"), ctx.PathParam("*"), @@ -436,6 +431,7 @@ func HomeRedirect(ctx *context.Context) { func treeRedirectURL(prefix, username, reponame, remainder string) string { return path.Join( prefix, + "/", // The prefix may be an empty string, so ensure we have a preceding slash url.PathEscape(username), url.PathEscape(reponame), "src", diff --git a/routers/web/repo/view_home_test.go b/routers/web/repo/view_home_test.go index 10871c174d2d1..a7605e407a779 100644 --- a/routers/web/repo/view_home_test.go +++ b/routers/web/repo/view_home_test.go @@ -19,17 +19,17 @@ func TestTreeRedirectURL(t *testing.T) { cases := []testcase{ { name: "ref", - prefix: "/", username: "user2", reponame: "readme-test", remainder: "main", + prefix: "", username: "user2", reponame: "readme-test", remainder: "main", expected: "/user2/readme-test/src/main", }, { name: "sha", - prefix: "/", username: "user2", reponame: "readme-test", remainder: "fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", + prefix: "", username: "user2", reponame: "readme-test", remainder: "fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", expected: "/user2/readme-test/src/fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", }, { name: "escape", - prefix: "/", username: "user/2%", reponame: "readme/test?", remainder: "$/&", + prefix: "", username: "user/2%", reponame: "readme/test?", remainder: "$/&", expected: "/user%2F2%25/readme%2Ftest%3F/src/$/&", }, { From 6096ad5c2587e4099db132cbe3cb61e3b024fd8e Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 29 Jan 2025 10:32:35 -0600 Subject: [PATCH 5/7] fmt --- routers/web/repo/view_home_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/web/repo/view_home_test.go b/routers/web/repo/view_home_test.go index a7605e407a779..d81778566bc4d 100644 --- a/routers/web/repo/view_home_test.go +++ b/routers/web/repo/view_home_test.go @@ -46,5 +46,4 @@ func TestTreeRedirectURL(t *testing.T) { assert.Equal(t, c.expected, actual) }) } - } From 92cb2a80a578bb5fbbaa1004bef9f0d418224709 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 29 Jan 2025 10:47:58 -0600 Subject: [PATCH 6/7] simplify to use ctx.Repo.RepoLink --- routers/web/repo/view_home.go | 24 ++------------- routers/web/repo/view_home_test.go | 49 ------------------------------ 2 files changed, 2 insertions(+), 71 deletions(-) delete mode 100644 routers/web/repo/view_home_test.go diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index d47737ce1da70..6c6e007b50cf4 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -8,7 +8,6 @@ import ( "fmt" "html/template" "net/http" - "net/url" "path" "strings" "time" @@ -416,25 +415,6 @@ func Home(ctx *context.Context) { // HomeRedirect redirects from /tree/* to /src/* in order to maintain a similar URL structure. func HomeRedirect(ctx *context.Context) { - url := treeRedirectURL( - setting.AppSubURL, - ctx.PathParam("username"), - ctx.PathParam("reponame"), - ctx.PathParam("*"), - ) - ctx.Redirect(url) -} - -// treeRedirectURL constructs a "src" URL from the given "tree" -// prefixed path params. This is a private function for testing -// purposes. -func treeRedirectURL(prefix, username, reponame, remainder string) string { - return path.Join( - prefix, - "/", // The prefix may be an empty string, so ensure we have a preceding slash - url.PathEscape(username), - url.PathEscape(reponame), - "src", - util.PathEscapeSegments(remainder), - ) + remainder := ctx.PathParam("*") + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + util.PathEscapeSegments(remainder)) } diff --git a/routers/web/repo/view_home_test.go b/routers/web/repo/view_home_test.go deleted file mode 100644 index d81778566bc4d..0000000000000 --- a/routers/web/repo/view_home_test.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2025 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repo - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestTreeRedirectURL(t *testing.T) { - type testcase struct { - name string - prefix, username, reponame, remainder string - expected string - } - - cases := []testcase{ - { - name: "ref", - prefix: "", username: "user2", reponame: "readme-test", remainder: "main", - expected: "/user2/readme-test/src/main", - }, - { - name: "sha", - prefix: "", username: "user2", reponame: "readme-test", remainder: "fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", - expected: "/user2/readme-test/src/fe495ea336f079ef2bed68648d0ba9a37cdbd4aa", - }, - { - name: "escape", - prefix: "", username: "user/2%", reponame: "readme/test?", remainder: "$/&", - expected: "/user%2F2%25/readme%2Ftest%3F/src/$/&", - }, - { - name: "appPath", - prefix: "/app-path", username: "user2", reponame: "readme-test", remainder: "main", - expected: "/app-path/user2/readme-test/src/main", - }, - } - - for _, c := range cases { - c := c - t.Run(c.name, func(t *testing.T) { - actual := treeRedirectURL(c.prefix, c.username, c.reponame, c.remainder) - assert.Equal(t, c.expected, actual) - }) - } -} From 7da9b01028b0ff2c720b1da989896a5cc5d2499b Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 29 Jan 2025 11:45:45 -0600 Subject: [PATCH 7/7] update comment to explain why we add a /tree/ path to the repo URLs --- routers/web/web.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index 1d17db3909a7b..01b88aae00e91 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1584,8 +1584,11 @@ func registerRoutes(m *web.Router) { m.Get("/*", context.RepoRefByType(""), repo.Home) // "/*" route is deprecated, and kept for backward compatibility }, repo.SetEditorconfigIfExists) - // Add a /tree/* path to redirect to the deprecated /src/* - // path. This emulates both Github & Gitlab's URL structure. + // Add a /tree/* path to redirect to the /src/* path, which + // will redirect to the canonical URL for that ref. This is + // included so that Gitea's repo URL structure matches what + // other forges provide, allowing clients to construct URLs + // that work across forges. m.Get("/tree/*", repo.HomeRedirect) m.Get("/forks", context.RepoRef(), repo.Forks)