heartbeat: setup default ports in http monitors (elastic#3924)

* heartbeat: setup default ports in http monitors

Default http to 80 and https to 443

Resolves elastic#3915

* Add entry to changelog

* Add test

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

CHANGELOG.asciidoc

@@ -58,6 +58,7 @@ https://github.com/elastic/beats/compare/v5.1.1...master[Check the HEAD diff]
 - Downgrade Elasticsearch per batch item failure log to debug level. {issue}3953[3953]
 
 *Heartbeat*
+- Add default ports in HTTP monitor. {pull}3924[3924]
 
 *Metricbeat*
 

heartbeat/monitors/active/http/simple_transp.go

@@ -14,6 +14,13 @@ import (
 	"github.com/elastic/beats/libbeat/outputs/transport"
 )
 
+const (
+	gzipEncoding   = "gzip"
+	urlSchemaHTTP  = "http"
+	urlSchemaHTTPS = "https"
+)
+
+// SimpleTransport contains the dialer and read/write callbacks
 type SimpleTransport struct {
 	Dialer             transport.Dialer
 	DisableCompression bool
@@ -32,7 +39,7 @@ func (t *SimpleTransport) checkRequest(req *http.Request) error {
 	}
 
 	scheme := req.URL.Scheme
-	isHTTP := scheme == "http" || scheme == "https"
+	isHTTP := scheme == urlSchemaHTTP || scheme == urlSchemaHTTPS
 	if !isHTTP {
 		return fmt.Errorf("http: unsupported scheme %v", scheme)
 	}
@@ -43,6 +50,7 @@ func (t *SimpleTransport) checkRequest(req *http.Request) error {
 	return nil
 }
 
+// RoundTrip sets up goroutines to write the request and read the responses
 func (t *SimpleTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	type readReturn struct {
 		resp *http.Response
@@ -68,7 +76,7 @@ func (t *SimpleTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 		req.Method != "HEAD" {
 
 		requestedGzip = true
-		req.Header.Add("Accept-Encoding", "gzip")
+		req.Header.Add("Accept-Encoding", gzipEncoding)
 		defer req.Header.Del("Accept-Encoding")
 	}
 
@@ -132,7 +140,7 @@ func (t *SimpleTransport) readResponse(
 	}
 	t.sigStartRead()
 
-	if requestedGzip && resp.Header.Get("Content-Encoding") == "gzip" {
+	if requestedGzip && resp.Header.Get("Content-Encoding") == gzipEncoding {
 		resp.Header.Del("Content-Encoding")
 		resp.Header.Del("Content-Length")
 		resp.ContentLength = -1

heartbeat/monitors/active/http/task.go

@@ -213,16 +213,24 @@ func execPing(
 }
 
 func splitHostnamePort(requ *http.Request) (string, uint16, error) {
-	host, port, err := net.SplitHostPort(requ.URL.Host)
+	host := requ.URL.Host
+	// Try to add a default port if needed
+	if strings.LastIndex(host, ":") == -1 {
+		switch requ.URL.Scheme {
+		case urlSchemaHTTP:
+			host += ":80"
+		case urlSchemaHTTPS:
+			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)
+		}
+	}
+}