From 632a274b8f32a02c68b5724d6b16f0c2cf4a6905 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 30 Apr 2024 11:21:41 +0200 Subject: [PATCH 1/3] Fix Issue watching / unwatching on the web ui When subscribing or unsubscribing to/from an issue on the web ui, the request was posted to a route handled by `repo.IssueWatch`. This function used `ctx.Req.PostForm.Get()`, erroneously. `request.PostForm` is *only* available if `request.ParseForm()` has been called before it. The function in question did not do that. Under some circumstances, something, somewhere did end up calling `ParseForm()`, but not in every scenario. Since we do not need to check for multiple values, the easiest fix here is to use `ctx.Req.PostFormValue`, which will call `ParseForm()` if necessary. Fixes #3516. Signed-off-by: Gergely Nagy --- release-notes/8.0.0/fix/3562.md | 1 + routers/web/repo/issue_watch.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 release-notes/8.0.0/fix/3562.md diff --git a/release-notes/8.0.0/fix/3562.md b/release-notes/8.0.0/fix/3562.md new file mode 100644 index 000000000..809905620 --- /dev/null +++ b/release-notes/8.0.0/fix/3562.md @@ -0,0 +1 @@ +Fixed a bug where subscribing to or unsubscribing from an issue in a repository with no code produced an internal server error. diff --git a/routers/web/repo/issue_watch.go b/routers/web/repo/issue_watch.go index c8d7187b8..5cff9f4dd 100644 --- a/routers/web/repo/issue_watch.go +++ b/routers/web/repo/issue_watch.go @@ -46,7 +46,7 @@ func IssueWatch(ctx *context.Context) { return } - watch, err := strconv.ParseBool(ctx.Req.PostForm.Get("watch")) + watch, err := strconv.ParseBool(ctx.Req.PostFormValue("watch")) if err != nil { ctx.ServerError("watch is not bool", err) return From 8cc5d5dc784b11b07b21daa6f46a29c263b94806 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 30 Apr 2024 01:35:40 +0200 Subject: [PATCH 2/3] tests: Support creating a declarative repo without AutoInit To be able to easily test cases where the repository does not have any code, where the git repo itself is completely uninitialized, lets support a case where the `AutoInit` property is false. For the sake of backwards compatibility, if the option is not set either way, it will default to `true`. Signed-off-by: Gergely Nagy --- tests/integration/integration_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index f5b87231f..f6daf0d14 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -692,6 +692,7 @@ type DeclarativeRepoOptions struct { DisabledUnits optional.Option[[]unit_model.Type] Files optional.Option[[]*files_service.ChangeRepoFile] WikiBranch optional.Option[string] + AutoInit optional.Option[bool] } func CreateDeclarativeRepoWithOptions(t *testing.T, owner *user_model.User, opts DeclarativeRepoOptions) (*repo_model.Repository, string, func()) { @@ -706,11 +707,18 @@ func CreateDeclarativeRepoWithOptions(t *testing.T, owner *user_model.User, opts repoName = gouuid.NewString() } + var autoInit bool + if opts.AutoInit.Has() { + autoInit = opts.AutoInit.Value() + } else { + autoInit = true + } + // Create the repository repo, err := repo_service.CreateRepository(db.DefaultContext, owner, owner, repo_service.CreateRepoOptions{ Name: repoName, Description: "Temporary Repo", - AutoInit: true, + AutoInit: autoInit, Gitignores: "", License: "WTFPL", Readme: "Default", @@ -742,6 +750,7 @@ func CreateDeclarativeRepoWithOptions(t *testing.T, owner *user_model.User, opts // Add files, if any. var sha string if opts.Files.Has() { + assert.True(t, autoInit, "Files cannot be specified if AutoInit is disabled") files := opts.Files.Value() resp, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, owner, &files_service.ChangeRepoFilesOptions{ From 21911bfe57a899736987f621f3970a8734d9395f Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 30 Apr 2024 13:06:31 +0200 Subject: [PATCH 3/3] Add a test case for unsubscribing from an issue Signed-off-by: Gergely Nagy --- tests/integration/issue_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 49d1cbb01..e09656c4e 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/indexer/issues" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -876,3 +877,23 @@ body: }) }) } + +func TestIssueUnsubscription(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + repo, _, f := CreateDeclarativeRepoWithOptions(t, user, DeclarativeRepoOptions{ + AutoInit: optional.Some(false), + }) + defer f() + session := loginUser(t, user.Name) + + issueURL := testNewIssue(t, session, user.Name, repo.Name, "Issue title", "Description") + req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/watch", issueURL), map[string]string{ + "_csrf": GetCSRF(t, session, issueURL), + "watch": "0", + }) + session.MakeRequest(t, req, http.StatusOK) + }) +}