Merge pull request #2 from fredbi/fix-789

fix(gitlabparser): ensured that unexpected path input does not panic
kubescape · Dec 1, 2022 · 4c93c14 · 4c93c14
2 parents d1928d1 + 25f1e40
commit 4c93c14
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 9 deletions.
diff --git a/githubparser/v1/parser_test.go b/githubparser/v1/parser_test.go
@@ -13,6 +13,7 @@ var (
 	urlC = "https://github.com/kubescape/go-git-url/tree/master/files"
 	urlD = "https://raw.githubusercontent.com/kubescape/go-git-url/master/files/file0.json"
 	urlE = "git@github.com:kubescape/go-git-url.git"
+	urlF = "git@github.com:foobar/kubescape/go-git-url.git"
 )
 
 func TestNewGitHubParserWithURL(t *testing.T) {
@@ -67,6 +68,11 @@ func TestNewGitHubParserWithURL(t *testing.T) {
 		assert.Equal(t, "", gh.GetBranchName())
 		assert.Equal(t, "", gh.GetPath())
 	}
+	{
+		assert.NotPanics(t, func() {
+			_, _ = NewGitHubParserWithURL(urlF)
+		})
+	}
 }
 
 func TestSetDefaultBranch(t *testing.T) {

diff --git a/gitlabparser/v1/parser.go b/gitlabparser/v1/parser.go
@@ -75,24 +75,23 @@ func (gl *GitLabURL) Parse(fullURL string) error {
 		return fmt.Errorf("expecting <user>/<repo> in url path, received: '%s'", parsedURL.Path)
 	}
 	gl.owner = splittedRepo[index]
-	index += 1
+	index++
 	gl.repo = strings.TrimSuffix(splittedRepo[index], ".git")
-	index += 1
+	index++
 
 	// root of repo
 	if len(splittedRepo) < index+1 {
 		return nil
 	}
-	index += 1 // skip "-" symbol in URL
+
+	if splittedRepo[index] == "-" {
+		index++ // skip "-" symbol in URL
+	}
 
 	// is file or dir
 	switch splittedRepo[index] {
-	case "blob":
-		index += 1
-	case "tree":
-		index += 1
-	case "raw":
-		index += 1
+	case "blob", "tree", "raw":
+		index++
 	}
 
 	if len(splittedRepo) < index+1 {

diff --git a/gitlabparser/v1/parser_test.go b/gitlabparser/v1/parser_test.go
@@ -13,6 +13,12 @@ var (
 	urlD = "https://gitlab.com/kubescape/testing/-/tree/dev"                                          // branch
 	urlE = "https://gitlab.com/kubescape/testing/-/blob/v0.0.0/README.md"                             // tag
 	urlF = "https://gitlab.com/kubescape/testing/-/raw/main/stable/acs-engine-autoscaler/Chart.yaml"
+	// scp-like syntax supported by git for ssh
+	// see: https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-clone.html#URLS
+	// regular form
+	urlG = "git@gitlab.com:owner/repo.git"
+	// unexpected form: should not panic
+	urlH = "git@gitlab.com:path/to/repo.git"
 )
 
 func TestNewGitHubParserWithURL(t *testing.T) {
@@ -82,4 +88,26 @@ func TestNewGitHubParserWithURL(t *testing.T) {
 		assert.Equal(t, "main", gl.GetBranchName())
 		assert.Equal(t, "stable/acs-engine-autoscaler/Chart.yaml", gl.GetPath())
 	}
+	{
+		gl, err := NewGitLabParserWithURL(urlG)
+		assert.NoError(t, err)
+		assert.Equal(t, "gitlab.com", gl.GetHostName())
+		assert.Equal(t, "gitlab", gl.GetProvider())
+		assert.Equal(t, "owner", gl.GetOwnerName())
+		assert.Equal(t, "repo", gl.GetRepoName())
+		assert.Equal(t, "https://gitlab.com/owner/repo", gl.GetURL().String())
+		assert.Equal(t, "", gl.GetBranchName())
+		assert.Equal(t, "", gl.GetPath())
+	}
+	{
+		gl, err := NewGitLabParserWithURL(urlH)
+		assert.NoError(t, err)
+		assert.Equal(t, "gitlab.com", gl.GetHostName())
+		assert.Equal(t, "gitlab", gl.GetProvider())
+		assert.Equal(t, "path", gl.GetOwnerName())
+		assert.Equal(t, "to", gl.GetRepoName())
+		assert.Equal(t, "https://gitlab.com/path/to", gl.GetURL().String())
+		assert.Equal(t, "repo.git", gl.GetBranchName()) // invalid input leads to incorrect guess. At least this does not panic.
+		assert.Equal(t, "", gl.GetPath())
+	}
 }