From cc9449c72e7cedf8cb284d1e01714d615546be2c Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 16 Aug 2024 20:40:51 +0800 Subject: [PATCH 1/4] Fix raw wiki links (#31825) Fix #31395 This regression is introduced by #30273. To find out how GitHub handles this case, I did [some tests](https://github.com/go-gitea/gitea/issues/31395#issuecomment-2278929115). I use redirect in this PR instead of checking if the corresponding `.md` file exists when rendering the link because GitHub also uses redirect. With this PR, there is no need to resolve the raw wiki link when rendering a wiki page. If a wiki link points to a raw file, access will be redirected to the raw link. --- modules/markup/html_link.go | 12 ++--- modules/markup/html_test.go | 2 +- modules/markup/markdown/markdown_test.go | 24 ++++----- routers/web/repo/wiki.go | 64 ++++++++++++++++++++---- routers/web/repo/wiki_test.go | 14 ++++++ 5 files changed, 84 insertions(+), 32 deletions(-) diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index a41b87e9fa83d..b08613534852d 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -4,8 +4,6 @@ package markup import ( - "path" - "code.gitea.io/gitea/modules/util" ) @@ -14,13 +12,9 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu if !isAnchorFragment && !IsFullURLString(link) { linkBase := ctx.Links.Base if ctx.IsWiki { - if ext := path.Ext(link); ext == "" || ext == ".-" { - linkBase = ctx.Links.WikiLink() // the link is for a wiki page - } else if DetectMarkupTypeByFileName(link) != "" { - linkBase = ctx.Links.WikiLink() // the link is renderable as a wiki page - } else { - linkBase = ctx.Links.WikiRawLink() // otherwise, use a raw link instead to view&download medias - } + // no need to check if the link should be resolved as a wiki link or a wiki raw link + // just use wiki link here and it will be redirected to a wiki raw link if necessary + linkBase = ctx.Links.WikiLink() } else if ctx.Links.BranchPath != "" || ctx.Links.TreePath != "" { // if there is no BranchPath, then the link will be something like "/owner/repo/src/{the-file-path}" // and then this link will be handled by the "legacy-ref" code and be redirected to the default branch like "/owner/repo/src/branch/main/{the-file-path}" diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 8911bf3f2ef40..85ed78a922a84 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -435,7 +435,7 @@ func TestRender_ShortLinks(t *testing.T) { renderableFileURL := util.URLJoin(tree, "markdown_file.md") renderableFileURLWiki := util.URLJoin(markup.TestRepoURL, "wiki", "markdown_file.md") unrenderableFileURL := util.URLJoin(tree, "file.zip") - unrenderableFileURLWiki := util.URLJoin(markup.TestRepoURL, "wiki", "raw", "file.zip") + unrenderableFileURLWiki := util.URLJoin(markup.TestRepoURL, "wiki", "file.zip") favicon := "http://google.com/favicon.ico" test( diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index d71ca363864ff..14344d91550d6 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -655,9 +655,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
@@ -713,9 +713,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
@@ -771,9 +771,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
@@ -831,9 +831,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
@@ -891,9 +891,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
@@ -953,9 +953,9 @@ space

Expected: `

space @mention-user
/just/a/path.bin
https://example.com/file.bin
-local link
+local link
remote link
-local link
+local link
remote link
local image
local image
diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 13b6a7b8e3b92..d2056353d886d 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -138,18 +138,41 @@ func wikiContentsByEntry(ctx *context.Context, entry *git.TreeEntry) []byte { return content } -// wikiContentsByName returns the contents of a wiki page, along with a boolean -// indicating whether the page exists. Writes to ctx if an error occurs. -func wikiContentsByName(ctx *context.Context, commit *git.Commit, wikiName wiki_service.WebPath) ([]byte, *git.TreeEntry, string, bool) { +// wikiEntryByName returns the entry of a wiki page, along with a boolean +// indicating whether the entry exists. Writes to ctx if an error occurs. +// The last return value indicates whether the file should be returned as a raw file +func wikiEntryByName(ctx *context.Context, commit *git.Commit, wikiName wiki_service.WebPath) (*git.TreeEntry, string, bool, bool) { + isRaw := false gitFilename := wiki_service.WebPathToGitPath(wikiName) entry, err := findEntryForFile(commit, gitFilename) if err != nil && !git.IsErrNotExist(err) { ctx.ServerError("findEntryForFile", err) - return nil, nil, "", false - } else if entry == nil { + return nil, "", false, false + } + if entry == nil { + // check if the file without ".md" suffix exists + gitFilename := strings.TrimSuffix(gitFilename, ".md") + entry, err = findEntryForFile(commit, gitFilename) + if err != nil && !git.IsErrNotExist(err) { + ctx.ServerError("findEntryForFile", err) + return nil, "", false, false + } + isRaw = true + } + if entry == nil { + return nil, "", true, false + } + return entry, gitFilename, false, isRaw +} + +// wikiContentsByName returns the contents of a wiki page, along with a boolean +// indicating whether the page exists. Writes to ctx if an error occurs. +func wikiContentsByName(ctx *context.Context, commit *git.Commit, wikiName wiki_service.WebPath) ([]byte, *git.TreeEntry, string, bool) { + entry, gitFilename, noEntry, _ := wikiEntryByName(ctx, commit, wikiName) + if entry == nil { return nil, nil, "", true } - return wikiContentsByEntry(ctx, entry), entry, gitFilename, false + return wikiContentsByEntry(ctx, entry), entry, gitFilename, noEntry } func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { @@ -215,11 +238,14 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { isSideBar := pageName == "_Sidebar" isFooter := pageName == "_Footer" - // lookup filename in wiki - get filecontent, gitTree entry , real filename - data, entry, pageFilename, noEntry := wikiContentsByName(ctx, commit, pageName) + // lookup filename in wiki - get gitTree entry , real filename + entry, pageFilename, noEntry, isRaw := wikiEntryByName(ctx, commit, pageName) if noEntry { ctx.Redirect(ctx.Repo.RepoLink + "/wiki/?action=_pages") } + if isRaw { + ctx.Redirect(util.URLJoin(ctx.Repo.RepoLink, "wiki/raw", string(pageName))) + } if entry == nil || ctx.Written() { if wikiRepo != nil { wikiRepo.Close() @@ -227,6 +253,15 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { return nil, nil } + // get filecontent + data := wikiContentsByEntry(ctx, entry) + if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } + return nil, nil + } + var sidebarContent []byte if !isSideBar { sidebarContent, _, _, _ = wikiContentsByName(ctx, commit, "_Sidebar") @@ -442,15 +477,24 @@ func renderEditPage(ctx *context.Context) { ctx.Data["Title"] = displayName ctx.Data["title"] = displayName - // lookup filename in wiki - get filecontent, gitTree entry , real filename - data, entry, _, noEntry := wikiContentsByName(ctx, commit, pageName) + // lookup filename in wiki - gitTree entry , real filename + entry, _, noEntry, isRaw := wikiEntryByName(ctx, commit, pageName) if noEntry { ctx.Redirect(ctx.Repo.RepoLink + "/wiki/?action=_pages") } + if isRaw { + ctx.Error(http.StatusForbidden, "Editing of raw wiki files is not allowed") + } if entry == nil || ctx.Written() { return } + // get filecontent + data := wikiContentsByEntry(ctx, entry) + if ctx.Written() { + return + } + ctx.Data["content"] = string(data) ctx.Data["sidebarPresent"] = false ctx.Data["sidebarContent"] = "" diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index 4602dcfeb40e0..2dee56cfd02d5 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -87,6 +87,13 @@ func TestWiki(t *testing.T) { assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) assert.EqualValues(t, "Home", ctx.Data["Title"]) assertPagesMetas(t, []string{"Home", "Page With Image", "Page With Spaced Name", "Unescaped File"}, ctx.Data["Pages"]) + + ctx, _ = contexttest.MockContext(t, "user2/repo1/jpeg.jpg") + ctx.SetPathParam("*", "jpeg.jpg") + contexttest.LoadRepo(t, ctx, 1) + Wiki(ctx) + assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) + assert.Equal(t, "/user2/repo1/wiki/raw/jpeg.jpg", ctx.Resp.Header().Get("Location")) } func TestWikiPages(t *testing.T) { @@ -160,6 +167,13 @@ func TestEditWiki(t *testing.T) { assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) assert.EqualValues(t, "Home", ctx.Data["Title"]) assert.Equal(t, wikiContent(t, ctx.Repo.Repository, "Home"), ctx.Data["content"]) + + ctx, _ = contexttest.MockContext(t, "user2/repo1/wiki/jpeg.jpg?action=_edit") + ctx.SetPathParam("*", "jpeg.jpg") + contexttest.LoadUser(t, ctx, 2) + contexttest.LoadRepo(t, ctx, 1) + EditWiki(ctx) + assert.EqualValues(t, http.StatusForbidden, ctx.Resp.Status()) } func TestEditWikiPost(t *testing.T) { From 771fb453a1f6f23ff63571cb83d97daf0035ef1e Mon Sep 17 00:00:00 2001 From: Giteabot Date: Fri, 16 Aug 2024 20:41:45 +0800 Subject: [PATCH 2/4] Add missing repository type filter parameters to pager (#31832) (#31837) Backport #31832 by @yp05327 Fix #31807 ps: the newly added params's value will be changed. When the first time you selected the filter, the values of params will be `0` or `1` But in pager it will be `true` or `false`. So do we have `boolToInt` function? Co-authored-by: yp05327 <576951401@qq.com> --- routers/web/explore/repo.go | 15 +++++++++++++++ routers/web/org/home.go | 16 ++++++++++++++++ routers/web/user/notification.go | 15 +++++++++++++++ routers/web/user/profile.go | 15 +++++++++++++++ 4 files changed, 61 insertions(+) diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go index 764c226d40ba0..dcfcae5cc4288 100644 --- a/routers/web/explore/repo.go +++ b/routers/web/explore/repo.go @@ -172,6 +172,21 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { pager.AddParamString("topic", fmt.Sprint(topicOnly)) pager.AddParamString("language", language) pager.AddParamString(relevantReposOnlyParam, fmt.Sprint(opts.OnlyShowRelevant)) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.HTML(http.StatusOK, opts.TplName) diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 846b1de18ab13..1e04b72cbb486 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -4,6 +4,7 @@ package org import ( + "fmt" "net/http" "path" "strings" @@ -155,6 +156,21 @@ func Home(ctx *context.Context) { pager := context.NewPagination(int(count), setting.UI.User.RepoPagingNum, page, 5) pager.SetDefaultParams(ctx) pager.AddParamString("language", language) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.Data["ShowMemberAndTeamTab"] = ctx.Org.IsMember || len(members) > 0 diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index ae0132e6e2871..9427b0d74ad0f 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -439,6 +439,21 @@ func NotificationWatching(ctx *context.Context) { // redirect to last page if request page is more than total pages pager := context.NewPagination(total, setting.UI.User.RepoPagingNum, page, 5) pager.SetDefaultParams(ctx) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.Data["Status"] = 2 diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index f0749e10216ee..3f91233ee6ee6 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -333,6 +333,21 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb pager.AddParamString("date", fmt.Sprint(ctx.Data["Date"])) } } + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager } From 1cf8f69b3840b04270042c53e3959e1caa623777 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sat, 17 Aug 2024 01:50:12 +0800 Subject: [PATCH 3/4] Avoid returning without written ctx when posting PR (#31843) (#31848) Backport #31843 by @wolfogre Fix #31625. If `pull_service.NewPullRequest` return an error which misses each `if` check, `CompareAndPullRequestPost` will return immediately, since it doesn't write the HTTP response, a 200 response with empty body will be sent to clients. ```go if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) } else if git.IsErrPushRejected(err) { // ... ctx.JSONError(flashError) } else if errors.Is(err, user_model.ErrBlockedUser) { // ... ctx.JSONError(flashError) } else if errors.Is(err, issues_model.ErrMustCollaborator) { // ... ctx.JSONError(flashError) } return } ``` Not sure what kind of error can cause it to happen, so this PR just expose it. And we can fix it when users report that creating PRs failed with error responses. It's all my guess since I cannot reproduce the problem, but even if it's not related, the code here needs to be improved. Co-authored-by: Jason Song --- routers/web/repo/pull.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index e71b7f4be2a56..237742d6737cb 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1296,9 +1296,10 @@ func CompareAndPullRequestPost(ctx *context.Context) { // instead of 500. if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { - if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { + switch { + case repo_model.IsErrUserDoesNotHaveAccessToRepo(err): ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) - } else if git.IsErrPushRejected(err) { + case git.IsErrPushRejected(err): pushrejErr := err.(*git.ErrPushRejected) message := pushrejErr.Message if len(message) == 0 { @@ -1315,7 +1316,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } ctx.JSONError(flashError) - } else if errors.Is(err, user_model.ErrBlockedUser) { + case errors.Is(err, user_model.ErrBlockedUser): flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.blocked_user"), @@ -1325,7 +1326,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } ctx.JSONError(flashError) - } else if errors.Is(err, issues_model.ErrMustCollaborator) { + case errors.Is(err, issues_model.ErrMustCollaborator): flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ "Message": ctx.Tr("repo.pulls.push_rejected"), "Summary": ctx.Tr("repo.pulls.new.must_collaborator"), @@ -1335,6 +1336,11 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } ctx.JSONError(flashError) + default: + // It's an unexpected error. + // If it happens, we should add another case to handle it. + log.Error("Unexpected error of NewPullRequest: %T %s", err, err) + ctx.ServerError("CompareAndPullRequest", err) } return } From 4f61c0c027a5e0de7f1065ff84f1ba5abc04e7bf Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 16 Aug 2024 19:44:15 -0700 Subject: [PATCH 4/4] Fix bug --- routers/web/repo/wiki_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index 2dee56cfd02d5..86971f15f1166 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -89,7 +89,7 @@ func TestWiki(t *testing.T) { assertPagesMetas(t, []string{"Home", "Page With Image", "Page With Spaced Name", "Unescaped File"}, ctx.Data["Pages"]) ctx, _ = contexttest.MockContext(t, "user2/repo1/jpeg.jpg") - ctx.SetPathParam("*", "jpeg.jpg") + ctx.SetParams("*", "jpeg.jpg") contexttest.LoadRepo(t, ctx, 1) Wiki(ctx) assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) @@ -169,7 +169,7 @@ func TestEditWiki(t *testing.T) { assert.Equal(t, wikiContent(t, ctx.Repo.Repository, "Home"), ctx.Data["content"]) ctx, _ = contexttest.MockContext(t, "user2/repo1/wiki/jpeg.jpg?action=_edit") - ctx.SetPathParam("*", "jpeg.jpg") + ctx.SetParams("*", "jpeg.jpg") contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, 1) EditWiki(ctx)