]> source.dussan.org Git - gitea.git/commitdiff
Refactor AppURL usage (#30885) (#30891)
authorwxiaoguang <wxiaoguang@gmail.com>
Wed, 8 May 2024 13:34:43 +0000 (21:34 +0800)
committerGitHub <noreply@github.com>
Wed, 8 May 2024 13:34:43 +0000 (13:34 +0000)
Backport #30885
Fix #30883
Fix #29591

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
13 files changed:
models/repo/avatar.go
models/user/avatar.go
modules/httplib/url.go
modules/httplib/url_test.go
modules/markup/html_codepreview.go
routers/api/actions/artifacts.go
routers/api/actions/artifactsv4.go
routers/api/packages/container/container.go
routers/common/middleware.go
routers/common/redirect.go
routers/web/auth/auth.go
services/context/base.go
services/context/context_response.go

index 72ee938adae425a93ccb8321166f61b79b1bf887..8395b8c2b741bc66d47766634ee59730c882d3d9 100644 (file)
@@ -9,10 +9,10 @@ import (
        "image/png"
        "io"
        "net/url"
-       "strings"
 
        "code.gitea.io/gitea/models/db"
        "code.gitea.io/gitea/modules/avatar"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/storage"
@@ -84,13 +84,7 @@ func (repo *Repository) relAvatarLink(ctx context.Context) string {
        return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar)
 }
 
-// AvatarLink returns a link to the repository's avatar.
+// 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 (repo *Repository) AvatarLink(ctx context.Context) string {
-       link := repo.relAvatarLink(ctx)
-       // we only prepend our AppURL to our known (relative, internal) avatar link to get an absolute URL
-       if strings.HasPrefix(link, "/") && !strings.HasPrefix(link, "//") {
-               return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:]
-       }
-       // otherwise, return the link as it is
-       return link
+       return httplib.MakeAbsoluteURL(ctx, repo.relAvatarLink(ctx))
 }
index c6937d7b51f76e0b135ae53446eb1d6031b41212..921bc1b1a1ce8d11416d373c8f6d8bf9971e3233 100644 (file)
@@ -9,11 +9,11 @@ import (
        "fmt"
        "image/png"
        "io"
-       "strings"
 
        "code.gitea.io/gitea/models/avatars"
        "code.gitea.io/gitea/models/db"
        "code.gitea.io/gitea/modules/avatar"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/storage"
@@ -89,13 +89,9 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
        return avatars.GenerateEmailAvatarFastLink(ctx, u.AvatarEmail, size)
 }
 
-// AvatarLink returns the full avatar link with http host
+// 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 {
-       link := u.AvatarLinkWithSize(ctx, 0)
-       if !strings.HasPrefix(link, "//") && !strings.Contains(link, "://") {
-               return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL+"/")
-       }
-       return link
+       return httplib.MakeAbsoluteURL(ctx, u.AvatarLinkWithSize(ctx, 0))
 }
 
 // IsUploadAvatarChanged returns true if the current user's avatar would be changed with the provided data
