diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index 84f9a3da2..9e822fe7b 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -1377,7 +1377,7 @@ func TestWriteSetConfigResponse(t *testing.T) { }, } - testURL, err := url.Parse("dummy.com") + testURL, err := url.Parse("http://dummy.com") if err != nil { t.Fatalf("Failed to parse a place-holder url") } diff --git a/cmd/config-v18_test.go b/cmd/config-v18_test.go index 341c3182f..f443e1c14 100644 --- a/cmd/config-v18_test.go +++ b/cmd/config-v18_test.go @@ -299,10 +299,10 @@ func TestValidateConfig(t *testing.T) { {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "elasticsearch": { "1": { "enable": true, "format": "namespace", "url": "example.com", "index": "myindex" } }}}`, true}, // Test 27 - Test Format for Redis - {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "invalid", "address": "example.com", "password": "xxx", "key": "key1" } }}}`, false}, + {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "invalid", "address": "example.com:80", "password": "xxx", "key": "key1" } }}}`, false}, // Test 28 - Test valid Format for Redis - {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "namespace", "address": "example.com", "password": "xxx", "key": "key1" } }}}`, true}, + {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "namespace", "address": "example.com:80", "password": "xxx", "key": "key1" } }}}`, true}, } for i, testCase := range testCases { diff --git a/cmd/notify-kafka.go b/cmd/notify-kafka.go index 3ce4112d2..8a938855a 100644 --- a/cmd/notify-kafka.go +++ b/cmd/notify-kafka.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io/ioutil" + "net" "github.com/Sirupsen/logrus" @@ -47,6 +48,12 @@ func (k *kafkaNotify) Validate() error { if len(k.Brokers) == 0 { return errors.New("No broker specified") } + // Validate all specified brokers. + for _, brokerAddr := range k.Brokers { + if _, _, err := net.SplitHostPort(brokerAddr); err != nil { + return err + } + } return nil } diff --git a/cmd/notify-nats.go b/cmd/notify-nats.go index 1659eefc2..7826e3d62 100644 --- a/cmd/notify-nats.go +++ b/cmd/notify-nats.go @@ -18,6 +18,7 @@ package cmd import ( "io/ioutil" + "net" "github.com/Sirupsen/logrus" "github.com/nats-io/go-nats-streaming" @@ -52,7 +53,7 @@ func (n *natsNotify) Validate() error { if !n.Enable { return nil } - if _, err := checkURL(n.Address); err != nil { + if _, _, err := net.SplitHostPort(n.Address); err != nil { return err } return nil diff --git a/cmd/notify-redis.go b/cmd/notify-redis.go index 249eb446d..328ab1d15 100644 --- a/cmd/notify-redis.go +++ b/cmd/notify-redis.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net" "time" "github.com/Sirupsen/logrus" @@ -52,7 +53,7 @@ func (r *redisNotify) Validate() error { if r.Format != formatNamespace && r.Format != formatAccess { return rdNFormatError } - if _, err := checkURL(r.Addr); err != nil { + if _, _, err := net.SplitHostPort(r.Addr); err != nil { return err } if r.Key == "" { @@ -73,6 +74,7 @@ func dialRedis(rNotify redisNotify) (*redis.Pool, error) { if !rNotify.Enable { return nil, errNotifyNotEnabled } + addr := rNotify.Addr password := rNotify.Password rPool := &redis.Pool{ diff --git a/cmd/utils.go b/cmd/utils.go index a252c1c80..5de27f5a0 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -266,13 +266,13 @@ func isFile(path string) bool { } // checkURL - checks if passed address correspond -func checkURL(address string) (*url.URL, error) { - if address == "" { +func checkURL(urlStr string) (*url.URL, error) { + if urlStr == "" { return nil, errors.New("Address cannot be empty") } - u, err := url.Parse(address) + u, err := url.Parse(urlStr) if err != nil { - return nil, fmt.Errorf("`%s` invalid: %s", address, err.Error()) + return nil, fmt.Errorf("`%s` invalid: %s", urlStr, err.Error()) } return u, nil } diff --git a/cmd/utils_test.go b/cmd/utils_test.go index fa06e8e09..f8a241fc2 100644 --- a/cmd/utils_test.go +++ b/cmd/utils_test.go @@ -389,16 +389,14 @@ func TestLocalAddress(t *testing.T) { } -// TestCheckURL tests valid address +// TestCheckURL tests valid url. func TestCheckURL(t *testing.T) { testCases := []struct { - addr string + urlStr string shouldPass bool }{ {"", false}, {":", false}, - {"localhost", true}, - {"127.0.0.1", true}, {"http://localhost/", true}, {"http://127.0.0.1/", true}, {"proto://myhostname/path", true}, @@ -406,7 +404,7 @@ func TestCheckURL(t *testing.T) { // Validates fetching local address. for i, testCase := range testCases { - _, err := checkURL(testCase.addr) + _, err := checkURL(testCase.urlStr) if testCase.shouldPass && err != nil { t.Errorf("Test %d: expected to pass but got an error: %v\n", i+1, err) }