Escape DSN param values

Not really a behavior change, since it was mostly broken before

Author: julienschmidt

README.md

@@ -110,7 +110,7 @@ Possible Parameters are:
   * `allowOldPasswords`: `allowAllFiles=true` allows the usage of the insecure old password method. This should be avoided, but is necessary in some cases. See also [the old_passwords wiki page](https://github.com/go-sql-driver/mysql/wiki/old_passwords).
   * `charset`: Sets the charset used for client-server interaction ("SET NAMES `value`"). If multiple charsets are set (separated by a comma), the following charset is used if setting the charset failes. This enables support for `utf8mb4` ([introduced in MySQL 5.5.3](http://dev.mysql.com/doc/refman/5.5/en/charset-unicode-utf8mb4.html)) with fallback to `utf8` for older servers (`charset=utf8mb4,utf8`).
   * `clientFoundRows`: `clientFoundRows=true` causes an UPDATE to return the number of matching rows instead of the number of rows changed.
-  * `loc`: Sets the location for time.Time values (when using `parseTime=true`). The default is `UTC`. *"Local"* sets the system's location. See [time.LoadLocation](http://golang.org/pkg/time/#LoadLocation) for details.
+  * `loc`: Sets the location for time.Time values (when using `parseTime=true`). The default is `UTC`. *"Local"* sets the system's location. See [time.LoadLocation](http://golang.org/pkg/time/#LoadLocation) for details. Please keep in mind, that param values must be [url.QueryEscape](http://golang.org/pkg/net/url/#QueryEscape)'ed. Alternatively you can manually replace the `/` with `%2F`. For example `US/Pacific` would be `US%2FPacific`.
   * `parseTime`: `parseTime=true` changes the output type of `DATE` and `DATETIME` values to `time.Time` instead of `[]byte` / `string`
   * `strict`: Enable strict mode. MySQL warnings are treated as errors.
   * `timeout`: **Driver** side connection timeout. The value must be a string of decimal numbers, each with optional fraction and a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. To set a server side timeout, use the parameter [`wait_timeout`](http://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_wait_timeout).
@@ -122,6 +122,8 @@ All other parameters are interpreted as system variables:
   * `tx_isolation`: *"SET [tx_isolation](https://dev.mysql.com/doc/refman/5.5/en/server-system-variables.html#sysvar_tx_isolation)=`value`"*
   * `param`: *"SET `param`=`value`"*
 
+***The values must be [url.QueryEscape](http://golang.org/pkg/net/url/#QueryEscape)'ed!***
+
 #### Examples
 ```
 user@unix(/path/to/socket)/dbname
@@ -132,7 +134,7 @@ user:password@tcp(localhost:5555)/dbname?autocommit=true
 ```
 
 ```
-user:password@tcp([de:ad:be:ef::ca:fe]:80)/dbname?tls=skip-verify&charset=utf8mb4,utf8
+user:password@tcp([de:ad:be:ef::ca:fe]:80)/dbname?tls=skip-verify&charset=utf8mb4,utf8&sys_var=withSlash%2FandAt%40
 ```
 
 ```

driver_test.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -206,7 +207,7 @@ func TestTimezoneConversion(t *testing.T) {
 	}
 
 	for _, tz := range zones {
-		runTests(t, dsn+"&parseTime=true&loc="+tz, tzTest)
+		runTests(t, dsn+"&parseTime=true&loc="+url.QueryEscape(tz), tzTest)
 	}
 }
 

utils.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net/url"
 	"os"
 	"strings"
 	"time"
@@ -26,7 +27,8 @@ var (
 	errLog            *log.Logger            // Error Logger
 	tlsConfigRegister map[string]*tls.Config // Register for custom tls.Configs
 
-	errInvalidDSN = errors.New("Invalid DSN")
+	errInvalidDSNUnescaped = errors.New("Invalid DSN: Did you forget to escape a param value?")
+	errInvalidDSNAddr      = errors.New("Invalid DSN: Network Address not terminated (missing closing brace)")
 )
 
 func init() {
@@ -71,13 +73,14 @@ func DeregisterTLSConfig(key string) {
 	delete(tlsConfigRegister, key)
 }
 
+// parseDSN parses the DSN string to a config
 func parseDSN(dsn string) (cfg *config, err error) {
 	cfg = new(config)
 
 	// TODO: use strings.IndexByte when we can depend on Go 1.2
 
 	// [user[:password]@][net[(addr)]]/dbname[?param1=value1&paramN=valueN]
-	// Find the last '/'
+	// Find the last '/' (since the password might contain a '/')
 	for i := len(dsn) - 1; i >= 0; i-- {
 		if dsn[i] == '/' {
 			var j int
@@ -105,7 +108,10 @@ func parseDSN(dsn string) (cfg *config, err error) {
 							if dsn[k] == '(' {
 								// dsn[i-1] must be == ')' if an adress is specified
 								if dsn[i-1] != ')' {
-									return nil, errInvalidDSN
+									if strings.ContainsRune(dsn[k+1:i], ')') {
+										return nil, errInvalidDSNUnescaped
+									}
+									return nil, errInvalidDSNAddr
 								}
 								cfg.addr = dsn[k+1 : i-1]
 								break
@@ -119,7 +125,7 @@ func parseDSN(dsn string) (cfg *config, err error) {
 
 				// non-empty left part must contain an '@'
 				if j < 0 {
-					return nil, errInvalidDSN
+					return nil, errInvalidDSNUnescaped
 				}
 			}
 
@@ -157,17 +163,17 @@ func parseDSN(dsn string) (cfg *config, err error) {
 
 	}
 
-	// Set default location if not set
+	// Set default location if empty
 	if cfg.loc == nil {
 		cfg.loc = time.UTC
 	}
 
 	return
 }
 
+// parseDSNParams parses the DSN "query string"
+// Values must be url.QueryEscape'ed
 func parseDSNParams(cfg *config, params string) (err error) {
-	cfg.params = make(map[string]string)
-
 	for _, v := range strings.Split(params, "&") {
 		param := strings.SplitN(v, "=", 2)
 		if len(param) != 2 {
@@ -203,6 +209,9 @@ func parseDSNParams(cfg *config, params string) (err error) {
 
 		// Time Location
 		case "loc":
+			if value, err = url.QueryUnescape(value); err != nil {
+				return
+			}
 			cfg.loc, err = time.LoadLocation(value)
 			if err != nil {
 				return
@@ -233,7 +242,14 @@ func parseDSNParams(cfg *config, params string) (err error) {
 			}
 
 		default:
-			cfg.params[param[0]] = value
+			// lazy init
+			if cfg.params == nil {
+				cfg.params = make(map[string]string)
+			}
+
+			if cfg.params[param[0]], err = url.QueryUnescape(value); err != nil {
+				return
+			}
 		}
 	}
 

utils_test.go

@@ -30,7 +30,7 @@ var testDSNs = []struct {
 	{"/", "&{user: passwd: net:tcp addr:127.0.0.1:3306 dbname: params:map[] loc:%p timeout:0 tls:<nil> allowAllFiles:false allowOldPasswords:false clientFoundRows:false}", time.UTC},
 	{"", "&{user: passwd: net:tcp addr:127.0.0.1:3306 dbname: params:map[] loc:%p timeout:0 tls:<nil> allowAllFiles:false allowOldPasswords:false clientFoundRows:false}", time.UTC},
 	{"user:p@/ssword@/", "&{user:user passwd:p@/ssword net:tcp addr:127.0.0.1:3306 dbname: params:map[] loc:%p timeout:0 tls:<nil> allowAllFiles:false allowOldPasswords:false clientFoundRows:false}", time.UTC},
-	{"@unix/", "&{user: passwd: net:unix addr:/tmp/mysql.sock dbname: params:map[] loc:%p timeout:0 tls:<nil> allowAllFiles:false allowOldPasswords:false clientFoundRows:false}", time.UTC},
+	{"@unix/?arg=%2Fsome%2Fpath.ext", "&{user: passwd: net:unix addr:/tmp/mysql.sock dbname: params:map[arg:/some/path.ext] loc:%p timeout:0 tls:<nil> allowAllFiles:false allowOldPasswords:false clientFoundRows:false}", time.UTC},
 }
 
 func TestDSNParser(t *testing.T) {
@@ -57,6 +57,7 @@ func TestDSNParser(t *testing.T) {
 func TestDSNParserInvalid(t *testing.T) {
 	var invalidDSNs = []string{
 		"asdf/dbname",
+		"@net(addr/",
 		//"/dbname?arg=/some/unescaped/path",
 	}