Fix repository adoption on Windows (#21646) (#21650)

Backport #21646

A bug was introduced in #17865 where filepath.Join is used to join
putative unadopted repository owner and names together. This is
incorrect as these names are then used as repository names - which shoud
have the '/' separator. This means that adoption will not work on
Windows servers.

Fix #21632

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2022-11-01 22:32:03 +00:00 committed by GitHub
parent fd4e7447e7
commit 3a0d000b94
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 9 deletions

View File

@ -8,6 +8,7 @@ import (
"context" "context"
"fmt" "fmt"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
@ -218,21 +219,21 @@ func DeleteUnadoptedRepository(doer, u *user_model.User, repoName string) error
return util.RemoveAll(repoPath) return util.RemoveAll(repoPath)
} }
type unadoptedRrepositories struct { type unadoptedRepositories struct {
repositories []string repositories []string
index int index int
start int start int
end int end int
} }
func (unadopted *unadoptedRrepositories) add(repository string) { func (unadopted *unadoptedRepositories) add(repository string) {
if unadopted.index >= unadopted.start && unadopted.index < unadopted.end { if unadopted.index >= unadopted.start && unadopted.index < unadopted.end {
unadopted.repositories = append(unadopted.repositories, repository) unadopted.repositories = append(unadopted.repositories, repository)
} }
unadopted.index++ unadopted.index++
} }
func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unadopted *unadoptedRrepositories) error { func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unadopted *unadoptedRepositories) error {
if len(repoNamesToCheck) == 0 { if len(repoNamesToCheck) == 0 {
return nil return nil
} }
@ -264,7 +265,7 @@ func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unad
} }
for _, repoName := range repoNamesToCheck { for _, repoName := range repoNamesToCheck {
if !repoNames.Contains(repoName) { if !repoNames.Contains(repoName) {
unadopted.add(filepath.Join(userName, repoName)) unadopted.add(path.Join(userName, repoName)) // These are not used as filepaths - but as reponames - therefore use path.Join not filepath.Join
} }
} }
return nil return nil
@ -292,7 +293,7 @@ func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, in
var repoNamesToCheck []string var repoNamesToCheck []string
start := (opts.Page - 1) * opts.PageSize start := (opts.Page - 1) * opts.PageSize
unadopted := &unadoptedRrepositories{ unadopted := &unadoptedRepositories{
repositories: make([]string, 0, opts.PageSize), repositories: make([]string, 0, opts.PageSize),
start: start, start: start,
end: start + opts.PageSize, end: start + opts.PageSize,

View File

@ -19,7 +19,7 @@ import (
func TestCheckUnadoptedRepositories_Add(t *testing.T) { func TestCheckUnadoptedRepositories_Add(t *testing.T) {
start := 10 start := 10
end := 20 end := 20
unadopted := &unadoptedRrepositories{ unadopted := &unadoptedRepositories{
start: start, start: start,
end: end, end: end,
index: 0, index: 0,
@ -39,7 +39,7 @@ func TestCheckUnadoptedRepositories(t *testing.T) {
// //
// Non existent user // Non existent user
// //
unadopted := &unadoptedRrepositories{start: 0, end: 100} unadopted := &unadoptedRepositories{start: 0, end: 100}
err := checkUnadoptedRepositories("notauser", []string{"repo"}, unadopted) err := checkUnadoptedRepositories("notauser", []string{"repo"}, unadopted)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, 0, len(unadopted.repositories)) assert.Equal(t, 0, len(unadopted.repositories))
@ -50,14 +50,14 @@ func TestCheckUnadoptedRepositories(t *testing.T) {
userName := "user2" userName := "user2"
repoName := "repo2" repoName := "repo2"
unadoptedRepoName := "unadopted" unadoptedRepoName := "unadopted"
unadopted = &unadoptedRrepositories{start: 0, end: 100} unadopted = &unadoptedRepositories{start: 0, end: 100}
err = checkUnadoptedRepositories(userName, []string{repoName, unadoptedRepoName}, unadopted) err = checkUnadoptedRepositories(userName, []string{repoName, unadoptedRepoName}, unadopted)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, []string{path.Join(userName, unadoptedRepoName)}, unadopted.repositories) assert.Equal(t, []string{path.Join(userName, unadoptedRepoName)}, unadopted.repositories)
// //
// Existing (adopted) repository is not returned // Existing (adopted) repository is not returned
// //
unadopted = &unadoptedRrepositories{start: 0, end: 100} unadopted = &unadoptedRepositories{start: 0, end: 100}
err = checkUnadoptedRepositories(userName, []string{repoName}, unadopted) err = checkUnadoptedRepositories(userName, []string{repoName}, unadopted)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, 0, len(unadopted.repositories)) assert.Equal(t, 0, len(unadopted.repositories))