Skip to content

Commit d4c2db3

Browse files
wxiaoguangKN4CK3R
wxiaoguang
and
KN4CK3R
authored
Refactor AppURL usage (#30885) (#30891)
Backport #30885 Fix #30883 Fix #29591 Co-authored-by: KN4CK3R <[email protected]>
1 parent d410e2a commit d4c2db3

File tree

13 files changed

+138
-39
lines changed

13 files changed

+138
-39
lines changed

‎models/repo/avatar.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import (
99
"image/png"
1010
"io"
1111
"net/url"
12-
"strings"
1312

1413
"code.gitea.io/gitea/models/db"
1514
"code.gitea.io/gitea/modules/avatar"
15+
"code.gitea.io/gitea/modules/httplib"
1616
"code.gitea.io/gitea/modules/log"
1717
"code.gitea.io/gitea/modules/setting"
1818
"code.gitea.io/gitea/modules/storage"
@@ -84,13 +84,7 @@ func (repo *Repository) relAvatarLink(ctx context.Context) string {
8484
return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar)
8585
}
8686

87-
// AvatarLink returns a link to the repository's avatar.
87+
// 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
8888
func (repo *Repository) AvatarLink(ctx context.Context) string {
89-
link := repo.relAvatarLink(ctx)
90-
// we only prepend our AppURL to our known (relative, internal) avatar link to get an absolute URL
91-
if strings.HasPrefix(link, "/") && !strings.HasPrefix(link, "//") {
92-
return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:]
93-
}
94-
// otherwise, return the link as it is
95-
return link
89+
return httplib.MakeAbsoluteURL(ctx, repo.relAvatarLink(ctx))
9690
}

‎models/user/avatar.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ import (
99
"fmt"
1010
"image/png"
1111
"io"
12-
"strings"
1312

1413
"code.gitea.io/gitea/models/avatars"
1514
"code.gitea.io/gitea/models/db"
1615
"code.gitea.io/gitea/modules/avatar"
16+
"code.gitea.io/gitea/modules/httplib"
1717
"code.gitea.io/gitea/modules/log"
1818
"code.gitea.io/gitea/modules/setting"
1919
"code.gitea.io/gitea/modules/storage"
@@ -89,13 +89,9 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
8989
return avatars.GenerateEmailAvatarFastLink(ctx, u.AvatarEmail, size)
9090
}
9191

92-
// AvatarLink returns the full avatar link with http host
92+
// 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
9393
func (u *User) AvatarLink(ctx context.Context) string {
94-
link := u.AvatarLinkWithSize(ctx, 0)
95-
if !strings.HasPrefix(link, "//") && !strings.Contains(link, "://") {
96-
return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL+"/")
97-
}
98-
return link
94+
return httplib.MakeAbsoluteURL(ctx, u.AvatarLinkWithSize(ctx, 0))
9995
}
10096

10197
// IsUploadAvatarChanged returns true if the current user's avatar would be changed with the provided data

‎modules/httplib/url.go

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,19 @@
44
package httplib
55

66
import (
7+
"context"
8+
"net/http"
79
"net/url"
810
"strings"
911

1012
"code.gitea.io/gitea/modules/setting"
1113
"code.gitea.io/gitea/modules/util"
1214
)
1315

