mirror of
				https://codeberg.org/forgejo/forgejo.git
				synced 2025-11-04 08:21:11 +00:00 
			
		
		
		
	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>
This commit is contained in:
		
					parent
					
						
							
								6ba60f61cb
							
						
					
				
			
			
				commit
				
					
						b08aef967e
					
				
			
		
					 2 changed files with 89 additions and 1 deletions
				
			
		| 
						 | 
					@ -109,7 +109,7 @@ func RemoveDependency(ctx *context.Context) {
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// Dependency Type
 | 
						// Dependency Type
 | 
				
			||||||
	depTypeStr := ctx.Req.PostForm.Get("dependencyType")
 | 
						depTypeStr := ctx.Req.PostFormValue("dependencyType")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	var depType issues_model.DependencyType
 | 
						var depType issues_model.DependencyType
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -15,6 +15,7 @@ import (
 | 
				
			||||||
	"testing"
 | 
						"testing"
 | 
				
			||||||
	"time"
 | 
						"time"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						auth_model "code.gitea.io/gitea/models/auth"
 | 
				
			||||||
	"code.gitea.io/gitea/models/db"
 | 
						"code.gitea.io/gitea/models/db"
 | 
				
			||||||
	issues_model "code.gitea.io/gitea/models/issues"
 | 
						issues_model "code.gitea.io/gitea/models/issues"
 | 
				
			||||||
	repo_model "code.gitea.io/gitea/models/repo"
 | 
						repo_model "code.gitea.io/gitea/models/repo"
 | 
				
			||||||
| 
						 | 
					@ -193,6 +194,93 @@ func TestNewIssue(t *testing.T) {
 | 
				
			||||||
	testNewIssue(t, session, "user2", "repo1", "Title", "Description")
 | 
						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) {
 | 
					func TestIssueCommentClose(t *testing.T) {
 | 
				
			||||||
	defer tests.PrepareTestEnv(t)()
 | 
						defer tests.PrepareTestEnv(t)()
 | 
				
			||||||
	session := loginUser(t, "user2")
 | 
						session := loginUser(t, "user2")
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue