diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 226c666c64..3c923c2c5e 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -785,7 +785,7 @@ func PrepareBranchList(ctx *context.Context) { func SyncFork(ctx *context.Context) { redirectURL := fmt.Sprintf("%s/src/branch/%s", ctx.Repo.RepoLink, util.PathEscapeSegments(ctx.Repo.BranchName)) - branch := ctx.Params("branch") + branch := ctx.FormString("branch") syncForkInfo, err := repo_service.GetSyncForkInfo(ctx, ctx.Repo.Repository, branch) if err != nil { diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index dcbf665f0f..219de722d2 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -1165,7 +1165,6 @@ PostRecentBranchCheck: if syncForkInfo.Allowed { ctx.Data["CanSyncFork"] = true ctx.Data["ForkCommitsBehind"] = syncForkInfo.CommitsBehind - ctx.Data["SyncForkLink"] = fmt.Sprintf("%s/sync_fork/%s", ctx.Repo.RepoLink, util.PathEscapeSegments(ctx.Repo.BranchName)) ctx.Data["BaseBranchLink"] = fmt.Sprintf("%s/src/branch/%s", ctx.Repo.Repository.BaseRepo.HTMLURL(), util.PathEscapeSegments(ctx.Repo.BranchName)) } } diff --git a/routers/web/web.go b/routers/web/web.go index d7b34d3a86..3372a5bca2 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1593,7 +1593,7 @@ func registerRoutes(m *web.Route) { } m.Get("/commit/{sha:([a-f0-9]{4,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, reqRepoCodeReader, repo.RawDiff) - m.Get("/sync_fork/{branch}", context.RepoMustNotBeArchived(), repo.MustBeNotEmpty, reqRepoCodeWriter, repo.SyncFork) + m.Post("/sync_fork", context.RepoMustNotBeArchived(), repo.MustBeNotEmpty, reqRepoCodeWriter, repo.SyncFork) }, ignSignIn, context.RepoAssignment, context.UnitTypes()) m.Post("/{username}/{reponame}/lastcommit/*", ignSignInAndCsrf, context.RepoAssignment, context.UnitTypes(), context.RepoRefByType(context.RepoRefCommit), reqRepoCodeReader, repo.LastCommit) diff --git a/templates/repo/code/fork_sync_message.tmpl b/templates/repo/code/fork_sync_message.tmpl index 7edbaa8d4b..b09d7b138e 100644 --- a/templates/repo/code/fork_sync_message.tmpl +++ b/templates/repo/code/fork_sync_message.tmpl @@ -1,10 +1,15 @@ {{if .CanSyncFork}} -
+
- {{ctx.Locale.TrN .ForkCommitsBehind "repo.sync_fork.branch_behind_one" "repo.sync_fork.branch_behind_few" .ForkCommitsBehind (printf "%s:%s" .BaseBranchLink .Repository.BaseRepo.FullName .BranchName | SafeHTML)}} + {{$baseBranchHTML := HTMLFormat "%s:%s" .BaseBranchLink .Repository.BaseRepo.FullName .BranchName}} + {{ctx.Locale.TrN .ForkCommitsBehind "repo.sync_fork.branch_behind_one" "repo.sync_fork.branch_behind_few" .ForkCommitsBehind $baseBranchHTML}}
- - {{ctx.Locale.Tr "repo.sync_fork.button"}} - +
+ {{.CsrfTokenHtml}} + + +
{{end}} diff --git a/tests/integration/repo_sync_fork_test.go b/tests/integration/repo_sync_fork_test.go index 956494cfc6..fb08a69d8b 100644 --- a/tests/integration/repo_sync_fork_test.go +++ b/tests/integration/repo_sync_fork_test.go @@ -19,7 +19,7 @@ import ( "github.com/stretchr/testify/require" ) -func syncForkTest(t *testing.T, forkName, urlPart string, webSync bool) { +func syncForkTest(t *testing.T, forkName, branchName string, webSync bool) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -28,11 +28,11 @@ func syncForkTest(t *testing.T, forkName, urlPart string, webSync bool) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - /// Create a new fork - req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", baseUser.Name, baseRepo.LowerName), &api.CreateForkOption{Name: &forkName}).AddTokenAuth(token) + // Create a new fork + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/forks", baseRepo.FullName()), &api.CreateForkOption{Name: &forkName}).AddTokenAuth(token) MakeRequest(t, req, http.StatusAccepted) - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/%s", user.Name, forkName, urlPart).AddTokenAuth(token) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/sync_fork/%s", user.Name, forkName, branchName).AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) var syncForkInfo *api.SyncForkInfo @@ -43,10 +43,10 @@ func syncForkTest(t *testing.T, forkName, urlPart string, webSync bool) { assert.Equal(t, syncForkInfo.BaseCommit, syncForkInfo.ForkCommit) // Make a commit on the base branch - err := createOrReplaceFileInBranch(baseUser, baseRepo, "sync_fork.txt", "master", "Hello") + err := createOrReplaceFileInBranch(baseUser, baseRepo, "sync_fork.txt", branchName, "Hello") require.NoError(t, err) - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/%s", user.Name, forkName, urlPart).AddTokenAuth(token) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/sync_fork/%s", user.Name, forkName, branchName).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &syncForkInfo) @@ -57,13 +57,16 @@ func syncForkTest(t *testing.T, forkName, urlPart string, webSync bool) { // Sync the fork if webSync { - session.MakeRequest(t, NewRequestf(t, "GET", "/%s/%s/sync_fork/master", user.Name, forkName), http.StatusSeeOther) + session.MakeRequest(t, NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/sync_fork", user.Name, forkName), map[string]string{ + "_csrf": GetCSRF(t, session, fmt.Sprintf("/%s/%s", user.Name, forkName)), + "branch": branchName, + }), http.StatusSeeOther) } else { - req = NewRequestf(t, "POST", "/api/v1/repos/%s/%s/%s", user.Name, forkName, urlPart).AddTokenAuth(token) + req = NewRequestf(t, "POST", "/api/v1/repos/%s/%s/sync_fork/%s", user.Name, forkName, branchName).AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) } - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/%s", user.Name, forkName, urlPart).AddTokenAuth(token) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/sync_fork/%s", user.Name, forkName, branchName).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &syncForkInfo) @@ -75,43 +78,75 @@ func syncForkTest(t *testing.T, forkName, urlPart string, webSync bool) { func TestAPIRepoSyncForkDefault(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { - syncForkTest(t, "SyncForkDefault", "sync_fork", false) + syncForkTest(t, "SyncForkDefault", "master", false) }) } func TestAPIRepoSyncForkBranch(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { - syncForkTest(t, "SyncForkBranch", "sync_fork/master", false) + syncForkTest(t, "SyncForkBranch", "master", false) }) } func TestWebRepoSyncForkBranch(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { - syncForkTest(t, "SyncForkBranch", "sync_fork/master", true) + syncForkTest(t, "SyncForkBranch", "master", true) }) } func TestWebRepoSyncForkHomepage(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { - forkName := "SyncForkHomepage" - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) - baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - baseUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: baseRepo.OwnerID}) + baseOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: baseRepo.OwnerID}) + baseOwnerSession := loginUser(t, baseOwner.Name) - session := loginUser(t, user.Name) - token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + forkOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) + forkOwnerSession := loginUser(t, forkOwner.Name) + token := getTokenForLoggedInUser(t, forkOwnerSession, auth_model.AccessTokenScopeWriteRepository) - /// Create a new fork - req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", baseUser.Name, baseRepo.LowerName), &api.CreateForkOption{Name: &forkName}).AddTokenAuth(token) + forkName := "SyncForkHomepage" + forkLink := fmt.Sprintf("/%s/%s", forkOwner.Name, forkName) + branchName := "&" + branchHTMLEscaped := "<script>alert('0ko')</script>&amp;" + branchURLEscaped := "%3Cscript%3Ealert%28%270ko%27%29%3C/script%3E&amp%3B" + + // Rename branch "master" to test name escaping in the UI + baseOwnerSession.MakeRequest(t, NewRequestWithValues(t, "POST", + "/user2/repo1/settings/rename_branch", map[string]string{ + "_csrf": GetCSRF(t, baseOwnerSession, "/user2/repo1/branches"), + "from": "master", + "to": branchName, + }), http.StatusSeeOther) + + // Create a new fork + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/forks", baseRepo.FullName()), &api.CreateForkOption{Name: &forkName}).AddTokenAuth(token) MakeRequest(t, req, http.StatusAccepted) // Make a commit on the base branch - err := createOrReplaceFileInBranch(baseUser, baseRepo, "sync_fork.txt", "master", "Hello") + err := createOrReplaceFileInBranch(baseOwner, baseRepo, "sync_fork.txt", branchName, "Hello") require.NoError(t, err) - resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/%s", user.Name, forkName), http.StatusOK) + doc := NewHTMLParser(t, forkOwnerSession.MakeRequest(t, + NewRequest(t, "GET", forkLink), http.StatusOK).Body) - assert.Contains(t, resp.Body.String(), fmt.Sprintf("This branch is 1 commit behind user2/repo1:master", u.Port())) + // Verify correct URL escaping of branch name in the form + form := doc.Find("#sync_fork_msg form") + assert.Equal(t, 1, form.Length()) + updateLink, exists := form.Attr("action") + assert.True(t, exists) + + // Verify correct escaping of branch name in the message + raw, _ := doc.Find("#sync_fork_msg").Html() + assert.Contains(t, raw, fmt.Sprintf(`This branch is 1 commit behind user2/repo1:%s`, + u.Port(), branchURLEscaped, branchHTMLEscaped)) + + // Verify that the form link doesn't do anything for a GET request + forkOwnerSession.MakeRequest(t, NewRequest(t, "GET", updateLink), http.StatusMethodNotAllowed) + + // Verify that the form link does not error out + forkOwnerSession.MakeRequest(t, NewRequestWithValues(t, "POST", updateLink, map[string]string{ + "_csrf": GetCSRF(t, forkOwnerSession, forkLink), + "branch": branchName, + }), http.StatusSeeOther) }) }