change (#276)

add url parse validator.
add gitlab as one of prefix

Author: Greyeye

cmd/locations.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"net/url"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -83,11 +84,12 @@ func maybeCloneGitUrl(ctx context.Context, repoManager cloner, repoRefreshDurati
 	return path, nil
 }
 
-func isGitURL(str string) bool {
-	if IsURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
+func isGitURL(url string) bool {
+	str := strings.ToLower(url)
+	if isValidURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
 		return true
 	}
-	for _, prefix := range []string{"git://", "github.com/", "git@"} {
+	for _, prefix := range []string{"git://", "github.com/", "gitlab.com/", "git@"} {
 		if strings.HasPrefix(str, prefix) {
 			return true
 		}
@@ -99,9 +101,8 @@ func isGitURL(str string) bool {
 // context from the Git repository. See IsGitURL for details.
 var urlPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
 
-// IsURL returns true if the provided str is an HTTP(S) URL by checking if it
-// has a http:// or https:// scheme. No validation is performed to verify if the
-// URL is well-formed.
-func IsURL(str string) bool {
-	return strings.HasPrefix(str, "https://") || strings.HasPrefix(str, "http://")
+// isValidURL returns true if the provided str is a well-formed HTTP(S) URL.
+func isValidURL(str string) bool {
+	u, err := url.Parse(str)
+	return err == nil && (u.Scheme == "http" || u.Scheme == "https")
 }

cmd/locations_test.go

@@ -19,7 +19,7 @@ type fakeCloner struct {
 	err                  error
 }
 
-func (f *fakeCloner) Clone(ctx context.Context, cloneUrl, branchName string) (*git.Repo, error) {
+func (f *fakeCloner) Clone(_ context.Context, cloneUrl, branchName string) (*git.Repo, error) {
 	f.cloneUrl = cloneUrl
 	f.branchName = branchName
 	return f.result, f.err
@@ -199,3 +199,76 @@ func TestMaybeCloneGitUrl_CloneError(t *testing.T) {
 		})
 	}
 }
+
+func Test_isGitURL(t *testing.T) {
+	type args struct {
+		str string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "git url 1",
+			args: args{
+				str: "https://gitlab.com/org/team/project.git",
+			},
+			want: true,
+		},
+		{
+			name: "git url 2",
+			args: args{
+				str: "git://github.com/org/team/project.git",
+			},
+			want: true,
+		},
+		{
+			name: "git url 3",
+			args: args{
+				str: "http://github.com/org/team/project.git",
+			},
+			want: true,
+		},
+		{
+			name: "git url 4",
+			args: args{
+				str: "git://test.local/org/team/project.git",
+			},
+			want: true,
+		},
+		{
+			name: "git url invalid 1",
+			args: args{
+				str: "scp://whatever.com/org/team/project.git",
+			},
+			want: false,
+		},
+		{
+			name: "git url invalid 2",
+			args: args{
+				str: "ftp://github.com/org/team/project.git",
+			},
+			want: false,
+		},
+		{
+			name: "git url invalid 3",
+			args: args{
+				str: "thisisnoturl",
+			},
+			want: false,
+		},
+		{
+			name: "git url invalid 4",
+			args: args{
+				str: "http://zapier.com",
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, isGitURL(tt.args.str), "isGitURL(%v)", tt.args.str)
+		})
+	}
+}