aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwxiaoguang <wxiaoguang@gmail.com>2024-05-07 16:26:13 +0800
committerGitHub <noreply@github.com>2024-05-07 08:26:13 +0000
commit67c1a07285008cc00036a87cef966c3bd519a50c (patch)
tree19d4ff344c4782596442a8c8de0a20142b0ca8ed
parentebf0c969403d91ed80745ff5bd7dfbdb08174fc7 (diff)
downloadgitea-67c1a07285008cc00036a87cef966c3bd519a50c.tar.gz
gitea-67c1a07285008cc00036a87cef966c3bd519a50c.zip
Refactor AppURL usage (#30885)
Fix #30883 Fix #29591 --------- Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
-rw-r--r--models/repo/avatar.go12
-rw-r--r--models/user/avatar.go10
-rw-r--r--modules/httplib/url.go60
-rw-r--r--modules/httplib/url_test.go59
-rw-r--r--modules/markup/html_codepreview.go2
-rw-r--r--routers/api/actions/artifacts.go11
-rw-r--r--routers/api/actions/artifactsv4.go9
-rw-r--r--routers/api/packages/container/container.go3
-rw-r--r--routers/common/middleware.go3
-rw-r--r--routers/common/redirect.go2
-rw-r--r--routers/web/auth/auth.go2
-rw-r--r--services/context/base.go2
-rw-r--r--services/context/context_response.go2
13 files changed, 138 insertions, 39 deletions
diff --git a/models/repo/avatar.go b/models/repo/avatar.go
index 72ee938ada..8395b8c2b7 100644
--- a/models/repo/avatar.go
+++ b/models/repo/avatar.go
@@ -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))
}
diff --git a/models/user/avatar.go b/models/user/avatar.go
index c6937d7b51..921bc1b1a1 100644
--- a/models/user/avatar.go
+++ b/models/user/avatar.go
@@ -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
diff --git a/modules/httplib/url.go b/modules/httplib/url.go
index 903799cb68..541c4f325b 100644
--- a/modules/httplib/url.go
+++ b/modules/httplib/url.go
@@ -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)))
}
diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go
index 9bf09bcf2f..e021cd610d 100644
--- a/modules/httplib/url_test.go
+++ b/modules/httplib/url_test.go
@@ -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"))
}
diff --git a/modules/markup/html_codepreview.go b/modules/markup/html_codepreview.go
index 5ef2217e3d..5ab9290b3e 100644
--- a/modules/markup/html_codepreview.go
+++ b/modules/markup/html_codepreview.go
@@ -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)
diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go
index 5bd004bd37..35e3ee6906 100644
--- a/routers/api/actions/artifacts.go
+++ b/routers/api/actions/artifacts.go
@@ -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),
diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go
index 8300989c75..dde9caf4f2 100644
--- a/routers/api/actions/artifactsv4.go
+++ b/routers/api/actions/artifactsv4.go
@@ -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)
}
diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index 2cb16daebc..1efd166eb3 100644
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -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)
}
diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index c7c75fb099..8b661993bb 100644
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -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)
})
})
diff --git a/routers/common/redirect.go b/routers/common/redirect.go
index 34044e814b..d64f74ec82 100644
--- a/routers/common/redirect.go
+++ b/routers/common/redirect.go
@@ -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
}
diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
index 7c873796fe..4083d64226 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -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)
diff --git a/services/context/base.go b/services/context/base.go
index 05b8ab1b9b..29e62ae389 100644
--- a/services/context/base.go
+++ b/services/context/base.go
@@ -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)
diff --git a/services/context/context_response.go b/services/context/context_response.go
index 87c34c35ed..c43a649b49 100644
--- a/services/context/context_response.go
+++ b/services/context/context_response.go
@@ -52,7 +52,7 @@ func (ctx *Context) RedirectToCurrentSite(location ...string) {
continue
}
- if !httplib.IsCurrentGiteaSiteURL(loc) {
+ if !httplib.IsCurrentGiteaSiteURL(ctx, loc) {
continue
}