index 903799cb68867acbcbcaee5902053495eb2c1f07..541c4f325bb6302824aed8f181ead7f89e9d3838 100644 (file)
@@ -4,6 +4,8 @@
 package httplib
 
 import (
+       "context"
+       "net/http"
        "net/url"
        "strings"
 
@@ -11,6 +13,10 @@ import (
        "code.gitea.io/gitea/modules/util"
 )
 
+type RequestContextKeyStruct struct{}
+
+var RequestContextKey = RequestContextKeyStruct{}
+
 func urlIsRelative(s string, u *url.URL) bool {
        // Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
        // Therefore we should ignore these redirect locations to prevent open redirects
@@ -26,7 +32,56 @@ func IsRelativeURL(s string) bool {
        return err == nil && urlIsRelative(s, u)
 }
 
-func IsCurrentGiteaSiteURL(s string) bool {
+func guessRequestScheme(req *http.Request, def string) string {
+       // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
+       if s := req.Header.Get("X-Forwarded-Proto"); s != "" {
+               return s
+       }
+       if s := req.Header.Get("X-Forwarded-Protocol"); s != "" {
+               return s
+       }
+       if s := req.Header.Get("X-Url-Scheme"); s != "" {
+               return s
+       }
+       if s := req.Header.Get("Front-End-Https"); s != "" {
+               return util.Iif(s == "on", "https", "http")
+       }
+       if s := req.Header.Get("X-Forwarded-Ssl"); s != "" {
+               return util.Iif(s == "on", "https", "http")
+       }
+       return def
+}
+
+func guessForwardedHost(req *http.Request) string {
+       // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
+       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
+func GuessCurrentAppURL(ctx context.Context) string {
+       req, ok := ctx.Value(RequestContextKey).(*http.Request)
+       if !ok {
+               return setting.AppURL
+       }
+       if host := guessForwardedHost(req); host != "" {
+               // 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
+               return guessRequestScheme(req, "https") + "://" + host + setting.AppSubURL + "/"
+       } else if req.Host != "" {
+               // if it is not behind a reverse proxy, use the scheme from config options, meanwhile use "https" as much as possible
+               defaultScheme := util.Iif(setting.Protocol == "http", "http", "https")
+               return guessRequestScheme(req, defaultScheme) + "://" + req.Host + setting.AppSubURL + "/"
+       }
+       return setting.AppURL
+}
+
+func MakeAbsoluteURL(ctx context.Context, s string) string {
+       if IsRelativeURL(s) {
+               return GuessCurrentAppURL(ctx) + strings.TrimPrefix(s, "/")
+       }
+       return s
+}
+
+func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool {
        u, err := url.Parse(s)
        if err != nil {
                return false
@@ -45,5 +100,6 @@ func IsCurrentGiteaSiteURL(s string) bool {
        if u.Path == "" {
                u.Path = "/"
        }
-       return strings.HasPrefix(strings.ToLower(u.String()), strings.ToLower(setting.AppURL))
+       urlLower := strings.ToLower(u.String())
+       return strings.HasPrefix(urlLower, strings.ToLower(setting.AppURL)) || strings.HasPrefix(urlLower, strings.ToLower(GuessCurrentAppURL(ctx)))
 }
index 9bf09bcf2f3ec17e1df71f1cab876b5c31c6965d..e021cd610d3f40859197ab5f3abad62a85fa45ee 100644 (file)
@@ -4,6 +4,8 @@
 package httplib
 
 import (
+       "context"
+       "net/http"
        "testing"
 
        "code.gitea.io/gitea/modules/setting"
@@ -37,9 +39,44 @@ func TestIsRelativeURL(t *testing.T) {
        }
 }
 
+func TestMakeAbsoluteURL(t *testing.T) {
+       defer test.MockVariableValue(&setting.Protocol, "http")()
+       defer test.MockVariableValue(&setting.AppURL, "http://the-host/sub/")()
+       defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
+
+       ctx := context.Background()
+       assert.Equal(t, "http://the-host/sub/", MakeAbsoluteURL(ctx, ""))
+       assert.Equal(t, "http://the-host/sub/foo", MakeAbsoluteURL(ctx, "foo"))
+       assert.Equal(t, "http://the-host/sub/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://user-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+
+       ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
+               Host: "user-host",
+               Header: map[string][]string{
+                       "X-Forwarded-Host": {"forwarded-host"},
+               },
+       })
+       assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+
+       ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
+               Host: "user-host",
+               Header: map[string][]string{
+                       "X-Forwarded-Host":  {"forwarded-host"},
+                       "X-Forwarded-Proto": {"https"},
+               },
+       })
+       assert.Equal(t, "https://forwarded-host/sub/foo", MakeAbsoluteURL(ctx, "/foo"))
+}
+
 func TestIsCurrentGiteaSiteURL(t *testing.T) {
        defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
        defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
+       ctx := context.Background()
        good := []string{
                "?key=val",
                "/sub",
@@ -50,7 +87,7 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
                "http://localhost:3000/sub/",
        }
        for _, s := range good {
-               assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s)
+               assert.True(t, IsCurrentGiteaSiteURL(ctx, s), "good = %q", s)
        }
        bad := []string{
                ".",
@@ -64,13 +101,23 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
                "http://other/",
        }
        for _, s := range bad {
-               assert.False(t, IsCurrentGiteaSiteURL(s), "bad = %q", s)
+               assert.False(t, IsCurrentGiteaSiteURL(ctx, s), "bad = %q", s)
        }
 
        setting.AppURL = "http://localhost:3000/"
        setting.AppSubURL = ""
-       assert.False(t, IsCurrentGiteaSiteURL("//"))
-       assert.False(t, IsCurrentGiteaSiteURL("\\\\"))
-       assert.False(t, IsCurrentGiteaSiteURL("http://localhost"))
-       assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000?key=val"))
+       assert.False(t, IsCurrentGiteaSiteURL(ctx, "//"))
+       assert.False(t, IsCurrentGiteaSiteURL(ctx, "\\\\"))
+       assert.False(t, IsCurrentGiteaSiteURL(ctx, "http://localhost"))
+       assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000?key=val"))
+
+       ctx = context.WithValue(ctx, RequestContextKey, &http.Request{
+               Host: "user-host",
+               Header: map[string][]string{
+                       "X-Forwarded-Host":  {"forwarded-host"},
+                       "X-Forwarded-Proto": {"https"},
+               },
+       })
+       assert.True(t, IsCurrentGiteaSiteURL(ctx, "http://localhost:3000"))
+       assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host"))
 }
index 5ef2217e3d765b0bca7347ac43dbd113abc86b56..5ab9290b3e42f5187a43b66a5ae5fbbec4cfae65 100644 (file)
@@ -42,7 +42,7 @@ func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosSt
                CommitID:  node.Data[m[6]:m[7]],
                FilePath:  node.Data[m[8]:m[9]],
        }
-       if !httplib.IsCurrentGiteaSiteURL(opts.FullURL) {
+       if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, opts.FullURL) {
                return 0, 0, "", nil
        }
        u, err := url.Parse(opts.FilePath)
index 5bd004bd374a67307197faf0e369313a77ced1b2..35e3ee6906837100935d8efda66e3a937bd88fb6 100644 (file)
@@ -71,6 +71,7 @@ import (
 
        "code.gitea.io/gitea/models/actions"
        "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/json"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
@@ -184,8 +185,8 @@ type artifactRoutes struct {
        fs     storage.ObjectStorage
 }
 
-func (ar artifactRoutes) buildArtifactURL(runID int64, artifactHash, suffix string) string {
-       uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(ar.prefix, "/") +
+func (ar artifactRoutes) buildArtifactURL(ctx *ArtifactContext, runID int64, artifactHash, suffix string) string {
+       uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(ar.prefix, "/") +
                strings.ReplaceAll(artifactRouteBase, "{run_id}", strconv.FormatInt(runID, 10)) +
                "/" + artifactHash + "/" + suffix
        return uploadURL
@@ -224,7 +225,7 @@ func (ar artifactRoutes) getUploadArtifactURL(ctx *ArtifactContext) {
        // use md5(artifact_name) to create upload url
        artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(req.Name)))
        resp := getUploadArtifactResponse{
-               FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "upload"+retentionQuery),
+               FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "upload"+retentionQuery),
        }
        log.Debug("[artifact] get upload url: %s", resp.FileContainerResourceURL)
        ctx.JSON(http.StatusOK, resp)
@@ -365,7 +366,7 @@ func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) {
                artifactHash := fmt.Sprintf("%x", md5.Sum([]byte(art.ArtifactName)))
                item := listArtifactsResponseItem{
                        Name:                     art.ArtifactName,
-                       FileContainerResourceURL: ar.buildArtifactURL(runID, artifactHash, "download_url"),
+                       FileContainerResourceURL: ar.buildArtifactURL(ctx, runID, artifactHash, "download_url"),
                }
                items = append(items, item)
                values[art.ArtifactName] = true
@@ -437,7 +438,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
                        }
                }
                if downloadURL == "" {
-                       downloadURL = ar.buildArtifactURL(runID, strconv.FormatInt(artifact.ID, 10), "download")
+                       downloadURL = ar.buildArtifactURL(ctx, runID, strconv.FormatInt(artifact.ID, 10), "download")
                }
                item := downloadArtifactResponseItem{
                        Path:            util.PathJoinRel(itemPath, artifact.ArtifactPath),
index 8300989c75ce5c9fbb6a095a4740f9d389955f84..dde9caf4f26b2afa3e778e3f03e73c1bf88cce4c 100644 (file)
@@ -92,6 +92,7 @@ import (
 
        "code.gitea.io/gitea/models/actions"
        "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/storage"
@@ -160,9 +161,9 @@ func (r artifactV4Routes) buildSignature(endp, expires, artifactName string, tas
        return mac.Sum(nil)
 }
 
-func (r artifactV4Routes) buildArtifactURL(endp, artifactName string, taskID int64) string {
+func (r artifactV4Routes) buildArtifactURL(ctx *ArtifactContext, endp, artifactName string, taskID int64) string {
        expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST")
-       uploadURL := strings.TrimSuffix(setting.AppURL, "/") + strings.TrimSuffix(r.prefix, "/") +
+       uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + strings.TrimSuffix(r.prefix, "/") +
                "/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(r.buildSignature(endp, expires, artifactName, taskID)) + "&expires=" + url.QueryEscape(expires) + "&artifactName=" + url.QueryEscape(artifactName) + "&taskID=" + fmt.Sprint(taskID)
        return uploadURL
 }
@@ -278,7 +279,7 @@ func (r *artifactV4Routes) createArtifact(ctx *ArtifactContext) {
 
        respData := CreateArtifactResponse{
                Ok:              true,
-               SignedUploadUrl: r.buildArtifactURL("UploadArtifact", artifactName, ctx.ActionTask.ID),
+               SignedUploadUrl: r.buildArtifactURL(ctx, "UploadArtifact", artifactName, ctx.ActionTask.ID),
        }
        r.sendProtbufBody(ctx, &respData)
 }
@@ -454,7 +455,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
                }
        }
        if respData.SignedUrl == "" {
-               respData.SignedUrl = r.buildArtifactURL("DownloadArtifact", artifactName, ctx.ActionTask.ID)
+               respData.SignedUrl = r.buildArtifactURL(ctx, "DownloadArtifact", artifactName, ctx.ActionTask.ID)
        }
        r.sendProtbufBody(ctx, &respData)
 }
