Repository: danvergara/dblab (go)

Issue:
Implement a better validation for mysql url connection
**Is your feature request related to a problem? Please describe.**
Currenty, I haven't implemented a reliable way to validate mysql urls, due to their peculiarities (e. x. user:password@tcp(localhost:5555)/dbname?tls=skip-verify&autocommit=true). I tried to use [url package](https://golang.org/pkg/net/url/), but it is not able to parse those kind of urls. 

**Describe the solution you'd like**
From my point of view, we should implement our regex solution to parse and validate those kind of urls. 

**Describe alternatives you've considered**
I considered the use of [dburl](https://github.com/xo/dburl), but that package doesn't support mysql urls connection strings either.
Maybe we could take advantage of  [regexp](https://golang.org/pkg/regexp/) from the standard library.

Pull request description:
Mysql url validation
# Mysql url validation

## Description
We had a very naive validation method for mysql dsn's. So I implemented a validation method using regex.

Fixes #44

## Type of change

Please delete options that are not relevant.

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] This change requires a documentation update

## How Has This Been Tested?
I added a new test function with multiple cases. 

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have checked my code and corrected any misspellings


Patch:
diff --git a/pkg/connection/connection.go b/pkg/connection/connection.go
index 20c13f0..137d7b1 100644
--- a/pkg/connection/connection.go
+++ b/pkg/connection/connection.go
@@ -6,12 +6,15 @@ import (
 	"net/url"
 	"os"
 	"os/user"
+	"regexp"
 	"strings"
 
 	"github.com/danvergara/dblab/pkg/command"
 )
 
 var (
+	// pattern used to parse an incoming dsn for mysql connection.
+	dsnPattern *regexp.Regexp
 	// ErrCantDetectUSer is the error used to notify that a default username is not found
 	// in the system to be used as database username.
 	ErrCantDetectUSer = errors.New("could not detect default username")
@@ -25,6 +28,14 @@ var (
 	ErrInvalidDriver = errors.New("invalid driver")
 )
 
+func init() {
+	dsnPattern = regexp.MustCompile(
+		`^(?:(?P<user>.*?)(?::(?P<passwd>.*))?@)?` + // [user[:password]@]
+			`(?:(?P<net>[^\(]*)(?:\((?P<addr>[^\)]*)\))?)?` + // [net[(addr)]]
+			`\/(?P<dbname>.*?)` + // /dbname
+			`(?:\?(?P<params>[^\?]*))?$`) // [?param1=value1&paramN=valueN]
+}
+
 // BuildConnectionFromOpts return the connection uri string given the options passed by the uses.
 func BuildConnectionFromOpts(opts command.Options) (string, error) {
 	if opts.URL != "" {
@@ -127,12 +138,22 @@ func formatMySQLURL(opts command.Options) (string, error) {
 		return "", fmt.Errorf("%s, %w", opts.URL, ErrInvalidMySQLURLFormat)
 	}
 
-	if strings.Contains(opts.URL, "3306") {
-		return opts.URL, nil
-	}
+	var e *url.Error
 
 	uri, err := url.Parse(opts.URL)
 	if err != nil {
+		// checks if *url.Error is the type of the error.
+		// if the url is a dsn for mysql connection
+		// the most likely is this is gonna be true.
+		if errors.As(err, &e) {
+			url, err := parseDSN(opts.URL)
+			if err != nil {
+				return "", fmt.Errorf("%v %w", err, ErrInvalidMySQLURLFormat)
+			}
+
+			return url, nil
+		}
+
 		return "", fmt.Errorf("%v %w", err, ErrInvalidMySQLURLFormat)
 	}
 
@@ -150,6 +171,21 @@ func formatMySQLURL(opts command.Options) (string, error) {
 	return uri.String(), nil
 }
 
+// validates if dsn pattern match with the parameter.
+func parseDSN(dsn string) (string, error) {
+	matches := dsnPattern.FindStringSubmatch(dsn)
+	if matches == nil {
+		return "", errors.New("not match")
+	}
+
+	names := dsnPattern.SubexpNames()
+	if len(names) == 0 {
+		return "", errors.New("not names")
+	}
+
+	return dsn, nil
+}
+
 // hasValidPostgresPrefix checks if a given url has the driver name in it.
 func hasValidPostgresPrefix(rawurl string) bool {
 	return strings.HasPrefix(rawurl, "postgres://") || strings.HasPrefix(rawurl, "postgresql://")


Test patch:
diff --git a/pkg/connection/connection_test.go b/pkg/connection/connection_test.go
index 5dcaaa6..eee661b 100644
--- a/pkg/connection/connection_test.go
+++ b/pkg/connection/connection_test.go
@@ -443,6 +443,30 @@ func TestFormatMySQLURL(t *testing.T) {
 				uri: "mysql://user:password@tcp(your-amazonaws-uri.com:3306)/dbname",
 			},
 		},
+		{
+			name: "valid mysql traditional",
+			given: given{
+				opts: command.Options{
+					Driver: "mysql",
+					URL:    "mysql://user:password@/dbname?sql_mode=TRADITIONAL",
+				},
+			},
+			want: want{
+				uri: "mysql://user:password@/dbname?sql_mode=TRADITIONAL",
+			},
+		},
+		{
+			name: "valid mysql google cloud sql on app engine",
+			given: given{
+				opts: command.Options{
+					Driver: "mysql",
+					URL:    "mysql://user:password@unix(/cloudsql/project-id:region-name:instance-name)/dbname",
+				},
+			},
+			want: want{
+				uri: "mysql://user:password@unix(/cloudsql/project-id:region-name:instance-name)/dbname",
+			},
+		},
 		{
 			name: "error misspelled mysql",
 			given: given{
@@ -493,3 +517,63 @@ func TestFormatMySQLURL(t *testing.T) {
 		})
 	}
 }
+
+func TestParseDSN(t *testing.T) {
+	type given struct {
+		dsn string
+	}
+	type want struct {
+		uri      string
+		hasError bool
+	}
+	var cases = []struct {
+		name  string
+		given given
+		want  want
+	}{
+		{
+			name: "valid mysql traditional",
+			given: given{
+				dsn: "mysql://user:password@/dbname?sql_mode=TRADITIONAL",
+			},
+			want: want{
+				uri: "mysql://user:password@/dbname?sql_mode=TRADITIONAL",
+			},
+		},
+		{
+			name: "valid mysql google cloud sql on app engine",
+			given: given{
+				dsn: "mysql://user:password@unix(/cloudsql/project-id:region-name:instance-name)/dbname",
+			},
+			want: want{
+				uri: "mysql://user:password@unix(/cloudsql/project-id:region-name:instance-name)/dbname",
+			},
+		},
+		{
+			name: "error invalid url",
+			given: given{
+				dsn: "not-a-url",
+			},
+			want: want{
+				hasError: true,
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			url, err := parseDSN(tc.given.dsn)
+
+			if tc.want.hasError {
+				assert.Error(t, err)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tc.want.uri, url)
+		})
+	}
+}