lianguan/gitea
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor external URL detection (#29973)

Browse Source 
Follow #29960, `IsExternalURL` is not needed anymore. 
Add some tests for `RedirectToCurrentSite`
This commit is contained in:
wxiaoguang 2024-03-22 04:32:40 +08:00 committed by GitHub
parent bfa160fc98
commit ca4107dc96
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 40 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
modules/httplib/url.go
View File

@ -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) {

5
modules/httplib/url_test.go
View File

@ -53,6 +53,8 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) {
assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s)
}
bad := []string{
".",
"foo",
"/",
"//",
"\\\\",
@ -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
routers/utils/utils.go
View File

@ -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
routers/utils/utils_test.go
View File

@ -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

4
routers/web/auth/auth.go
View File

@ -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"
@ -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"
@ -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)

3
routers/web/auth/password.go
View File

@ -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"
@ -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

27
services/context/context_test.go
View File

@ -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"
)
@ -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)
})
}
}