Fix duplicate sub-path for avatars (#31365) (#31368)

Backport #31365, only backport necessary changes.

diff --git a/models/repo/avatar_test.go b/models/repo/avatar_test.go
new file mode 100644 (file)
index 0000000..fc1f8ba
--- /dev/null
+++ b/models/repo/avatar_test.go
@@ -0,0 +1,28 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package repo
+
+import (
+       "testing"
+
+       "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/test"
+
+       "github.com/stretchr/testify/assert"
+)
+
+func TestRepoAvatarLink(t *testing.T) {
+       defer test.MockVariableValue(&setting.AppURL, "https://localhost/")()
+       defer test.MockVariableValue(&setting.AppSubURL, "")()
+
+       repo := &Repository{ID: 1, Avatar: "avatar.png"}
+       link := repo.AvatarLink(db.DefaultContext)
+       assert.Equal(t, "https://localhost/repo-avatars/avatar.png", link)
+
+       setting.AppURL = "https://localhost/sub-path/"
+       setting.AppSubURL = "/sub-path"
+       link = repo.AvatarLink(db.DefaultContext)
+       assert.Equal(t, "https://localhost/sub-path/repo-avatars/avatar.png", link)
+}

diff --git a/models/user/avatar.go b/models/user/avatar.go
index 921bc1b1a1ce8d11416d373c8f6d8bf9971e3233..5453c78fc61dbdcf1cbde3a422e45e307b2bd785 100644 (file)
--- a/models/user/avatar.go
+++ b/models/user/avatar.go
@@ -89,9 +89,11 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
        return avatars.GenerateEmailAvatarFastLink(ctx, u.AvatarEmail, size)
 }
 
-// AvatarLink returns the full avatar url with http host. TODO: refactor it to a relative URL, but it is still used in API response at the moment
+// AvatarLink returns the full avatar url with http host.
+// TODO: refactor it to a relative URL, but it is still used in API response at the moment
 func (u *User) AvatarLink(ctx context.Context) string {
-       return httplib.MakeAbsoluteURL(ctx, u.AvatarLinkWithSize(ctx, 0))
+       relLink := u.AvatarLinkWithSize(ctx, 0) // it can't be empty
+       return httplib.MakeAbsoluteURL(ctx, relLink)
 }
 
 // IsUploadAvatarChanged returns true if the current user's avatar would be changed with the provided data

diff --git a/models/user/avatar_test.go b/models/user/avatar_test.go
new file mode 100644 (file)
index 0000000..1078875
--- /dev/null
+++ b/models/user/avatar_test.go
@@ -0,0 +1,28 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package user
+
+import (
+       "testing"
+
+       "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/test"
+
+       "github.com/stretchr/testify/assert"
+)
+
+func TestUserAvatarLink(t *testing.T) {
+       defer test.MockVariableValue(&setting.AppURL, "https://localhost/")()
+       defer test.MockVariableValue(&setting.AppSubURL, "")()
+
+       u := &User{ID: 1, Avatar: "avatar.png"}
+       link := u.AvatarLink(db.DefaultContext)
+       assert.Equal(t, "https://localhost/avatars/avatar.png", link)
+
+       setting.AppURL = "https://localhost/sub-path/"
+       setting.AppSubURL = "/sub-path"
+       link = u.AvatarLink(db.DefaultContext)
+       assert.Equal(t, "https://localhost/sub-path/avatars/avatar.png", link)
+}

diff --git a/modules/httplib/url.go b/modules/httplib/url.go
index 8dc5b71181abd7ac67715fb8d7a1db043a1b6daa..219dfe695c4d58a705eb4eebf05ef30599daa547 100644 (file)
--- a/modules/httplib/url.go
+++ b/modules/httplib/url.go
@@ -57,11 +57,16 @@ func getForwardedHost(req *http.Request) string {
        return req.Header.Get("X-Forwarded-Host")
 }
 
