Skip to content

Refactor AppURL usage #30885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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