Use mysql.ParseDSN func instead of url.Parse

The MySQL DB driver has it's own DSN parsing function. Previously we were using the url.Parse function, but this causes problems because a valid MySQL DSN can be an invalid http URL, namely when using some special characters in the password. This change uses the MySQL DB driver's builtin ParseDSN function and applies a timeout parameter natively via that. Another benefit of this change is that we fail earlier if given an invalid MySQL DSN. closes #870 closes #1842
influxdata · Oct 12, 2016 · 7026f54 · 7026f54
1 parent 974221f
commit 7026f54
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 23 deletions.
diff --git a/cmd/telegraf/telegraf.go b/cmd/telegraf/telegraf.go
@@ -196,7 +196,7 @@ func reloadLoop(stop chan struct{}, s service.Service) {
 		if *fConfigDirectory != "" {
 			err = c.LoadDirectory(*fConfigDirectory)
 			if err != nil {
-				log.Fatal(err)
+				log.Fatal("E! " + err.Error())
 			}
 		}
 		if len(c.Outputs) == 0 {
@@ -208,7 +208,7 @@ func reloadLoop(stop chan struct{}, s service.Service) {
 
 		ag, err := agent.NewAgent(c)
 		if err != nil {
-			log.Fatal(err)
+			log.Fatal("E! " + err.Error())
 		}
 
 		// Setup logging
@@ -221,14 +221,14 @@ func reloadLoop(stop chan struct{}, s service.Service) {
 		if *fTest {
 			err = ag.Test()
 			if err != nil {
-				log.Fatal(err)
+				log.Fatal("E! " + err.Error())
 			}
 			return
 		}
 
 		err = ag.Connect()
 		if err != nil {
-			log.Fatal(err)
+			log.Fatal("E! " + err.Error())
 		}
 
 		shutdown := make(chan struct{})
@@ -304,7 +304,7 @@ func main() {
 		prg := &program{}
 		s, err := service.New(prg, svcConfig)
 		if err != nil {
-			log.Fatal(err)
+			log.Fatal("E! " + err.Error())
 		}
 		// Handle the -service flag here to prevent any issues with tooling that
 		// may not have an interactive session, e.g. installing from Ansible.
@@ -314,7 +314,7 @@ func main() {
 			}
 			err := service.Control(s, *fService)
 			if err != nil {
-				log.Fatal(err)
+				log.Fatal("E! " + err.Error())
 			}
 		} else {
 			err = s.Run()

diff --git a/plugins/inputs/mysql/mysql.go b/plugins/inputs/mysql/mysql.go
@@ -4,16 +4,16 @@ import (
 	"bytes"
 	"database/sql"
 	"fmt"
-	"net/url"
 	"strconv"
 	"strings"
 	"sync"
 	"time"
 
-	_ "github.com/go-sql-driver/mysql"
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/internal/errchan"
 	"github.com/influxdata/telegraf/plugins/inputs"
+
+	"github.com/go-sql-driver/mysql"
 )
 
 type Mysql struct {
@@ -399,24 +399,16 @@ var (
 )
 
 func dsnAddTimeout(dsn string) (string, error) {
-
-	// DSN "?timeout=5s" is not valid, but "/?timeout=5s" is valid ("" and "/"
-	// are the same DSN)
-	if dsn == "" {
-		dsn = "/"
-	}
-	u, err := url.Parse(dsn)
+	conf, err := mysql.ParseDSN(dsn)
 	if err != nil {
 		return "", err
 	}
-	v := u.Query()
 
-	// Only override timeout if not already defined
-	if _, ok := v["timeout"]; ok == false {
-		v.Add("timeout", defaultTimeout.String())
-		u.RawQuery = v.Encode()
+	if conf.Timeout == 0 {
+		conf.Timeout = time.Second * 5
 	}
-	return u.String(), nil
+
+	return conf.FormatDSN(), nil
 }
 
 // Math constants

diff --git a/plugins/inputs/mysql/mysql_test.go b/plugins/inputs/mysql/mysql_test.go
@@ -92,7 +92,7 @@ func TestMysqlDNSAddTimeout(t *testing.T) {
 	}{
 		{
 			"",
-			"/?timeout=5s",
+			"tcp(127.0.0.1:3306)/?timeout=5s",
 		},
 		{
 			"tcp(192.168.1.1:3306)/",
@@ -104,7 +104,19 @@ func TestMysqlDNSAddTimeout(t *testing.T) {
 		},
 		{
 			"root:passwd@tcp(192.168.1.1:3306)/?tls=false&timeout=10s",
-			"root:passwd@tcp(192.168.1.1:3306)/?tls=false&timeout=10s",
+			"root:passwd@tcp(192.168.1.1:3306)/?timeout=10s&tls=false",
+		},
+		{
+			"tcp(10.150.1.123:3306)/",
+			"tcp(10.150.1.123:3306)/?timeout=5s",
+		},
+		{
+			"root:@!~(*&$#%(&@#(@&#Password@tcp(10.150.1.123:3306)/",
+			"root:@!~(*&$#%(&@#(@&#Password@tcp(10.150.1.123:3306)/?timeout=5s",
+		},
+		{
+			"root:Test3a#@!@tcp(10.150.1.123:3306)/",
+			"root:Test3a#@!@tcp(10.150.1.123:3306)/?timeout=5s",
 		},
 	}