-// GuessCurrentAppURL tries to guess the current full URL by http headers. It always has a '/' suffix, exactly the same as setting.AppURL
+// GuessCurrentAppURL tries to guess the current full app URL (with sub-path) by http headers. It always has a '/' suffix, exactly the same as setting.AppURL
 func GuessCurrentAppURL(ctx context.Context) string {
+       return GuessCurrentHostURL(ctx) + setting.AppSubURL + "/"
+}
+
+// GuessCurrentHostURL tries to guess the current full host URL (no sub-path) by http headers, there is no trailing slash.
+func GuessCurrentHostURL(ctx context.Context) string {
        req, ok := ctx.Value(RequestContextKey).(*http.Request)
        if !ok {
-               return setting.AppURL
+               return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/")
        }
        // If no scheme provided by reverse proxy, then do not guess the AppURL, use the configured one.
        // At the moment, if site admin doesn't configure the proxy headers correctly, then Gitea would guess wrong.
@@ -74,20 +79,27 @@ func GuessCurrentAppURL(ctx context.Context) string {
        // So in the future maybe it should introduce a new config option, to let site admin decide how to guess the AppURL.
        reqScheme := getRequestScheme(req)
        if reqScheme == "" {
-               return setting.AppURL
+               return strings.TrimSuffix(setting.AppURL, setting.AppSubURL+"/")
        }
        reqHost := getForwardedHost(req)
        if reqHost == "" {
                reqHost = req.Host
        }
-       return reqScheme + "://" + reqHost + setting.AppSubURL + "/"
+       return reqScheme + "://" + reqHost
 }
 
-func MakeAbsoluteURL(ctx context.Context, s string) string {
-       if IsRelativeURL(s) {
-               return GuessCurrentAppURL(ctx) + strings.TrimPrefix(s, "/")
+// MakeAbsoluteURL tries to make a link to an absolute URL:
+// * If link is empty, it returns the current app URL.
+// * If link is absolute, it returns the link.
+// * Otherwise, it returns the current host URL + link, the link itself should have correct sub-path (AppSubURL) if needed.
+func MakeAbsoluteURL(ctx context.Context, link string) string {
+       if link == "" {
+               return GuessCurrentAppURL(ctx)
+       }
+       if !IsRelativeURL(link) {
+               return link
        }
-       return s
+       return GuessCurrentHostURL(ctx) + "/" + strings.TrimPrefix(link, "/")
 }
 
 func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool {

diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go
index 9980cb74e8b29b76c5b3d11b41ca647741429df0..28aaee6e1293cf1d2e2a2aa81d99d46ebd902c90 100644 (file)
--- a/modules/httplib/url_test.go
+++ b/modules/httplib/url_test.go
@@ -46,14 +46,14 @@ func TestMakeAbsoluteURL(t *testing.T) {
 
        ctx := context.Background()
        assert.Equal(t, "http://cfg-host/sub/", MakeAbsoluteURL(ctx, ""))
-       assert.Equal(t, "http://cfg-host/sub/foo", MakeAbsoluteURL(ctx, "foo"))
-       assert.Equal(t, "http://cfg-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+       assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "foo"))
+       assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo"))
        assert.Equal(t, "http://other/foo", MakeAbsoluteURL(ctx, "http://other/foo"))
 
        ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
                Host: "user-host",
        })
-       assert.Equal(t, "http://cfg-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+       assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo"))
 
        ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
                Host: "user-host",
@@ -61,7 +61,7 @@ func TestMakeAbsoluteURL(t *testing.T) {
                        "X-Forwarded-Host": {"forwarded-host"},
                },
        })
-       assert.Equal(t, "http://cfg-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+       assert.Equal(t, "http://cfg-host/foo", MakeAbsoluteURL(ctx, "/foo"))
 
        ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
                Host: "user-host",
@@ -70,7 +70,7 @@ func TestMakeAbsoluteURL(t *testing.T) {
                        "X-Forwarded-Proto": {"https"},
                },
        })
-       assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+       assert.Equal(t, "https://forwarded-host/foo", MakeAbsoluteURL(ctx, "/foo"))
 }
 
 func TestIsCurrentGiteaSiteURL(t *testing.T) {

diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index b0c4458d5114ee9ae1fe1983556ca54ef1d0e1f4..5007037beebf4e8bdabd0deb279ae8bb71611ac5 100644 (file)
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -117,7 +117,7 @@ func apiErrorDefined(ctx *context.Context, err *namedError) {
 
 func apiUnauthorizedError(ctx *context.Context) {
        // container registry requires that the "/v2" must be in the root, so the sub-path in AppURL should be removed
-       realmURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), setting.AppSubURL+"/") + "/v2/token"
+       realmURL := httplib.GuessCurrentHostURL(ctx) + "/v2/token"
        ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+realmURL+`",service="container_registry",scope="*"`)
        apiErrorDefined(ctx, errUnauthorized)
 }