Add test

Also refactor the code, turns out net.SplitHostPort() doesn't like hosts
without ports.

heartbeat/monitors/active/http/task.go

@@ -213,25 +213,24 @@ func execPing(
 }
 
 func splitHostnamePort(requ *http.Request) (string, uint16, error) {
-	host, port, err := net.SplitHostPort(requ.URL.Host)
-	if err != nil {
-		return "", 0, err
-	}
-	var p uint64
-	if port == "" {
+	host := requ.URL.Host
+	// Try to add a default port if needed
+	if strings.LastIndex(host, ":") == -1 {
 		switch requ.URL.Scheme {
 		case urlSchemaHTTP:
-			p = 80
+			host += ":80"
 		case urlSchemaHTTPS:
-			p = 443
-		}
-	} else {
-		p, err = strconv.ParseUint(port, 10, 16)
-		if err != nil {
-			return "", 0, fmt.Errorf("'%v' is no valid port number in '%v'", port, requ.URL.Host)
+			host += ":443"
 		}
 	}
-
+	host, port, err := net.SplitHostPort(host)
+	if err != nil {
+		return "", 0, err
+	}
+	p, err := strconv.ParseUint(port, 10, 16)
+	if err != nil {
+		return "", 0, fmt.Errorf("'%v' is no valid port number in '%v'", port, requ.URL.Host)
+	}
 	return host, uint16(p), nil
 }
 

heartbeat/monitors/active/http/task_test.go

@@ -0,0 +1,93 @@
+package http
+
+import (
+	"net"
+	"net/http"
+	"net/url"
+	"reflect"
+	"testing"
+)
+
+func TestSplitHostnamePort(t *testing.T) {
+	var urlTests = []struct {
+		scheme        string
+		host          string
+		expectedHost  string
+		expectedPort  uint16
+		expectedError error
+	}{
+		{
+			"http",
+			"foo",
+			"foo",
+			80,
+			nil,
+		},
+		{
+			"http",
+			"www.foo.com",
+			"www.foo.com",
+			80,
+			nil,
+		},
+		{
+			"http",
+			"www.foo.com:8080",
+			"www.foo.com",
+			8080,
+			nil,
+		},
+		{
+			"https",
+			"foo",
+			"foo",
+			443,
+			nil,
+		},
+		{
+			"http",
+			"foo:81",
+			"foo",
+			81,
+			nil,
+		},
+		{
+			"https",
+			"foo:444",
+			"foo",
+			444,
+			nil,
+		},
+		{
+			"httpz",
+			"foo",
+			"foo",
+			81,
+			&net.AddrError{},
+		},
+	}
+	for _, test := range urlTests {
+		url := &url.URL{
+			Scheme: test.scheme,
+			Host:   test.host,
+		}
+		request := &http.Request{
+			URL: url,
+		}
+		host, port, err := splitHostnamePort(request)
+		if err != nil {
+			if test.expectedError == nil {
+				t.Error(err)
+			} else if reflect.TypeOf(err) != reflect.TypeOf(test.expectedError) {
+				t.Errorf("Expected %T but got %T", err, test.expectedError)
+			}
+			continue
+		}
+		if host != test.expectedHost {
+			t.Errorf("Unexpected host for %#v: expected %q, got %q", request, test.expectedHost, host)
+		}
+		if port != test.expectedPort {
+			t.Errorf("Unexpected port for %#v: expected %q, got %q", request, test.expectedPort, port)
+		}
+	}
+}