From 49b3b7df1181f3b5ffe63fe9536c8f1d5b485073 Mon Sep 17 00:00:00 2001 From: Ashok Siyani Date: Tue, 19 Mar 2024 14:28:14 +0000 Subject: [PATCH] repo name can contain ., -, and _ also .git not req --- pkg/mirror/git_url.go | 22 ++++++++++++++-------- pkg/mirror/git_url_test.go | 36 ++++++++++++++++++++++++++++++------ pkg/mirror/repository.go | 6 +++++- pkg/mirror/z_e2e_test.go | 8 ++++---- 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/pkg/mirror/git_url.go b/pkg/mirror/git_url.go index 9b9cfb7..a69821e 100644 --- a/pkg/mirror/git_url.go +++ b/pkg/mirror/git_url.go @@ -7,17 +7,20 @@ import ( ) var ( + // The repository name can contain + // ASCII letters, digits, and the characters ., -, and _. + // user@host.xz:path/to/repo.git - scpURLRgx = regexp.MustCompile(`^(?P[a-zA-Z]+)@(?P(\w+\.?\w+)+(\:\d+)?):(?P(\w+\/)*)(?P\w+.git)$`) + scpURLRgx = regexp.MustCompile(`^(?P[\w\-\.]+)@(?P([\w\-]+\.?[\w\-]+)+(\:\d+)?):(?P([\w\-\.]+\/)*)(?P[\w\-\.]+(\.git)?)$`) // ssh://user@host.xz[:port]/path/to/repo.git - sshURLRgx = regexp.MustCompile(`^ssh://(?P[a-zA-Z]+)@(?P(\w+\.?\w+)+(\:\d+)??)/(?P(\w+\/)*)(?P\w+.git)$`) + sshURLRgx = regexp.MustCompile(`^ssh://(?P[\w\-\.]+)@(?P([\w\-]+\.?[\w\-]+)+(\:\d+)??)/(?P([\w\-\.]+\/)*)(?P[\w\-\.]+(\.git)?)$`) // https://host.xz[:port]/path/to/repo.git - httpsURLRgx = regexp.MustCompile(`^https://(?P(\w+\.?\w+)+(\:\d+)?)/(?P(\w+\/)*)(?P\w+.git)$`) + httpsURLRgx = regexp.MustCompile(`^https://(?P([\w\-]+\.?[\w\-]+)+(\:\d+)?)/(?P([\w\-\.]+\/)*)(?P[\w\-\.]+(\.git)?)$`) // file:///path/to/repo.git - localURLRgx = regexp.MustCompile(`^file:///(?P([\w-]+\/)*)(?P\w+.git)$`) + localURLRgx = regexp.MustCompile(`^file:///(?P([\w\-\.]+\/)*)(?P[\w\-\.]+(\.git)?)$`) ) type GitURL struct { @@ -32,10 +35,6 @@ func NormaliseURL(rawURL string) string { nURL := strings.ToLower(strings.TrimSpace(rawURL)) nURL = strings.TrimRight(nURL, "/") - // add .git suffix if needed - if !strings.HasSuffix(nURL, ".git") { - nURL = nURL + ".git" - } return nURL } @@ -83,6 +82,13 @@ func ParseGitURL(rawURL string) (*GitURL, error) { // also removing training "/" for consistency gURL.path = strings.Trim(gURL.path, "/") + if gURL.path == "" { + return nil, fmt.Errorf("repo path (org) cannot be empty") + } + if gURL.repo == "" || gURL.repo == ".git" { + return nil, fmt.Errorf("repo name is invalid") + } + return gURL, nil } diff --git a/pkg/mirror/git_url_test.go b/pkg/mirror/git_url_test.go index e77a691..21777a6 100644 --- a/pkg/mirror/git_url_test.go +++ b/pkg/mirror/git_url_test.go @@ -20,29 +20,53 @@ func TestParseGitURL(t *testing.T) { false, }, {"2", - "git@github.com:org/repo.git", - &GitURL{scheme: "scp", user: "git", host: "github.com", path: "org", repo: "repo.git"}, + "git@github.com:org/repo", + &GitURL{scheme: "scp", user: "git", host: "github.com", path: "org", repo: "repo"}, false}, {"3", "ssh://user@host.xz:123/path/to/repo.git", &GitURL{scheme: "ssh", user: "user", host: "host.xz:123", path: "path/to", repo: "repo.git"}, false}, {"4", - "ssh://git@github.com/org/repo.git", - &GitURL{scheme: "ssh", user: "git", host: "github.com", path: "org", repo: "repo.git"}, + "ssh://git@github.com/org/repo", + &GitURL{scheme: "ssh", user: "git", host: "github.com", path: "org", repo: "repo"}, false}, {"5", "https://host.xz:345/path/to/repo.git", &GitURL{scheme: "https", host: "host.xz:345", path: "path/to", repo: "repo.git"}, false}, {"6", - "https://github.com/org/repo.git", - &GitURL{scheme: "https", host: "github.com", path: "org", repo: "repo.git"}, + "https://github.com/org/repo", + &GitURL{scheme: "https", host: "github.com", path: "org", repo: "repo"}, false}, {"7", "https://host.xz:123/path/to/repo.git", &GitURL{scheme: "https", host: "host.xz:123", path: "path/to", repo: "repo.git"}, false}, + { + "valid-special-char-scp", + "user.name-with_@host-with_x.xz:123:path-with_.x/to/prr.test_test-repo0.git", + &GitURL{scheme: "scp", user: "user.name-with_", host: "host-with_x.xz:123", path: "path-with_.x/to", repo: "prr.test_test-repo0.git"}, + false, + }, + { + "valid-special-char-ssh", + "ssh://user.name-with_@host-with_x.xz:123/path-with_.x/to/prr.test_test-repo1.git", + &GitURL{scheme: "ssh", user: "user.name-with_", host: "host-with_x.xz:123", path: "path-with_.x/to", repo: "prr.test_test-repo1.git"}, + false, + }, + { + "valid-special-char-https", + "https://host-with_x.xz:123/path-with_.x/to/prr.test_test-repo2.git", + &GitURL{scheme: "https", host: "host-with_x.xz:123", path: "path-with_.x/to", repo: "prr.test_test-repo2.git"}, + false, + }, + { + "valid-special-char-local", + "file:///path-with_.x/to/prr.test_test-repo3.git", + &GitURL{scheme: "local", path: "path-with_.x/to", repo: "prr.test_test-repo3.git"}, + false, + }, {"invalid_ssh_hostname", "ssh://git@github.com:org/repo.git", nil, true}, {"invalid_scp_url", "git@github.com/org/repo.git", nil, true}, diff --git a/pkg/mirror/repository.go b/pkg/mirror/repository.go index aa539b8..a2f7746 100644 --- a/pkg/mirror/repository.go +++ b/pkg/mirror/repository.go @@ -96,7 +96,11 @@ func NewRepository(repoConf RepositoryConfig, envs []string, log *slog.Logger) ( // hence we can add repo dir (with .git suffix to indicate bare repo) to the provided root. // this also makes it safe to delete this dir and re-create it if needed // also this root could have been shared with other mirror repository (repoPool) - repoDir := filepath.Join(repoConf.Root, gURL.repo) + repoDir := gURL.repo + if !strings.HasSuffix(repoDir, ".git") { + repoDir += ".git" + } + repoDir = filepath.Join(repoConf.Root, repoDir) repo := &Repository{ gitURL: gURL, diff --git a/pkg/mirror/z_e2e_test.go b/pkg/mirror/z_e2e_test.go index f2f2437..07f9fe2 100644 --- a/pkg/mirror/z_e2e_test.go +++ b/pkg/mirror/z_e2e_test.go @@ -13,7 +13,7 @@ import ( ) const ( - testUpstreamRepo = "upstream.git" + testUpstreamRepo = "upstream1" testRoot = "root" testInterval = 1 * time.Second @@ -949,7 +949,7 @@ func Test_RepoPool_Success(t *testing.T) { upstream1 := filepath.Join(testTmpDir, testUpstreamRepo) remote1 := "file://" + upstream1 - upstream2 := filepath.Join(testTmpDir, "upstream2.git") + upstream2 := filepath.Join(testTmpDir, "upstream2") remote2 := "file://" + upstream2 root := filepath.Join(testTmpDir, testRoot) @@ -1136,7 +1136,7 @@ func Test_RepoPool_Error(t *testing.T) { upstream1 := filepath.Join(testTmpDir, testUpstreamRepo) remote1 := "file://" + upstream1 - upstream2 := filepath.Join(testTmpDir, "upstream2.git") + upstream2 := filepath.Join(testTmpDir, "upstream2") remote2 := "file://" + upstream2 root := filepath.Join(testTmpDir, testRoot) @@ -1234,7 +1234,7 @@ func TestRepoPool_validateLinkPath(t *testing.T) { upstream1 := filepath.Join(testTmpDir, testUpstreamRepo) remote1 := "file://" + upstream1 - upstream2 := filepath.Join(testTmpDir, "upstream2.git") + upstream2 := filepath.Join(testTmpDir, "upstream2") remote2 := "file://" + upstream2 root := filepath.Join(testTmpDir, testRoot)