From 80c57ec126be669e4964a4697450cde86dbbeaf4 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Mar 2026 02:30:54 +0800 Subject: [PATCH] Remove `util.URLJoin` and replace all callers with direct path concatenation (#36867) `util.URLJoin` was deprecated with unclear semantics (path normalization via `url.Parse`/`ResolveReference` that surprised callers). This removes it entirely and replaces all usages with straightforward `"/"` string concatenation. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com> --- modules/markup/html_internal_test.go | 14 ++++---- modules/markup/html_test.go | 40 ++++++++++----------- modules/util/url.go | 24 ------------- modules/util/util_test.go | 33 ----------------- services/convert/git_commit_test.go | 3 +- tests/integration/api_packages_rpm_test.go | 3 +- tests/integration/api_repo_git_tags_test.go | 3 +- 7 files changed, 30 insertions(+), 90 deletions(-) diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index ca2579c8ea..3a91797e25 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -23,12 +23,12 @@ const ( // externalIssueLink an HTML link to an alphanumeric-style issue func externalIssueLink(baseURL, class, name string) string { - return link(util.URLJoin(baseURL, name), class, name) + return link(strings.TrimSuffix(baseURL, "/")+"/"+name, class, name) } // numericLink an HTML to a numeric-style issue func numericIssueLink(baseURL, class string, index int, marker string) string { - return link(util.URLJoin(baseURL, strconv.Itoa(index)), class, fmt.Sprintf("%s%d", marker, index)) + return link(strings.TrimSuffix(baseURL, "/")+"/"+strconv.Itoa(index), class, fmt.Sprintf("%s%d", marker, index)) } // link an HTML link @@ -116,7 +116,7 @@ func TestRender_IssueIndexPattern2(t *testing.T) { links := make([]any, len(indices)) for i, index := range indices { - links[i] = numericIssueLink(util.URLJoin("/test-owner/test-repo", path), "ref-issue", index, marker) + links[i] = numericIssueLink("/test-owner/test-repo/"+path, "ref-issue", index, marker) } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, NewTestRenderContext(TestAppURL, localMetas)) @@ -210,7 +210,7 @@ func TestRender_IssueIndexPattern5(t *testing.T) { metas["regexp"] = pattern links := make([]any, len(ids)) for i, id := range ids { - links[i] = link(util.URLJoin("https://someurl.com/someUser/someRepo/", id), "ref-issue ref-external-issue", names[i]) + links[i] = link("https://someurl.com/someUser/someRepo/"+id, "ref-issue ref-external-issue", names[i]) } expected := fmt.Sprintf(expectedFmt, links...) @@ -288,11 +288,11 @@ func TestRender_AutoLink(t *testing.T) { } // render valid issue URLs - test(util.URLJoin(TestRepoURL, "issues", "3333"), - numericIssueLink(util.URLJoin(TestRepoURL, "issues"), "ref-issue", 3333, "#")) + test(TestRepoURL+"issues/3333", + numericIssueLink(TestRepoURL+"issues", "ref-issue", 3333, "#")) // render valid commit URLs - tmp := util.URLJoin(TestRepoURL, "commit", "d8a994ef243349f321568f9e36d5c3f444b99cae") + tmp := TestRepoURL + "commit/d8a994ef243349f321568f9e36d5c3f444b99cae" test(tmp, "d8a994ef24") tmp += "#diff-2" test(tmp, "d8a994ef24 (diff-2)") diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 62c4ae3e0a..e62747c724 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -34,15 +34,15 @@ func TestRender_Commits(t *testing.T) { sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" repo := markup.TestAppURL + testRepoOwnerName + "/" + testRepoName + "/" - commit := util.URLJoin(repo, "commit", sha) + commit := repo + "commit/" + sha commitPath := "/user13/repo11/commit/" + sha - tree := util.URLJoin(repo, "tree", sha, "src") + tree := repo + "tree/" + sha + "/src" - file := util.URLJoin(repo, "commit", sha, "example.txt") + file := repo + "commit/" + sha + "/example.txt" fileWithExtra := file + ":" fileWithHash := file + "#L2" fileWithHasExtra := file + "#L2:" - commitCompare := util.URLJoin(repo, "compare", sha+"..."+sha) + commitCompare := repo + "compare/" + sha + "..." + sha commitCompareWithHash := commitCompare + "#L2" test(sha, `

65f1bf27bc

`) @@ -90,14 +90,14 @@ func TestRender_CrossReferences(t *testing.T) { "/home/gitea/go-gitea/gitea#12345", `

/home/gitea/go-gitea/gitea#12345

`) test( - util.URLJoin(markup.TestAppURL, "gogitea", "gitea", "issues", "12345"), - `

gogitea/gitea#12345

`) + markup.TestAppURL+"gogitea/gitea/issues/12345", + `

gogitea/gitea#12345

`) test( - util.URLJoin(markup.TestAppURL, "go-gitea", "gitea", "issues", "12345"), - `

go-gitea/gitea#12345

`) + markup.TestAppURL+"go-gitea/gitea/issues/12345", + `

go-gitea/gitea#12345

`) test( - util.URLJoin(markup.TestAppURL, "gogitea", "some-repo-name", "issues", "12345"), - `

gogitea/some-repo-name#12345

`) + markup.TestAppURL+"gogitea/some-repo-name/issues/12345", + `

gogitea/some-repo-name#12345

`) inputURL := setting.AppURL + "a/b/commit/0123456789012345678901234567890123456789/foo.txt?a=b#L2-L3" test( @@ -375,7 +375,7 @@ func TestRender_emoji(t *testing.T) { func TestRender_ShortLinks(t *testing.T) { setting.AppURL = markup.TestAppURL - tree := util.URLJoin(markup.TestRepoURL, "src", "master") + tree := markup.TestRepoURL + "src/master" test := func(input, expected string) { buffer, err := markdown.RenderString(markup.NewTestRenderContext(tree), input) @@ -383,15 +383,15 @@ func TestRender_ShortLinks(t *testing.T) { assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) } - url := util.URLJoin(tree, "Link") - otherURL := util.URLJoin(tree, "Other-Link") - encodedURL := util.URLJoin(tree, "Link%3F") - imgurl := util.URLJoin(tree, "Link.jpg") - otherImgurl := util.URLJoin(tree, "Link+Other.jpg") - encodedImgurl := util.URLJoin(tree, "Link+%23.jpg") - notencodedImgurl := util.URLJoin(tree, "some", "path", "Link%20#.jpg") - renderableFileURL := util.URLJoin(tree, "markdown_file.md") - unrenderableFileURL := util.URLJoin(tree, "file.zip") + url := tree + "/Link" + otherURL := tree + "/Other-Link" + encodedURL := tree + "/Link%3F" + imgurl := tree + "/Link.jpg" + otherImgurl := tree + "/Link+Other.jpg" + encodedImgurl := tree + "/Link+%23.jpg" + notencodedImgurl := tree + "/some/path/Link%20#.jpg" + renderableFileURL := tree + "/markdown_file.md" + unrenderableFileURL := tree + "/file.zip" favicon := "http://google.com/favicon.ico" test( diff --git a/modules/util/url.go b/modules/util/url.go index 6455b0b75c..dc31441ca8 100644 --- a/modules/util/url.go +++ b/modules/util/url.go @@ -5,7 +5,6 @@ package util import ( "net/url" - "path" "strings" ) @@ -19,29 +18,6 @@ func PathEscapeSegments(path string) string { return escapedPath } -// URLJoin joins url components, like path.Join, but preserving contents -// Deprecated: it has unclear behaviors, should not be used anymore. It is only used in some tests. -// Need to be removed in the future. -func URLJoin(base string, elems ...string) string { - if !strings.HasSuffix(base, "/") { - base += "/" - } - baseURL, err := url.Parse(base) - if err != nil { - return "" - } - joinedPath := path.Join(elems...) - argURL, err := url.Parse(joinedPath) - if err != nil { - return "" - } - joinedURL := baseURL.ResolveReference(argURL).String() - if !baseURL.IsAbs() && !strings.HasPrefix(base, "/") { - return joinedURL[1:] // Removing leading '/' if needed - } - return joinedURL -} - func SanitizeURL(s string) (string, error) { u, err := url.Parse(s) if err != nil { diff --git a/modules/util/util_test.go b/modules/util/util_test.go index fd677f5c11..4ec738e89e 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -11,39 +11,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestURLJoin(t *testing.T) { - type test struct { - Expected string - Base string - Elements []string - } - newTest := func(expected, base string, elements ...string) test { - return test{Expected: expected, Base: base, Elements: elements} - } - for _, test := range []test{ - newTest("https://try.gitea.io/a/b/c", - "https://try.gitea.io", "a/b", "c"), - newTest("https://try.gitea.io/a/b/c", - "https://try.gitea.io/", "/a/b/", "/c/"), - newTest("https://try.gitea.io/a/c", - "https://try.gitea.io/", "/a/./b/", "../c/"), - newTest("a/b/c", - "a", "b/c/"), - newTest("a/b/d", - "a/", "b/c/", "/../d/"), - newTest("https://try.gitea.io/a/b/c#d", - "https://try.gitea.io", "a/b", "c#d"), - newTest("/a/b/d", - "/a/", "b/c/", "/../d/"), - newTest("/a/b/c", - "/a", "b/c/"), - newTest("/a/b/c#hash", - "/a", "b/c#hash"), - } { - assert.Equal(t, test.Expected, URLJoin(test.Base, test.Elements...)) - } -} - func TestIsEmptyString(t *testing.T) { cases := []struct { s string diff --git a/services/convert/git_commit_test.go b/services/convert/git_commit_test.go index ad1cc0eca3..66a427fadf 100644 --- a/services/convert/git_commit_test.go +++ b/services/convert/git_commit_test.go @@ -11,7 +11,6 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -35,7 +34,7 @@ func TestToCommitMeta(t *testing.T) { assert.NotNil(t, commitMeta) assert.Equal(t, &api.CommitMeta{ SHA: sha1.EmptyObjectID().String(), - URL: util.URLJoin(headRepo.APIURL(), "git/commits", sha1.EmptyObjectID().String()), + URL: headRepo.APIURL() + "/git/commits/" + sha1.EmptyObjectID().String(), Created: time.Unix(0, 0), }, commitMeta) } diff --git a/tests/integration/api_packages_rpm_test.go b/tests/integration/api_packages_rpm_test.go index 2c9dddc7b4..61bd8ffc63 100644 --- a/tests/integration/api_packages_rpm_test.go +++ b/tests/integration/api_packages_rpm_test.go @@ -20,7 +20,6 @@ import ( user_model "code.gitea.io/gitea/models/user" rpm_module "code.gitea.io/gitea/modules/packages/rpm" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" "github.com/ProtonMail/go-crypto/openpgp" @@ -99,7 +98,7 @@ gpgcheck=1 gpgkey=%sapi/packages/%s/rpm/repository.key`, strings.Join(append([]string{user.LowerName}, groupParts...), "-"), strings.Join(append([]string{user.Name, setting.AppName}, groupParts...), " - "), - util.URLJoin(setting.AppURL, groupURL), + strings.TrimSuffix(setting.AppURL, "/")+groupURL, setting.AppURL, user.Name, ) diff --git a/tests/integration/api_repo_git_tags_test.go b/tests/integration/api_repo_git_tags_test.go index a0445bb800..d4c9f753ff 100644 --- a/tests/integration/api_repo_git_tags_test.go +++ b/tests/integration/api_repo_git_tags_test.go @@ -14,7 +14,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -58,7 +57,7 @@ func TestAPIGitTags(t *testing.T) { assert.Equal(t, aTagMessage+"\n", tag.Message) assert.Equal(t, user.Name, tag.Tagger.Name) assert.Equal(t, user.Email, tag.Tagger.Email) - assert.Equal(t, util.URLJoin(repo.APIURL(), "git/tags", aTag.ID.String()), tag.URL) + assert.Equal(t, repo.APIURL()+"/git/tags/"+aTag.ID.String(), tag.URL) // Should NOT work for lightweight tags badReq := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/git/tags/%s", user.Name, repo.Name, commit.ID.String()).