From b08aef967ede128e231b66fedfcbb068ca06a590 Mon Sep 17 00:00:00 2001
From: Gergely Nagy <forgejo@gergo.csillger.hu>
Date: Tue, 30 Apr 2024 11:55:46 +0200
Subject: [PATCH] Use PostFormValue instead of PostForm.Get

In `repo.RemoveDependency`, use `PostFormValue` instead of
`PostForm.Get`. The latter requires `ParseForm()` to be called prior,
and in this case, has no benefit over `PostFormValue` anyway (which
calls `ParseForm()` if necessary).

While this currently does not cause any issue as far as I can tell, it
feels like a bug lying in wait for the perfect opportunity. Lets squash
it before it can do harm.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
---
 routers/web/repo/issue_dependency.go |  2 +-
 tests/integration/issue_test.go      | 88 ++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go
index e3b85ee63..66b38688e 100644
--- a/routers/web/repo/issue_dependency.go
+++ b/routers/web/repo/issue_dependency.go
@@ -109,7 +109,7 @@ func RemoveDependency(ctx *context.Context) {
 	}
 
 	// Dependency Type
-	depTypeStr := ctx.Req.PostForm.Get("dependencyType")
+	depTypeStr := ctx.Req.PostFormValue("dependencyType")
 
 	var depType issues_model.DependencyType
 
diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go
index 49d1cbb01..27b95300d 100644
--- a/tests/integration/issue_test.go
+++ b/tests/integration/issue_test.go
@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -193,6 +194,93 @@ func TestNewIssue(t *testing.T) {
 	testNewIssue(t, session, "user2", "repo1", "Title", "Description")
 }
 
+func TestIssueDependencies(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	session := loginUser(t, owner.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
+
+	repo, _, f := CreateDeclarativeRepoWithOptions(t, owner, DeclarativeRepoOptions{})
+	defer f()
+
+	createIssue := func(t *testing.T, title string) api.Issue {
+		t.Helper()
+
+		urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues", owner.Name, repo.Name)
+		req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{
+			Body:  "",
+			Title: title,
+		}).AddTokenAuth(token)
+		resp := MakeRequest(t, req, http.StatusCreated)
+
+		var apiIssue api.Issue
+		DecodeJSON(t, resp, &apiIssue)
+
+		return apiIssue
+	}
+	addDependency := func(t *testing.T, issue, dependency api.Issue) {
+		t.Helper()
+
+		urlStr := fmt.Sprintf("/%s/%s/issues/%d/dependency/add", owner.Name, repo.Name, issue.Index)
+		req := NewRequestWithValues(t, "POST", urlStr, map[string]string{
+			"_csrf":         GetCSRF(t, session, fmt.Sprintf("/%s/%s/issues/%d", owner.Name, repo.Name, issue.Index)),
+			"newDependency": fmt.Sprintf("%d", dependency.Index),
+		})
+		session.MakeRequest(t, req, http.StatusSeeOther)
+	}
+	removeDependency := func(t *testing.T, issue, dependency api.Issue) {
+		t.Helper()
+
+		urlStr := fmt.Sprintf("/%s/%s/issues/%d/dependency/delete", owner.Name, repo.Name, issue.Index)
+		req := NewRequestWithValues(t, "POST", urlStr, map[string]string{
+			"_csrf":              GetCSRF(t, session, fmt.Sprintf("/%s/%s/issues/%d", owner.Name, repo.Name, issue.Index)),
+			"removeDependencyID": fmt.Sprintf("%d", dependency.Index),
+			"dependencyType":     "blockedBy",
+		})
+		session.MakeRequest(t, req, http.StatusSeeOther)
+	}
+
+	assertHasDependency := func(t *testing.T, issueID, dependencyID int64, hasDependency bool) {
+		t.Helper()
+
+		urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/dependencies", owner.Name, repo.Name, issueID)
+		req := NewRequest(t, "GET", urlStr)
+		resp := MakeRequest(t, req, http.StatusOK)
+
+		var issues []api.Issue
+		DecodeJSON(t, resp, &issues)
+
+		if hasDependency {
+			assert.NotEmpty(t, issues)
+			assert.EqualValues(t, issues[0].Index, dependencyID)
+		} else {
+			assert.Empty(t, issues)
+		}
+	}
+
+	t.Run("Add dependency", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		issue1 := createIssue(t, "issue #1")
+		issue2 := createIssue(t, "issue #2")
+		addDependency(t, issue1, issue2)
+
+		assertHasDependency(t, issue1.Index, issue2.Index, true)
+	})
+
+	t.Run("Remove dependency", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		issue1 := createIssue(t, "issue #1")
+		issue2 := createIssue(t, "issue #2")
+		addDependency(t, issue1, issue2)
+		removeDependency(t, issue1, issue2)
+
+		assertHasDependency(t, issue1.Index, issue2.Index, false)
+	})
+}
+
 func TestIssueCommentClose(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	session := loginUser(t, "user2")