index 2cb16daebc578bf2c8f66ee1a78092e465e30a6d..1efd166eb3e650a34cfe79545c4f7f10db6f5a9d 100644 (file)
@@ -17,6 +17,7 @@ import (
        packages_model "code.gitea.io/gitea/models/packages"
        container_model "code.gitea.io/gitea/models/packages/container"
        user_model "code.gitea.io/gitea/models/user"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/json"
        "code.gitea.io/gitea/modules/log"
        packages_module "code.gitea.io/gitea/modules/packages"
@@ -115,7 +116,7 @@ func apiErrorDefined(ctx *context.Context, err *namedError) {
 }
 
 func apiUnauthorizedError(ctx *context.Context) {
-       ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+setting.AppURL+`v2/token",service="container_registry",scope="*"`)
+       ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+httplib.GuessCurrentAppURL(ctx)+`v2/token",service="container_registry",scope="*"`)
        apiErrorDefined(ctx, errUnauthorized)
 }
 
index c7c75fb099b468537365ceead3cdd1d8fec49fa8..8b661993bb34046e2155b183088e6735ffb4f0e2 100644 (file)
@@ -4,11 +4,13 @@
 package common
 
 import (
+       go_context "context"
        "fmt"
        "net/http"
        "strings"
 
        "code.gitea.io/gitea/modules/cache"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/process"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/web/middleware"
@@ -34,6 +36,7 @@ func ProtocolMiddlewares() (handlers []any) {
                                }
                        }()
                        req = req.WithContext(middleware.WithContextData(req.Context()))
