Skip to content

Commit

Permalink
Refactor AppURL usage (#30885)
Browse files Browse the repository at this point in the history
Fix #30883
Fix #29591

---------

Co-authored-by: KN4CK3R <[email protected]>
  • Loading branch information
wxiaoguang and KN4CK3R authored May 7, 2024
1 parent ebf0c96 commit 67c1a07
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 39 deletions.
12 changes: 3 additions & 9 deletions models/repo/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
10 changes: 3 additions & 7 deletions models/user/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
60 changes: 58 additions & 2 deletions modules/httplib/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
package httplib

import (
"context"
"net/http"
"net/url"
"strings"

"code.gitea.io/gitea/modules/setting"
"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
Expand All @@ -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
Expand All @@ -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)))
}
59 changes: 53 additions & 6 deletions modules/httplib/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package httplib

import (
"context"
"net/http"
"testing"

"code.gitea.io/gitea/modules/setting"
Expand Down Expand Up @@ -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",
Expand All @@ -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{
".",
Expand All @@ -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"))
}
2 changes: 1 addition & 1 deletion modules/markup/html_codepreview.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions routers/api/actions/artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
9 changes: 5 additions & 4 deletions routers/api/actions/artifactsv4.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion routers/api/packages/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 3 additions & 0 deletions routers/common/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
})
})
Expand Down
2 changes: 1 addition & 1 deletion routers/common/redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion services/context/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 67c1a07

Please sign in to comment.