16+
type RequestContextKeyStruct struct{}
17+
18+
var RequestContextKey = RequestContextKeyStruct{}
19+
1420
func urlIsRelative(s string, u *url.URL) bool {
1521
// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
1622
// Therefore we should ignore these redirect locations to prevent open redirects
@@ -26,7 +32,56 @@ func IsRelativeURL(s string) bool {
2632
return err == nil && urlIsRelative(s, u)
2733
}
2834

29-
func IsCurrentGiteaSiteURL(s string) bool {
35+
func guessRequestScheme(req *http.Request, def string) string {
36+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
37+
if s := req.Header.Get("X-Forwarded-Proto"); s != "" {
38+
return s
39+
}
40+
if s := req.Header.Get("X-Forwarded-Protocol"); s != "" {
41+
return s
42+
}
43+
if s := req.Header.Get("X-Url-Scheme"); s != "" {
44+
return s
45+
}
46+
if s := req.Header.Get("Front-End-Https"); s != "" {
47+
return util.Iif(s == "on", "https", "http")
48+
}
49+
if s := req.Header.Get("X-Forwarded-Ssl"); s != "" {
50+
return util.Iif(s == "on", "https", "http")
51+
}
52+
return def
53+
}
54+
55+
func guessForwardedHost(req *http.Request) string {
56+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
57+
return req.Header.Get("X-Forwarded-Host")
58+
}
59+
60+
// GuessCurrentAppURL tries to guess the current full URL by http headers. It always has a '/' suffix, exactly the same as setting.AppURL
61+
func GuessCurrentAppURL(ctx context.Context) string {
62+
req, ok := ctx.Value(RequestContextKey).(*http.Request)
63+
if !ok {
64+
return setting.AppURL
65+
}
66+
if host := guessForwardedHost(req); host != "" {
67+
// if it is behind a reverse proxy, use "https" as default scheme in case the site admin forgets to set the correct forwarded-protocol headers
68+
return guessRequestScheme(req, "https") + "://" + host + setting.AppSubURL + "/"
69+
} else if req.Host != "" {
70+
// if it is not behind a reverse proxy, use the scheme from config options, meanwhile use "https" as much as possible
71+
defaultScheme := util.Iif(setting.Protocol == "http", "http", "https")
72+
return guessRequestScheme(req, defaultScheme) + "://" + req.Host + setting.AppSubURL + "/"
73+
}
74+
return setting.AppURL
75+
}
76+
77+
func MakeAbsoluteURL(ctx context.Context, s string) string {
78+
if IsRelativeURL(s) {
79+
return GuessCurrentAppURL(ctx) + strings.TrimPrefix(s, "/")
80+
}
81+
return s
82+
}
83+
84+
func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool {
3085
u, err := url.Parse(s)
3186
if err != nil {
3287
return false
@@ -45,5 +100,6 @@ func IsCurrentGiteaSiteURL(s string) bool {
45100
if u.Path == "" {
46101
u.Path = "/"
47102
}
48-
return strings.HasPrefix(strings.ToLower(u.String()), strings.ToLower(setting.AppURL))
103+
urlLower := strings.ToLower(u.String())
104+
return strings.HasPrefix(urlLower, strings.ToLower(setting.AppURL)) || strings.HasPrefix(urlLower, strings.ToLower(GuessCurrentAppURL(ctx)))
49105
}

‎modules/httplib/url_test.go

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package httplib
55

66
import (
7+
"context"
8+
"net/http"
79
"testing"
810

911
"code.gitea.io/gitea/modules/setting"
@@ -37,9 +39,44 @@ func TestIsRelativeURL(t *testing.T) {
3739
}
3840
}
3941

42+
func TestMakeAbsoluteURL(t *testing.T) {
43+
defer test.MockVariableValue(&setting.Protocol, "http")()
44+
defer test.MockVariableValue(&setting.AppURL, "http://the-host/sub/")()
45+
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
46+
47+
ctx := context.Background()
48+
assert.Equal(t, "http://the-host/sub/", MakeAbsoluteURL(ctx, ""))
49+
assert.Equal(t, "http://the-host/sub/foo", MakeAbsoluteURL(ctx, "foo"))
50+
assert.Equal(t, "http://the-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
51+
assert.Equal(t, "http://other/foo", MakeAbsoluteURL(ctx, "http://other/foo"))
52+
53+
ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
54+
Host: "user-host",
55+
})
56+
assert.Equal(t, "http://user-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
57+
58+
ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
59+
Host: "user-host",
60+
Header: map[string][]string{
61+
"X-Forwarded-Host": {"forwarded-host"},
62+
},
63+
})
64+
assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
65+
66+
ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
67+
Host: "user-host",
68+
Header: map[string][]string{
69+
"X-Forwarded-Host": {"forwarded-host"},
70+
"X-Forwarded-Proto": {"https"},
71+
},
72+
})
73+
assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
74+
}
75+
4076
func TestIsCurrentGiteaSiteURL(t *testing.T) {
4177
defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
4278
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
79+
ctx := context.Background()
4380
good := []string{
4481
"?key=val",
4582
"/sub",
@@ -50,7 +87,7 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
5087
"http://localhost:3000/sub/",
5188
}
5289
for _, s := range good {
53-
assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s)
90+
assert.True(t, IsCurrentGiteaSiteURL(ctx, s), "good = %q", s)
5491
}
5592
bad := []string{
5693
".",
@@ -64,13 +101,23 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
64101
"http://other/",
65102
}
66103
for _, s := range bad {
67-
assert.False(t, IsCurrentGiteaSiteURL(s), "bad = %q", s)
104+
assert.False(t, IsCurrentGiteaSiteURL(ctx, s), "bad = %q", s)
68105
}
69106

