Only allow returned deleted branche to be on repo (#17570)
- This will only allow `GetDeletedBranchByID` to return deletedBranch which are on the repo, and thus don't return a deletedBranch from another repo. - This just should prevent possible bugs in the futher when a code is passing the wrong ID into this function.
This commit is contained in:
		
							parent
							
								
									ebaf4c48ea
								
							
						
					
					
						commit
						640f0e1ddf
					
				| @ -536,7 +536,7 @@ func (repo *Repository) GetDeletedBranches() ([]*DeletedBranch, error) { | |||||||
| // GetDeletedBranchByID get a deleted branch by its ID
 | // GetDeletedBranchByID get a deleted branch by its ID
 | ||||||
| func (repo *Repository) GetDeletedBranchByID(id int64) (*DeletedBranch, error) { | func (repo *Repository) GetDeletedBranchByID(id int64) (*DeletedBranch, error) { | ||||||
| 	deletedBranch := &DeletedBranch{} | 	deletedBranch := &DeletedBranch{} | ||||||
| 	has, err := db.GetEngine(db.DefaultContext).ID(id).Get(deletedBranch) | 	has, err := db.GetEngine(db.DefaultContext).Where("repo_id = ?", repo.ID).And("id = ?", id).Get(deletedBranch) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, err | ||||||
| 	} | 	} | ||||||
|  | |||||||
| @ -128,3 +128,28 @@ func TestRenameBranch(t *testing.T) { | |||||||
| 		BranchName: "main", | 		BranchName: "main", | ||||||
| 	}) | 	}) | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | func TestOnlyGetDeletedBranchOnCorrectRepo(t *testing.T) { | ||||||
|  | 	assert.NoError(t, db.PrepareTestDatabase()) | ||||||
|  | 
 | ||||||
|  | 	// Get deletedBranch with ID of 1 on repo with ID 2.
 | ||||||
|  | 	// This should return a nil branch as this deleted branch
 | ||||||
|  | 	// is actually on repo with ID 1.
 | ||||||
|  | 	repo2 := db.AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository) | ||||||
|  | 
 | ||||||
|  | 	deletedBranch, err := repo2.GetDeletedBranchByID(1) | ||||||
|  | 
 | ||||||
|  | 	// Expect no error, and the returned branch is nil.
 | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  | 	assert.Nil(t, deletedBranch) | ||||||
|  | 
 | ||||||
|  | 	// Now get the deletedBranch with ID of 1 on repo with ID 1.
 | ||||||
|  | 	// This should return the deletedBranch.
 | ||||||
|  | 	repo1 := db.AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) | ||||||
|  | 
 | ||||||
|  | 	deletedBranch, err = repo1.GetDeletedBranchByID(1) | ||||||
|  | 
 | ||||||
|  | 	// Expect no error, and the returned branch to be not nil.
 | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  | 	assert.NotNil(t, deletedBranch) | ||||||
|  | } | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user