Skip to content

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
  • Loading branch information
wxiaoguang committed Mar 21, 2024
1 parent cdb4d1a commit d4ad30b
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 62 deletions.
8 changes: 5 additions & 3 deletions modules/httplib/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ func IsCurrentGiteaSiteURL(s string) bool {
return false
}
if u.Path != "" {
u.Path = "/" + util.PathJoinRelX(u.Path)
if !strings.HasSuffix(u.Path, "/") {
u.Path += "/"
cleanedPath := util.PathJoinRelX(u.Path)
if cleanedPath == "" || cleanedPath == "." {
u.Path = "/"
} else {
u.Path += "/" + cleanedPath + "/"
}
}
if urlIsRelative(s, u) {
Expand Down
5 changes: 5 additions & 0 deletions modules/httplib/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s)
}
bad := []string{
".",
"foo",
"/",
"//",
"\\\\",
Expand All @@ -67,5 +69,8 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {

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"))
}
16 changes: 0 additions & 16 deletions routers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,10 @@ package utils

import (
"html"
"net/url"
"strings"

"code.gitea.io/gitea/modules/setting"
)

// SanitizeFlashErrorString will sanitize a flash error string
func SanitizeFlashErrorString(x string) string {
return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
}

// IsExternalURL checks if rawURL points to an external URL like http://example.com
func IsExternalURL(rawURL string) bool {
parsed, err := url.Parse(rawURL)
if err != nil {
return true
}
appURL, _ := url.Parse(setting.AppURL)
if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(appURL.Host, "www.", "", 1) {
return true
}
return false
}
39 changes: 0 additions & 39 deletions routers/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,8 @@ package utils

import (
"testing"

"code.gitea.io/gitea/modules/setting"

"github.com/stretchr/testify/assert"
)

func TestIsExternalURL(t *testing.T) {
setting.AppURL = "https://try.gitea.io/"
type test struct {
Expected bool
RawURL string
}
newTest := func(expected bool, rawURL string) test {
return test{Expected: expected, RawURL: rawURL}
}
for _, test := range []test{
newTest(false,
"https://try.gitea.io"),
newTest(true,
"https://example.com/"),
newTest(true,
"//example.com"),
newTest(true,
"http://example.com"),
newTest(false,
"a/"),
newTest(false,
"https://try.gitea.io/test?param=false"),
newTest(false,
"test?param=false"),
newTest(false,
"//try.gitea.io/test?param=false"),
newTest(false,
"/hey/hey/hey#3244"),
newTest(true,
"://missing protocol scheme"),
} {
assert.Equal(t, test.Expected, IsExternalURL(test.RawURL))
}
}

func TestSanitizeFlashErrorString(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 2 additions & 2 deletions routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/modules/auth/password"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/eventsource"
"code.gitea.io/gitea/modules/httplib"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/session"
Expand All @@ -25,7 +26,6 @@ import (
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/routers/utils"
auth_service "code.gitea.io/gitea/services/auth"
"code.gitea.io/gitea/services/auth/source/oauth2"
"code.gitea.io/gitea/services/context"
Expand Down 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"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) {
if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" && httplib.IsCurrentGiteaSiteURL(redirectTo) {
middleware.DeleteRedirectToCookie(ctx.Resp)
if obeyRedirect {
ctx.RedirectToCurrentSite(redirectTo)
Expand Down
3 changes: 1 addition & 2 deletions routers/web/auth/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/routers/utils"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/mailer"
Expand Down Expand Up @@ -312,7 +311,7 @@ func MustChangePasswordPost(ctx *context.Context) {

log.Trace("User updated password: %s", ctx.Doer.Name)

if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) {
if redirectTo := ctx.GetSiteCookie("redirect_to"); redirectTo != "" {
middleware.DeleteRedirectToCookie(ctx.Resp)
ctx.RedirectToCurrentSite(redirectTo)
return
Expand Down
27 changes: 27 additions & 0 deletions services/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ package context
import (
"net/http"
"net/http/httptest"
"net/url"
"testing"

"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
)
Expand All @@ -22,3 +24,28 @@ func TestRemoveSessionCookieHeader(t *testing.T) {
assert.Len(t, w.Header().Values("Set-Cookie"), 1)
assert.Contains(t, "other=bar", w.Header().Get("Set-Cookie"))
}

func TestRedirectToCurrentSite(t *testing.T) {
defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
cases := []struct {
location string
want string
}{
{"/", "/sub/"},
{"http://localhost:3000/sub?k=v", "http://localhost:3000/sub?k=v"},
{"http://other", "/sub/"},
}
for _, c := range cases {
t.Run(c.location, func(t *testing.T) {
req := &http.Request{URL: &url.URL{Path: "/"}}
resp := httptest.NewRecorder()
base, baseCleanUp := NewBaseContext(resp, req)
defer baseCleanUp()
ctx := NewWebContext(base, nil, nil)
ctx.RedirectToCurrentSite(c.location)
redirect := test.RedirectURL(resp)
assert.Equal(t, c.want, redirect)
})
}
}

0 comments on commit d4ad30b

Please sign in to comment.