70107
setting.AppURL = "http://localhost:3000/"
71108
setting.AppSubURL = ""
72-
assert.False(t, IsCurrentGiteaSiteURL("//"))
73-
assert.False(t, IsCurrentGiteaSiteURL("\\\\"))
74-
assert.False(t, IsCurrentGiteaSiteURL("http://localhost"))
75-
assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000?key=val"))
109+
assert.False(t, IsCurrentGiteaSiteURL(ctx, "//"))
110+
assert.False(t, IsCurrentGiteaSiteURL(ctx, "\\\\"))
111+
assert.False(t, IsCurrentGiteaSiteURL(ctx, "http://localhost"))
112+
assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000?key=val"))
113+
114+
ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
115+
Host: "user-host",
116+
Header: map[string][]string{
117+
"X-Forwarded-Host": {"forwarded-host"},
118+
"X-Forwarded-Proto": {"https"},
119+
},
120+
})
121+
assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000"))
122+
assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host"))
76123
}

‎modules/markup/html_codepreview.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosSt
4242
CommitID: node.Data[m[6]:m[7]],
4343
FilePath: node.Data[m[8]:m[9]],
4444
}
45-
if !httplib.IsCurrentGiteaSiteURL(opts.FullURL) {
45+
if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, opts.FullURL) {
4646
return 0, 0, "", nil
4747
}
4848
u, err := url.Parse(opts.FilePath)

‎routers/api/actions/artifacts.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import (
7171

7272
"code.gitea.io/gitea/models/actions"
7373
"code.gitea.io/gitea/models/db"
74+
"code.gitea.io/gitea/modules/httplib"
7475
"code.gitea.io/gitea/modules/json"
7576
"code.gitea.io/gitea/modules/log"
7677
"code.gitea.io/gitea/modules/setting"
@@ -184,8 +185,8 @@ type artifactRoutes struct {
184185
fs storage.ObjectStorage
185186
}
186187

187-
func (ar artifactRoutes) buildArtifactURL(runID int64, artifactHash, suffix string) string {
188-
uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") +
188+
func (ar artifactRoutes) buildArtifactURL(ctx *ArtifactContext, runID int64, artifactHash, suffix string) string {
189+
uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(ar.prefix, "/") +
189190
strings.ReplaceAll(artifactRouteBase, "{run_id}", strconv.FormatInt(runID, 10)) +
190191
"/" + artifactHash + "/" + suffix
191192
return uploadURL
@@ -224,7 +225,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *ArtifactContext) {
224225
// use md5(artifact_name) to create upload url
225226
artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(req.Name)))
226227
resp := getUploadArtifactResponse{
227-
FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "upload"+retentionQuery),
228+
FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "upload"+retentionQuery),
228229
}
229230
log.Debug("[artifact] get upload url: %s", resp.FileContainerResourceURL)
230231
ctx.JSON(http.StatusOK, resp)
@@ -365,7 +366,7 @@ func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) {
365366
artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(art.ArtifactName)))
366367
item := listArtifactsResponseItem{
367368
Name: art.ArtifactName,
368-
FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "download_url"),
369+
FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "download_url"),
369370
}
370371
items = append(items, item)
371372
values[art.ArtifactName] = true
@@ -437,7 +438,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
437438
}
438439
}
439440
if downloadURL == "" {
440-
downloadURL = ar.buildArtifactURL(runID, strconv.FormatInt(artifact.ID, 10), "download")
441+
downloadURL = ar.buildArtifactURL(ctx, runID, strconv.FormatInt(artifact.ID, 10), "download")
441442
}
442443
item := downloadArtifactResponseItem{
443444
Path: util.PathJoinRel(itemPath, artifact.ArtifactPath),

‎routers/api/actions/artifactsv4.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import (
9292

9393
"code.gitea.io/gitea/models/actions"
9494
"code.gitea.io/gitea/models/db"
95+
"code.gitea.io/gitea/modules/httplib"
9596
"code.gitea.io/gitea/modules/log"
9697
"code.gitea.io/gitea/modules/setting"
9798
"code.gitea.io/gitea/modules/storage"
@@ -160,9 +161,9 @@ func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, tas
160161
return mac.Sum(nil)
161162
}
162163

163-
func (r artifactV4Routes) buildArtifactURL(endp, artifactName string, taskID int64) string {
164+
func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactName string, taskID int64) string {
164165
expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST")
165-
uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(r.prefix, "/") +
166+
uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(r.prefix, "/") +
166167
"/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + fmt.Sprint(taskID)
167168
return uploadURL
168169
}
@@ -278,7 +279,7 @@ func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) {
278279

279280
respData := CreateArtifactResponse{
280281
Ok: true,
281-
SignedUploadUrl: r.buildArtifactURL("UploadArtifact", artifactName, ctx.ActionTask.ID),
282+
SignedUploadUrl: r.buildArtifactURL(ctx, "UploadArtifact", artifactName, ctx.ActionTask.ID),
282283
}
283284
r.sendProtbufBody(ctx, &respData)
284285
}
@@ -454,7 +455,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
454455
}
455456
}
456457
if respData.SignedUrl == "" {
457-
respData.SignedUrl = r.buildArtifactURL("DownloadArtifact", artifactName, ctx.ActionTask.ID)
458+
respData.SignedUrl = r.buildArtifactURL(ctx, "DownloadArtifact", artifactName, ctx.ActionTask.ID)
458459
}
459460
r.sendProtbufBody(ctx, &respData)
460461
}

