Be more strict in accepted client URLs

Go 1.8 does more strict URL checks. E.g. Go 1.7 accepted to parse the
URL `"127.0.0.1:9200"` whereas Go 1.8 will raise an error: "first path
segment in URL cannot contain colon".

We use that as a chance to also be more strict about what is considered
a valid URL. Until now, we accepted strings like `"127.0.0.1"` or
`"127.0.0.1:9200"`. Those are invalid URLs in the first place, and we
now treat them as such.

Close #429

canonicalize.go

@@ -8,21 +8,17 @@ import "net/url"
 
 // canonicalize takes a list of URLs and returns its canonicalized form, i.e.
 // remove anything but scheme, userinfo, host, path, and port.
-// It also removes all trailing slashes. It also skips invalid URLs or
-// URLs that do not use protocol http or https.
+// It also removes all trailing slashes. Invalid URLs or URLs that do not
+// use protocol http or https are skipped.
 //
 // Example:
 // http://127.0.0.1:9200/?query=1 -> http://127.0.0.1:9200
 // http://127.0.0.1:9200/db1/ -> http://127.0.0.1:9200/db1
-// 127.0.0.1:9200 -> http://127.0.0.1:9200
 func canonicalize(rawurls ...string) []string {
 	var canonicalized []string
 	for _, rawurl := range rawurls {
 		u, err := url.Parse(rawurl)
 		if err == nil {
-			if len(u.Scheme) == 0 {
-				u.Scheme = DefaultScheme
-			}
 			if u.Scheme == "http" || u.Scheme == "https" {
 				// Trim trailing slashes
 				for len(u.Path) > 0 && u.Path[len(u.Path)-1] == '/' {

canonicalize_test.go

@@ -4,58 +4,69 @@
 
 package elastic
 
-import (
-	"reflect"
-	"testing"
-)
+import "testing"
 
 func TestCanonicalize(t *testing.T) {
 	tests := []struct {
 		Input  []string
 		Output []string
 	}{
+		// #0
 		{
 			Input:  []string{"http://127.0.0.1/"},
 			Output: []string{"http://127.0.0.1"},
 		},
+		// #1
 		{
 			Input:  []string{"http://127.0.0.1:9200/", "gopher://golang.org/", "http://127.0.0.1:9201"},
 			Output: []string{"http://127.0.0.1:9200", "http://127.0.0.1:9201"},
 		},
+		// #2
 		{
 			Input:  []string{"http://user:secret@127.0.0.1/path?query=1#fragment"},
 			Output: []string{"http://user:secret@127.0.0.1/path"},
 		},
+		// #3
 		{
 			Input:  []string{"https://somewhere.on.mars:9999/path?query=1#fragment"},
 			Output: []string{"https://somewhere.on.mars:9999/path"},
 		},
+		// #4
 		{
 			Input:  []string{"https://prod1:9999/one?query=1#fragment", "https://prod2:9998/two?query=1#fragment"},
 			Output: []string{"https://prod1:9999/one", "https://prod2:9998/two"},
 		},
+		// #5
 		{
 			Input:  []string{"http://127.0.0.1/one/"},
 			Output: []string{"http://127.0.0.1/one"},
 		},
+		// #6
 		{
 			Input:  []string{"http://127.0.0.1/one///"},
 			Output: []string{"http://127.0.0.1/one"},
 		},
+		// #7: Invalid URL
 		{
 			Input:  []string{"127.0.0.1/"},
-			Output: []string{"http://127.0.0.1"},
+			Output: []string{},
 		},
+		// #8: Invalid URL
 		{
 			Input:  []string{"127.0.0.1:9200"},
-			Output: []string{"http://127.0.0.1:9200"},
+			Output: []string{},
 		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
 		got := canonicalize(test.Input...)
-		if !reflect.DeepEqual(got, test.Output) {
-			t.Errorf("expected %v; got: %v", test.Output, got)
+		if want, have := len(test.Output), len(got); want != have {
+			t.Fatalf("#%d: expected %d elements; got: %d", i, want, have)
+		}
+		for i := 0; i < len(got); i++ {
+			if want, have := test.Output[i], got[i]; want != have {
+				t.Errorf("#%d: expected %q; got: %q", i, want, have)
+			}
 		}
 	}
 }