From 513375c429435ba60a667b219bdfb00e5b760b38 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Sun, 29 Oct 2017 19:04:25 -0700 Subject: [PATCH] Make URL scheme unambiguous (#2408) * Make URL scheme unambiguous Redirect old routes to new routes * Fix redirects to new URL scheme, and update template * Fix branches/_new endpoints, and update integration test --- integrations/editor_test.go | 4 +- integrations/links_test.go | 18 ++++- integrations/repo_branch_test.go | 104 ++++++++++++------------ integrations/repo_commits_test.go | 6 +- models/webhook_slack.go | 21 +++-- modules/context/repo.go | 120 +++++++++++++++++++++++----- routers/api/v1/api.go | 2 +- routers/repo/branch.go | 10 +-- routers/repo/commit.go | 2 +- routers/repo/editor.go | 18 ++--- routers/repo/repo.go | 15 ++++ routers/repo/view.go | 4 +- routers/routes/routes.go | 67 ++++++++++------ templates/repo/branch_dropdown.tmpl | 4 +- templates/repo/commits_table.tmpl | 8 +- templates/repo/editor/edit.tmpl | 2 +- templates/repo/home.tmpl | 2 +- templates/repo/issue/list.tmpl | 2 +- templates/repo/release/list.tmpl | 10 +-- templates/repo/view_file.tmpl | 4 +- 20 files changed, 279 insertions(+), 144 deletions(-) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index 453b38491..712cc91fb 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -111,7 +111,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa resp = session.MakeRequest(t, req, http.StatusFound) // Verify the change - req = NewRequest(t, "GET", path.Join(user, repo, "raw", branch, filePath)) + req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", branch, filePath)) resp = session.MakeRequest(t, req, http.StatusOK) assert.EqualValues(t, newContent, string(resp.Body)) @@ -142,7 +142,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra resp = session.MakeRequest(t, req, http.StatusFound) // Verify the change - req = NewRequest(t, "GET", path.Join(user, repo, "raw", targetBranch, filePath)) + req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", targetBranch, filePath)) resp = session.MakeRequest(t, req, http.StatusOK) assert.EqualValues(t, newContent, string(resp.Body)) diff --git a/integrations/links_test.go b/integrations/links_test.go index 27e405469..7eaf1c51e 100644 --- a/integrations/links_test.go +++ b/integrations/links_test.go @@ -10,6 +10,8 @@ import ( "testing" api "code.gitea.io/sdk/gitea" + + "github.com/stretchr/testify/assert" ) func TestLinksNoLogin(t *testing.T) { @@ -38,6 +40,20 @@ func TestLinksNoLogin(t *testing.T) { } } +func TestRedirectsNoLogin(t *testing.T) { + prepareTestEnv(t) + + var redirects = map[string]string{ + "/user2/repo1/commits/master": "/user2/repo1/commits/branch/master", + "/user2/repo1/src/master": "/user2/repo1/src/branch/master", + } + for link, redirectLink := range redirects { + req := NewRequest(t, "GET", link) + resp := MakeRequest(t, req, http.StatusFound) + assert.EqualValues(t, redirectLink, RedirectURL(t, resp)) + } +} + func testLinksAsUser(userName string, t *testing.T) { var links = []string{ "/explore/repos", @@ -99,7 +115,7 @@ func testLinksAsUser(userName string, t *testing.T) { "", "/issues", "/pulls", - "/commits/master", + "/commits/branch/master", "/graph", "/settings", "/settings/collaboration", diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go index 32e24e83f..df7f97bd2 100644 --- a/integrations/repo_branch_test.go +++ b/integrations/repo_branch_test.go @@ -14,14 +14,14 @@ import ( "github.com/stretchr/testify/assert" ) -func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefName, newBranchName string, expectedStatus int) string { +func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefSubURL, newBranchName string, expectedStatus int) string { var csrf string if expectedStatus == http.StatusNotFound { - csrf = GetCSRF(t, session, path.Join(user, repo, "src/master")) + csrf = GetCSRF(t, session, path.Join(user, repo, "src/branch/master")) } else { - csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefName)) + csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefSubURL)) } - req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefName), map[string]string{ + req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{ "_csrf": csrf, "new_branch_name": newBranchName, }) @@ -34,72 +34,72 @@ func testCreateBranch(t *testing.T, session *TestSession, user, repo, oldRefName func TestCreateBranch(t *testing.T) { tests := []struct { - OldBranchOrCommit string - NewBranch string - CreateRelease string - FlashMessage string - ExpectedStatus int + OldRefSubURL string + NewBranch string + CreateRelease string + FlashMessage string + ExpectedStatus int }{ { - OldBranchOrCommit: "master", - NewBranch: "feature/test1", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test1"), + OldRefSubURL: "branch/master", + NewBranch: "feature/test1", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test1"), }, { - OldBranchOrCommit: "master", - NewBranch: "", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.require_error"), + OldRefSubURL: "branch/master", + NewBranch: "", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.require_error"), }, { - OldBranchOrCommit: "master", - NewBranch: "feature=test1", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"), + OldRefSubURL: "branch/master", + NewBranch: "feature=test1", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.git_ref_name_error"), }, { - OldBranchOrCommit: "master", - NewBranch: strings.Repeat("b", 101), - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.max_size_error", "100"), + OldRefSubURL: "branch/master", + NewBranch: strings.Repeat("b", 101), + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "form.NewBranchName") + i18n.Tr("en", "form.max_size_error", "100"), }, { - OldBranchOrCommit: "master", - NewBranch: "master", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.branch_already_exists", "master"), + OldRefSubURL: "branch/master", + NewBranch: "master", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.branch_already_exists", "master"), }, { - OldBranchOrCommit: "master", - NewBranch: "master/test", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.branch_name_conflict", "master/test", "master"), + OldRefSubURL: "branch/master", + NewBranch: "master/test", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.branch_name_conflict", "master/test", "master"), }, { - OldBranchOrCommit: "acd1d892867872cb47f3993468605b8aa59aa2e0", - NewBranch: "feature/test2", - ExpectedStatus: http.StatusNotFound, + OldRefSubURL: "commit/acd1d892867872cb47f3993468605b8aa59aa2e0", + NewBranch: "feature/test2", + ExpectedStatus: http.StatusNotFound, }, { - OldBranchOrCommit: "65f1bf27bc3bf70f64657658635e66094edbcb4d", - NewBranch: "feature/test3", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test3"), + OldRefSubURL: "commit/65f1bf27bc3bf70f64657658635e66094edbcb4d", + NewBranch: "feature/test3", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test3"), }, { - OldBranchOrCommit: "master", - NewBranch: "v1.0.0", - CreateRelease: "v1.0.0", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.tag_collision", "v1.0.0"), + OldRefSubURL: "branch/master", + NewBranch: "v1.0.0", + CreateRelease: "v1.0.0", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.tag_collision", "v1.0.0"), }, { - OldBranchOrCommit: "v1.0.0", - NewBranch: "feature/test4", - CreateRelease: "v1.0.0", - ExpectedStatus: http.StatusFound, - FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test4"), + OldRefSubURL: "tag/v1.0.0", + NewBranch: "feature/test4", + CreateRelease: "v1.0.0", + ExpectedStatus: http.StatusFound, + FlashMessage: i18n.Tr("en", "repo.branch.create_success", "feature/test4"), }, } for _, test := range tests { @@ -108,7 +108,7 @@ func TestCreateBranch(t *testing.T) { if test.CreateRelease != "" { createNewRelease(t, session, "/user2/repo1", test.CreateRelease, test.CreateRelease, false, false) } - redirectURL := testCreateBranch(t, session, "user2", "repo1", test.OldBranchOrCommit, test.NewBranch, test.ExpectedStatus) + redirectURL := testCreateBranch(t, session, "user2", "repo1", test.OldRefSubURL, test.NewBranch, test.ExpectedStatus) if test.ExpectedStatus == http.StatusFound { req := NewRequest(t, "GET", redirectURL) resp := session.MakeRequest(t, req, http.StatusOK) @@ -124,7 +124,7 @@ func TestCreateBranch(t *testing.T) { func TestCreateBranchInvalidCSRF(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") - req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/master", map[string]string{ + req := NewRequestWithValues(t, "POST", "user2/repo1/branches/_new/branch/master", map[string]string{ "_csrf": "fake_csrf", "new_branch_name": "test", }) diff --git a/integrations/repo_commits_test.go b/integrations/repo_commits_test.go index bf3539824..94d513370 100644 --- a/integrations/repo_commits_test.go +++ b/integrations/repo_commits_test.go @@ -20,7 +20,7 @@ func TestRepoCommits(t *testing.T) { session := loginUser(t, "user2") // Request repository commits page - req := NewRequest(t, "GET", "/user2/repo1/commits/master") + req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master") resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) @@ -35,7 +35,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { session := loginUser(t, "user2") // Request repository commits page - req := NewRequest(t, "GET", "/user2/repo1/commits/master") + req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master") resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) @@ -56,7 +56,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { resp = session.MakeRequest(t, req, http.StatusCreated) - req = NewRequest(t, "GET", "/user2/repo1/commits/master") + req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master") resp = session.MakeRequest(t, req, http.StatusOK) doc = NewHTMLParser(t, resp.Body) diff --git a/models/webhook_slack.go b/models/webhook_slack.go index ca43cfd42..dd25a8d7d 100644 --- a/models/webhook_slack.go +++ b/models/webhook_slack.go @@ -80,12 +80,22 @@ func SlackLinkFormatter(url string, text string) string { return fmt.Sprintf("<%s|%s>", url, SlackTextFormatter(text)) } -func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) { - // created tag/branch - refName := git.RefEndName(p.Ref) +// SlackLinkToRef slack-formatter link to a repo ref +func SlackLinkToRef(repoURL, ref string) string { + refName := git.RefEndName(ref) + switch { + case strings.HasPrefix(ref, git.BranchPrefix): + return SlackLinkFormatter(repoURL+"/src/branch/"+refName, refName) + case strings.HasPrefix(ref, git.TagPrefix): + return SlackLinkFormatter(repoURL+"/src/tag/"+refName, refName) + default: + return SlackLinkFormatter(repoURL+"/src/commit/"+refName, refName) + } +} +func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) { repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name) - refLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+refName, refName) + refLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref) text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName) return &SlackPayload{ @@ -99,7 +109,6 @@ func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayloa func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, error) { // n new commits var ( - branchName = git.RefEndName(p.Ref) commitDesc string commitString string ) @@ -116,7 +125,7 @@ func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, e } repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name) - branchLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+branchName, branchName) + branchLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref) text := fmt.Sprintf("[%s:%s] %s pushed by %s", repoLink, branchLink, commitString, p.Pusher.UserName) var attachmentText string diff --git a/modules/context/repo.go b/modules/context/repo.go index 704dc59f9..b2b58c4f2 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/git" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "github.com/Unknwon/com" @@ -117,6 +118,20 @@ func (r *Repository) GetCommitsCount() (int64, error) { }) } +// BranchNameSubURL sub-URL for the BranchName field +func (r *Repository) BranchNameSubURL() string { + switch { + case r.IsViewBranch: + return "branch/" + r.BranchName + case r.IsViewTag: + return "tag/" + r.BranchName + case r.IsViewCommit: + return "commit/" + r.BranchName + } + log.Error(4, "Unknown view type for repo: %v", r) + return "" +} + // GetEditorconfig returns the .editorconfig definition if found in the // HEAD of the default repo branch. func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) { @@ -444,8 +459,81 @@ func RepoAssignment() macaron.Handler { } } -// RepoRef handles repository reference name including those contain `/`. +// RepoRefType type of repo reference +type RepoRefType int + +const ( + // RepoRefLegacy unknown type, make educated guess and redirect. + // for backward compatibility with previous URL scheme + RepoRefLegacy RepoRefType = iota + // RepoRefBranch branch + RepoRefBranch + // RepoRefTag tag + RepoRefTag + // RepoRefCommit commit + RepoRefCommit +) + +// RepoRef handles repository reference names when the ref name is not +// explicitly given func RepoRef() macaron.Handler { + // since no ref name is explicitly specified, ok to just use branch + return RepoRefByType(RepoRefBranch) +} + +func getRefNameFromPath(ctx *Context, path string, isExist func(string) bool) string { + refName := "" + parts := strings.Split(path, "/") + for i, part := range parts { + refName = strings.TrimPrefix(refName+"/"+part, "/") + if isExist(refName) { + ctx.Repo.TreePath = strings.Join(parts[i+1:], "/") + return refName + } + } + return "" +} + +func getRefName(ctx *Context, pathType RepoRefType) string { + path := ctx.Params("*") + switch pathType { + case RepoRefLegacy: + if refName := getRefName(ctx, RepoRefBranch); len(refName) > 0 { + return refName + } + if refName := getRefName(ctx, RepoRefTag); len(refName) > 0 { + return refName + } + return getRefName(ctx, RepoRefCommit) + case RepoRefBranch: + return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsBranchExist) + case RepoRefTag: + return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist) + case RepoRefCommit: + parts := strings.Split(path, "/") + if len(parts) > 0 && len(parts[0]) == 40 { + ctx.Repo.TreePath = strings.Join(parts[1:], "/") + return parts[0] + } + default: + log.Error(4, "Unrecognized path type: %v", path) + } + return "" +} + +// URL to redirect to for deprecated URL scheme +func repoRefRedirect(ctx *Context) string { + urlPath := ctx.Req.URL.String() + idx := strings.LastIndex(urlPath, ctx.Params("*")) + if idx < 0 { + idx = len(urlPath) + } + return path.Join(urlPath[:idx], ctx.Repo.BranchNameSubURL()) +} + +// RepoRefByType handles repository reference name for a specific type +// of repository reference +func RepoRefByType(refType RepoRefType) macaron.Handler { return func(ctx *Context) { // Empty repository does not have reference information. if ctx.Repo.Repository.IsBare { @@ -470,6 +558,7 @@ func RepoRef() macaron.Handler { // Get default branch. if len(ctx.Params("*")) == 0 { refName = ctx.Repo.Repository.DefaultBranch + ctx.Repo.BranchName = refName if !ctx.Repo.GitRepo.IsBranchExist(refName) { brs, err := ctx.Repo.GitRepo.GetBranches() if err != nil { @@ -492,25 +581,8 @@ func RepoRef() macaron.Handler { ctx.Repo.IsViewBranch = true } else { - hasMatched := false - parts := strings.Split(ctx.Params("*"), "/") - for i, part := range parts { - refName = strings.TrimPrefix(refName+"/"+part, "/") - - if ctx.Repo.GitRepo.IsBranchExist(refName) || - ctx.Repo.GitRepo.IsTagExist(refName) { - if i < len(parts)-1 { - ctx.Repo.TreePath = strings.Join(parts[i+1:], "/") - } - hasMatched = true - break - } - } - if !hasMatched && len(parts[0]) == 40 { - refName = parts[0] - ctx.Repo.TreePath = strings.Join(parts[1:], "/") - } - + refName = getRefName(ctx, refType) + ctx.Repo.BranchName = refName if ctx.Repo.GitRepo.IsBranchExist(refName) { ctx.Repo.IsViewBranch = true @@ -542,10 +614,16 @@ func RepoRef() macaron.Handler { ctx.Handle(404, "RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName)) return } + + if refType == RepoRefLegacy { + // redirect from old URL scheme to new URL scheme + ctx.Redirect(repoRefRedirect(ctx)) + return + } } - ctx.Repo.BranchName = refName ctx.Data["BranchName"] = ctx.Repo.BranchName + ctx.Data["BranchNameSubURL"] = ctx.Repo.BranchNameSubURL() ctx.Data["CommitID"] = ctx.Repo.CommitID ctx.Data["TreePath"] = ctx.Repo.TreePath ctx.Data["IsViewBranch"] = ctx.Repo.IsViewBranch diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 103913544..79ae5799e 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -391,7 +391,7 @@ func RegisterRoutes(m *macaron.Macaron) { Post(reqToken(), bind(api.CreateForkOption{}), repo.CreateFork) m.Group("/branches", func() { m.Get("", repo.ListBranches) - m.Get("/*", context.RepoRef(), repo.GetBranch) + m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch) }) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). diff --git a/routers/repo/branch.go b/routers/repo/branch.go index f6eca3935..c56e8e86b 100644 --- a/routers/repo/branch.go +++ b/routers/repo/branch.go @@ -202,7 +202,7 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) { if ctx.HasError() { ctx.Flash.Error(ctx.GetErrMsg()) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) return } @@ -216,19 +216,19 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) { if models.IsErrTagAlreadyExists(err) { e := err.(models.ErrTagAlreadyExists) ctx.Flash.Error(ctx.Tr("repo.branch.tag_collision", e.TagName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) return } if models.IsErrBranchAlreadyExists(err) { e := err.(models.ErrBranchAlreadyExists) ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", e.BranchName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) return } if models.IsErrBranchNameConflict(err) { e := err.(models.ErrBranchNameConflict) ctx.Flash.Error(ctx.Tr("repo.branch.branch_name_conflict", form.NewBranchName, e.BranchName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()) return } @@ -237,5 +237,5 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) { } ctx.Flash.Success(ctx.Tr("repo.branch.create_success", form.NewBranchName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + form.NewBranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + form.NewBranchName) } diff --git a/routers/repo/commit.go b/routers/repo/commit.go index 637a50543..afd6b113c 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -120,7 +120,7 @@ func SearchCommits(ctx *context.Context) { keyword := strings.Trim(ctx.Query("q"), " ") if len(keyword) == 0 { - ctx.Redirect(ctx.Repo.RepoLink + "/commits/" + ctx.Repo.BranchName) + ctx.Redirect(ctx.Repo.RepoLink + "/commits/" + ctx.Repo.BranchNameSubURL()) return } all := ctx.QueryBool("all") diff --git a/routers/repo/editor.go b/routers/repo/editor.go index 42ede9a28..a6cc92236 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -113,7 +113,7 @@ func editFile(ctx *context.Context, isNewFile bool) { ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" if canCommit { @@ -164,7 +164,7 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo ctx.Data["TreePath"] = form.TreePath ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + branchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/branch/" + branchName ctx.Data["FileContent"] = form.Content ctx.Data["commit_summary"] = form.CommitSummary ctx.Data["commit_message"] = form.CommitMessage @@ -304,7 +304,7 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo return } - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + branchName + "/" + strings.NewReplacer("%", "%25", "#", "%23", " ", "%20", "?", "%3F").Replace(form.TreePath)) + ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + branchName + "/" + strings.NewReplacer("%", "%25", "#", "%23", " ", "%20", "?", "%3F").Replace(form.TreePath)) } // EditFilePost response for editing file @@ -348,7 +348,7 @@ func DiffPreviewPost(ctx *context.Context, form auth.EditPreviewDiffForm) { // DeleteFile render delete file page func DeleteFile(ctx *context.Context) { ctx.Data["PageIsDelete"] = true - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() ctx.Data["TreePath"] = ctx.Repo.TreePath canCommit := renderCommitRights(ctx) @@ -367,7 +367,7 @@ func DeleteFile(ctx *context.Context) { // DeleteFilePost response for deleting file func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) { ctx.Data["PageIsDelete"] = true - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() ctx.Data["TreePath"] = ctx.Repo.TreePath canCommit := renderCommitRights(ctx) @@ -422,7 +422,7 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) { } ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + branchName) + ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + branchName) } func renderUploadSettings(ctx *context.Context) { @@ -446,7 +446,7 @@ func UploadFile(ctx *context.Context) { ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() ctx.Data["commit_summary"] = "" ctx.Data["commit_message"] = "" if canCommit { @@ -482,7 +482,7 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { ctx.Data["TreePath"] = form.TreePath ctx.Data["TreeNames"] = treeNames ctx.Data["TreePaths"] = treePaths - ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/" + branchName + ctx.Data["BranchLink"] = ctx.Repo.RepoLink + "/src/branch/" + branchName ctx.Data["commit_summary"] = form.CommitSummary ctx.Data["commit_message"] = form.CommitMessage ctx.Data["commit_choice"] = form.CommitChoice @@ -551,7 +551,7 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { return } - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + branchName + "/" + form.TreePath) + ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + branchName + "/" + form.TreePath) } // UploadFileToServer upload file to server file dir not git diff --git a/routers/repo/repo.go b/routers/repo/repo.go index ebbce1e19..dbe78f6d1 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -34,6 +34,21 @@ func MustBeNotBare(ctx *context.Context) { } } +// MustBeEditable check that repo can be edited +func MustBeEditable(ctx *context.Context) { + if !ctx.Repo.Repository.CanEnableEditor() || ctx.Repo.IsViewCommit { + ctx.Handle(404, "", nil) + return + } +} + +// MustBeAbleToUpload check that repo can be uploaded to +func MustBeAbleToUpload(ctx *context.Context) { + if !setting.Repository.Upload.Enabled { + ctx.Handle(404, "", nil) + } +} + func checkContextUser(ctx *context.Context, uid int64) *models.User { orgs, err := models.GetOwnedOrgsByUserIDDesc(ctx.User.ID, "updated_unix") if err != nil { diff --git a/routers/repo/view.go b/routers/repo/view.go index 38260d2be..d43b4d7f7 100644 --- a/routers/repo/view.go +++ b/routers/repo/view.go @@ -297,9 +297,9 @@ func renderCode(ctx *context.Context) { ctx.Data["Title"] = title ctx.Data["RequireHighlightJS"] = true - branchLink := ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName + branchLink := ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() treeLink := branchLink - rawLink := ctx.Repo.RepoLink + "/raw/" + ctx.Repo.BranchName + rawLink := ctx.Repo.RepoLink + "/raw/" + ctx.Repo.BranchNameSubURL() if len(ctx.Repo.TreePath) > 0 { treeLink += "/" + ctx.Repo.TreePath diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 94dfe0ab3..f1c9f1848 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -522,34 +522,30 @@ func RegisterRoutes(m *macaron.Macaron) { Post(bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost) m.Group("", func() { - m.Combo("/_edit/*").Get(repo.EditFile). - Post(bindIgnErr(auth.EditRepoFileForm{}), repo.EditFilePost) - m.Combo("/_new/*").Get(repo.NewFile). - Post(bindIgnErr(auth.EditRepoFileForm{}), repo.NewFilePost) - m.Post("/_preview/*", bindIgnErr(auth.EditPreviewDiffForm{}), repo.DiffPreviewPost) - m.Combo("/_delete/*").Get(repo.DeleteFile). - Post(bindIgnErr(auth.DeleteRepoFileForm{}), repo.DeleteFilePost) - m.Group("", func() { - m.Combo("/_upload/*").Get(repo.UploadFile). + m.Combo("/_edit/*").Get(repo.EditFile). + Post(bindIgnErr(auth.EditRepoFileForm{}), repo.EditFilePost) + m.Combo("/_new/*").Get(repo.NewFile). + Post(bindIgnErr(auth.EditRepoFileForm{}), repo.NewFilePost) + m.Post("/_preview/*", bindIgnErr(auth.EditPreviewDiffForm{}), repo.DiffPreviewPost) + m.Combo("/_delete/*").Get(repo.DeleteFile). + Post(bindIgnErr(auth.DeleteRepoFileForm{}), repo.DeleteFilePost) + m.Combo("/_upload/*", repo.MustBeAbleToUpload). + Get(repo.UploadFile). Post(bindIgnErr(auth.UploadRepoFileForm{}), repo.UploadFilePost) + }, context.RepoRefByType(context.RepoRefBranch), repo.MustBeEditable) + m.Group("", func() { m.Post("/upload-file", repo.UploadFileToServer) m.Post("/upload-remove", bindIgnErr(auth.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer) - }, func(ctx *context.Context) { - if !setting.Repository.Upload.Enabled { - ctx.Handle(404, "", nil) - return - } - }) - }, repo.MustBeNotBare, reqRepoWriter, context.RepoRef(), func(ctx *context.Context) { - if !ctx.Repo.Repository.CanEnableEditor() || ctx.Repo.IsViewCommit { - ctx.Handle(404, "", nil) - return - } - }) + }, context.RepoRef(), repo.MustBeEditable, repo.MustBeAbleToUpload) + }, repo.MustBeNotBare, reqRepoWriter) m.Group("/branches", func() { - m.Post("/_new/*", context.RepoRef(), bindIgnErr(auth.NewBranchForm{}), repo.CreateBranch) + m.Group("/_new/", func() { + m.Post("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.CreateBranch) + m.Post("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.CreateBranch) + m.Post("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.CreateBranch) + }, bindIgnErr(auth.NewBranchForm{})) m.Post("/delete", repo.DeleteBranchPost) m.Post("/restore", repo.RestoreBranchPost) }, reqRepoWriter, repo.MustBeNotBare, context.CheckUnit(models.UnitTypeCode)) @@ -629,15 +625,36 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/cleanup", context.RepoRef(), repo.CleanUpPullRequest) }, repo.MustAllowPulls) + m.Group("/raw", func() { + m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload) + m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload) + m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload) + // "/*" route is deprecated, and kept for backward compatibility + m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload) + }, repo.MustBeNotBare, context.CheckUnit(models.UnitTypeCode)) + + m.Group("/commits", func() { + m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RefCommits) + m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefCommits) + m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefCommits) + // "/*" route is deprecated, and kept for backward compatibility + m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.RefCommits) + }, repo.MustBeNotBare, context.CheckUnit(models.UnitTypeCode)) + m.Group("", func() { - m.Get("/raw/*", repo.SingleDownload) - m.Get("/commits/*", repo.RefCommits) m.Get("/graph", repo.Graph) m.Get("/commit/:sha([a-f0-9]{7,40})$", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.Diff) }, repo.MustBeNotBare, context.RepoRef(), context.CheckUnit(models.UnitTypeCode)) + m.Group("/src", func() { + m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home) + m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home) + m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home) + // "/*" route is deprecated, and kept for backward compatibility + m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.Home) + }, repo.SetEditorconfigIfExists) + m.Group("", func() { - m.Get("/src/*", repo.SetEditorconfigIfExists, repo.Home) m.Get("/forks", repo.Forks) }, context.RepoRef(), context.CheckUnit(models.UnitTypeCode)) m.Get("/commit/:sha([a-f0-9]{7,40})\\.:ext(patch|diff)", diff --git a/templates/repo/branch_dropdown.tmpl b/templates/repo/branch_dropdown.tmpl index 6b9343ac0..881747220 100644 --- a/templates/repo/branch_dropdown.tmpl +++ b/templates/repo/branch_dropdown.tmpl @@ -10,10 +10,10 @@
diff --git a/templates/repo/commits_table.tmpl b/templates/repo/commits_table.tmpl index b5bd8f33d..2ca861c62 100644 --- a/templates/repo/commits_table.tmpl +++ b/templates/repo/commits_table.tmpl @@ -2,7 +2,7 @@ {{.CommitCount}} {{.i18n.Tr "repo.commits.commits"}} {{if .Branch}}({{.Branch}}){{end}} {{if .PageIsCommits}}
-
+
@@ -75,17 +75,17 @@ {{if gt .TotalPages 1}}
diff --git a/templates/repo/editor/edit.tmpl b/templates/repo/editor/edit.tmpl index 64b09c374..77e40c6de 100644 --- a/templates/repo/editor/edit.tmpl +++ b/templates/repo/editor/edit.tmpl @@ -30,7 +30,7 @@
{{if .IsNewFile}}{{.i18n.Tr "repo.editor.new_file"}}{{else}}{{.i18n.Tr "repo.editor.edit_file"}}{{end}} {{if not .IsNewFile}} - {{.i18n.Tr "repo.release.preview"}} + {{.i18n.Tr "repo.release.preview"}} {{.i18n.Tr "repo.editor.preview_changes"}} {{end}}
diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index f0b4a4562..c7c88aafc 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -35,7 +35,7 @@ {{template "repo/branch_dropdown" .}} {{ $n := len .TreeNames}} {{ $l := Subtract $n 1}} -
{{EllipsisString .Repository.Name 30}}{{range $i, $v := .TreeNames}}/{{if eq $i $l}}{{EllipsisString $v 30}}{{else}}{{ $p := index $.Paths $i}}{{EllipsisString $v 30}}{{end}}{{end}}
+
{{EllipsisString .Repository.Name 30}}{{range $i, $v := .TreeNames}}/{{if eq $i $l}}{{EllipsisString $v 30}}{{else}}{{ $p := index $.Paths $i}}{{EllipsisString $v 30}}{{end}}{{end}}
{{if .Repository.CanEnableEditor}}
diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 711d70f2a..2b756cd88 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -172,7 +172,7 @@ {{.Title}} {{if .Ref}} - {{.Ref}} + {{.Ref}} {{end}} {{range .Labels}} {{.Name | Sanitize}} diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index 9106bba9b..2fed9ae58 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -28,26 +28,26 @@ {{$.i18n.Tr "repo.release.stable"}} {{end}} - {{.TagName}} + {{.TagName}} - {{ShortSha .Sha1}} + {{ShortSha .Sha1}} {{end}}
{{if .IsTag}}

- {{.TagName}} + {{.TagName}}

- {{ShortSha .Sha1}} + {{ShortSha .Sha1}} ZIP TAR.GZ
{{else}}

- {{.Title}} + {{.Title}} {{if $.IsRepositoryWriter}}({{$.i18n.Tr "repo.release.edit"}}){{end}}

diff --git a/templates/repo/view_file.tmpl b/templates/repo/view_file.tmpl index 52bd6dfb4..e8319881f 100644 --- a/templates/repo/view_file.tmpl +++ b/templates/repo/view_file.tmpl @@ -15,9 +15,9 @@

{{if not .IsViewCommit}} - {{.i18n.Tr "repo.file_permalink"}} + {{.i18n.Tr "repo.file_permalink"}} {{end}} - {{.i18n.Tr "repo.file_history"}} + {{.i18n.Tr "repo.file_history"}} {{.i18n.Tr "repo.file_raw"}}
{{if .Repository.CanEnableEditor}}