‎routers/api/packages/container/container.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
packages_model "code.gitea.io/gitea/models/packages"
1818
container_model "code.gitea.io/gitea/models/packages/container"
1919
user_model "code.gitea.io/gitea/models/user"
20+
"code.gitea.io/gitea/modules/httplib"
2021
"code.gitea.io/gitea/modules/json"
2122
"code.gitea.io/gitea/modules/log"
2223
packages_module "code.gitea.io/gitea/modules/packages"
@@ -115,7 +116,7 @@ func apiErrorDefined(ctx *context.Context, err *namedError) {
115116
}
116117

117118
func apiUnauthorizedError(ctx *context.Context) {
118-
ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+setting.AppURL+`v2/token",service="container_registry",scope="*"`)
119+
ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+httplib.GuessCurrentAppURL(ctx)+`v2/token",service="container_registry",scope="*"`)
119120
apiErrorDefined(ctx, errUnauthorized)
120121
}
121122

‎routers/common/middleware.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
package common
55

66
import (
7+
go_context "context"
78
"fmt"
89
"net/http"
910
"strings"
1011

1112
"code.gitea.io/gitea/modules/cache"
13+
"code.gitea.io/gitea/modules/httplib"
1214
"code.gitea.io/gitea/modules/process"
1315
"code.gitea.io/gitea/modules/setting"
1416
"code.gitea.io/gitea/modules/web/middleware"
@@ -34,6 +36,7 @@ func ProtocolMiddlewares() (handlers []any) {
3436
}
3537
}()
3638
req = req.WithContext(middleware.WithContextData(req.Context()))
39+
req = req.WithContext(go_context.WithValue(req.Context(), httplib.RequestContextKey, req))
3740
next.ServeHTTP(resp, req)
3841
})
3942
})

‎routers/common/redirect.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func FetchRedirectDelegate(resp http.ResponseWriter, req *http.Request) {
1717
// The typical page is "issue comment" page. The backend responds "/owner/repo/issues/1#comment-2",
1818
// then frontend needs this delegate to redirect to the new location with hash correctly.
1919
redirect := req.PostFormValue("redirect")
20-
if !httplib.IsCurrentGiteaSiteURL(redirect) {
20+
if !httplib.IsCurrentGiteaSiteURL(req.Context(), redirect) {
2121
resp.WriteHeader(http.StatusBadRequest)
2222
return
2323
}

‎routers/web/auth/auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
368368
return setting.AppSubURL + "/"
369369
}
370370

371-
if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(redirectTo) {
371+
if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(ctx, redirectTo) {
372372
middleware.DeleteRedirectToCookie(ctx.Resp)
373373
if obeyRedirect {
374374
ctx.RedirectToCurrentSite(redirectTo)

‎services/context/base.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func (b *Base) Redirect(location string, status ...int) {
254254
code = status[0]
255255
}
256256

257-
if strings.HasPrefix(location, "http://") || strings.HasPrefix(location, "https://") || strings.HasPrefix(location, "//") {
257+
if !httplib.IsRelativeURL(location) {
258258
// Some browsers (Safari) have buggy behavior for Cookie + Cache + External Redirection, eg: /my-path => https://other/path
259259
// 1. the first request to "/my-path" contains cookie
260260
// 2. some time later, the request to "/my-path" doesn't contain cookie (caused by Prevent web tracking)

0 commit comments

Comments
 (0)