+                       req = req.WithContext(go_context.WithValue(req.Context(), httplib.RequestContextKey, req))
                        next.ServeHTTP(resp, req)
                })
        })
index 34044e814bc61cb43b3db9458ac82cb4ca62245b..d64f74ec82afa7635c6aabe8561957f085a92b14 100644 (file)
@@ -17,7 +17,7 @@ func FetchRedirectDelegate(resp http.ResponseWriter, req *http.Request) {
        // The typical page is "issue comment" page. The backend responds "/owner/repo/issues/1#comment-2",
        // then frontend needs this delegate to redirect to the new location with hash correctly.
        redirect := req.PostFormValue("redirect")
-       if !httplib.IsCurrentGiteaSiteURL(redirect) {
+       if !httplib.IsCurrentGiteaSiteURL(req.Context(), redirect) {
                resp.WriteHeader(http.StatusBadRequest)
                return
        }
index 7c873796fe40e936f4e7dbd05aef5c85d99c4dc9..4083d64226cfa0d89dd4a76b691c84838f4b4932 100644 (file)
@@ -368,7 +368,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
                return setting.AppSubURL + "/"
        }
 
-       if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(redirectTo) {
+       if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(ctx, redirectTo) {
                middleware.DeleteRedirectToCookie(ctx.Resp)
                if obeyRedirect {
                        ctx.RedirectToCurrentSite(redirectTo)
index 05b8ab1b9be0b9413f1f4141d558b5c542ad1e21..29e62ae38924aeaab0296fe5f65dd42966323c67 100644 (file)
@@ -254,7 +254,7 @@ func (b *Base) Redirect(location string, status ...int) {
                code = status[0]
        }
 
-       if strings.HasPrefix(location, "http://") || strings.HasPrefix(location, "https://") || strings.HasPrefix(location, "//") {
+       if !httplib.IsRelativeURL(location) {
                // Some browsers (Safari) have buggy behavior for Cookie + Cache + External Redirection, eg: /my-path => https://other/path
                // 1. the first request to "/my-path" contains cookie
                // 2. some time later, the request to "/my-path" doesn't contain cookie (caused by Prevent web tracking)
index 87c34c35edebe8f226ef75bb289e4759b84cf260..c43a649b49e124de15158add8efc990d46cd3e8a 100644 (file)
@@ -52,7 +52,7 @@ func (ctx *Context) RedirectToCurrentSite(location ...string) {
                        continue
                }
 
-               if !httplib.IsCurrentGiteaSiteURL(loc) {
+               if !httplib.IsCurrentGiteaSiteURL(ctx, loc) {
                        continue
                }