Make sure to include the port in the Host header.

HTTP spec says that Host header should include port if port is not the default port. Host header's port matters mostly when server redirects the request to another relative URL -- in that case, Go HTTP client uses Host header's value for connection.

Also, add some tests for req.Host behavior.

Fixes: #365
PiperOrigin-RevId: 295666337

probes/http/http.go

@@ -281,7 +281,7 @@ func (p *Probe) updateTargets() {
 
 	for _, target := range p.targets {
 		// Update HTTP request
-		req := p.httpRequestForTarget(target)
+		req := p.httpRequestForTarget(target, nil)
 		if req != nil {
 			p.httpRequests[target.Name] = req
 		}

probes/http/request.go

@@ -17,6 +17,7 @@ package http
 import (
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 
 	"github.com/google/cloudprober/targets/endpoint"
@@ -37,26 +38,40 @@ func (rb *requestBody) Read(p []byte) (int, error) {
 	return copy(p, rb.b), io.EOF
 }
 
-func (p *Probe) httpRequestForTarget(target endpoint.Endpoint) *http.Request {
+// resolveFunc resolves the given host for the IP version.
+// This type is mainly used for testing. For all other cases, a nil function
+// should be passed to the httpRequestForTarget function.
+type resolveFunc func(host string, ipVer int) (net.IP, error)
+
+func (p *Probe) httpRequestForTarget(target endpoint.Endpoint, resolveF resolveFunc) *http.Request {
 	// Prepare HTTP.Request for Client.Do
-	host := target.Name
+	port := int(p.c.GetPort())
+	// If port is not configured explicitly, use target's port if available.
+	if port == 0 {
+		port = target.Port
+	}
+
+	urlHost, hostHeader := target.Name, target.Name
 
 	if p.c.GetResolveFirst() {
-		ip, err := p.opts.Targets.Resolve(target.Name, p.opts.IPVersion)
+		if resolveF == nil {
+			resolveF = p.opts.Targets.Resolve
+		}
+
+		ip, err := resolveF(target.Name, p.opts.IPVersion)
 		if err != nil {
 			p.l.Error("target: ", target.Name, ", resolve error: ", err.Error())
 			return nil
 		}
-		host = ip.String()
+		urlHost = ip.String()
 	}
 
-	if p.c.GetPort() != 0 {
-		host = fmt.Sprintf("%s:%d", host, p.c.GetPort())
-	} else if target.Port != 0 {
-		host = fmt.Sprintf("%s:%d", host, target.Port)
+	if port != 0 {
+		urlHost = fmt.Sprintf("%s:%d", urlHost, port)
+		hostHeader = fmt.Sprintf("%s:%d", hostHeader, port)
 	}
 
-	url := fmt.Sprintf("%s://%s%s", p.protocol, host, p.url)
+	url := fmt.Sprintf("%s://%s%s", p.protocol, urlHost, p.url)
 
 	// Prepare request body
 	var body io.Reader
@@ -70,9 +85,9 @@ func (p *Probe) httpRequestForTarget(target endpoint.Endpoint) *http.Request {
 	}
 
 	// If resolving early, URL contains IP for the hostname (see above). Update
-	// req.Host after request creation, so that correct Host header is sent to the
-	// web server.
-	req.Host = target.Name
+	// req.Host after request creation, so that correct Host header is sent to
+	// the web server.
+	req.Host = hostHeader
 
 	for _, header := range p.c.GetHeaders() {
 		if header.GetName() == "Host" {

probes/http/request_test.go

@@ -15,57 +15,119 @@
 package http
 
 import (
+	"fmt"
+	"net"
 	"testing"
 
 	"github.com/golang/protobuf/proto"
 	configpb "github.com/google/cloudprober/probes/http/proto"
+	"github.com/google/cloudprober/probes/options"
+	"github.com/google/cloudprober/targets"
 	"github.com/google/cloudprober/targets/endpoint"
 )
 
-func newProbe(port int) *Probe {
+func newProbe(port int, resolveFirst bool, target, hostHeader string) *Probe {
 	p := &Probe{
 		protocol: "http",
+		c: &configpb.ProbeConf{
+			ResolveFirst: proto.Bool(resolveFirst),
+		},
+		opts: &options.Options{Targets: targets.StaticTargets(target)},
 	}
 
 	if port != 0 {
-		p.c = &configpb.ProbeConf{
-			Port: proto.Int(port),
-		}
+		p.c.Port = proto.Int(port)
+	}
+
+	if hostHeader != "" {
+		p.c.Headers = append(p.c.Headers, &configpb.ProbeConf_Header{
+			Name:  proto.String("Host"),
+			Value: proto.String(hostHeader),
+		})
 	}
 
 	return p
 }
 
-func testReq(t *testing.T, p *Probe, ep endpoint.Endpoint, wantURL, wantHost string) {
+func hostWithPort(host string, port int) string {
+	if port == 0 {
+		return host
+	}
+	return fmt.Sprintf("%s:%d", host, port)
+}
+
+func testReq(t *testing.T, p *Probe, ep endpoint.Endpoint, wantURLHost, hostHeader string, port int, resolveF resolveFunc) {
 	t.Helper()
 
-	req := p.httpRequestForTarget(ep)
+	wantURL := fmt.Sprintf("http://%s", hostWithPort(wantURLHost, port))
+	wantHost := hostHeader
+	if wantHost == "" {
+		wantHost = hostWithPort(ep.Name, port)
+	}
+
+	req := p.httpRequestForTarget(ep, resolveF)
 	if req.URL.String() != wantURL {
 		t.Errorf("HTTP req URL: %s, wanted: %s", req.URL.String(), wantURL)
 	}
+
+	if req.Host != wantHost {
+		t.Errorf("HTTP req.Host: %s, wanted: %s", req.Host, wantHost)
+	}
 }
 
-func TestHTTPRequestForTarget(t *testing.T) {
+func testRequestHostAndURL(t *testing.T, resolveFirst bool, target, urlHost, hostHeader string) {
+	t.Helper()
+
+	var resolveF resolveFunc
+	if resolveFirst {
+		resolveF = func(target string, ipVer int) (net.IP, error) {
+			return net.ParseIP(urlHost), nil
+		}
+	}
+
 	// Probe has no port
-	p := newProbe(0)
+	p := newProbe(0, resolveFirst, target, hostHeader)
+	expectedPort := 0
+
+	// If hostHeader is set, change probe config and expectedHost.
 	ep := endpoint.Endpoint{
-		Name: "test-target.com",
+		Name: target,
 	}
-	testReq(t, p, ep, "http://test-target.com", ep.Name)
+	testReq(t, p, ep, urlHost, hostHeader, expectedPort, resolveF)
 
 	// Probe and endpoint have port, probe's port wins.
-	p = newProbe(8080)
+	p = newProbe(8080, resolveFirst, target, hostHeader)
 	ep = endpoint.Endpoint{
-		Name: "test-target.com",
+		Name: target,
 		Port: 9313,
 	}
-	testReq(t, p, ep, "http://test-target.com:8080", ep.Name)
+	expectedPort = 8080
+	testReq(t, p, ep, urlHost, hostHeader, expectedPort, resolveF)
 
 	// Only endpoint has port, endpoint's port is used.
-	p = newProbe(0)
+	p = newProbe(0, resolveFirst, target, hostHeader)
 	ep = endpoint.Endpoint{
-		Name: "test-target.com",
+		Name: target,
 		Port: 9313,
 	}
-	testReq(t, p, ep, "http://test-target.com:9313", ep.Name)
+	expectedPort = 9313
+	testReq(t, p, ep, urlHost, hostHeader, expectedPort, resolveF)
+}
+
+func TestRequestHostAndURL(t *testing.T) {
+	t.Run("no_resolve_first,no_host_header", func(t *testing.T) {
+		testRequestHostAndURL(t, false, "test-target.com", "test-target.com", "")
+	})
+
+	t.Run("no_resolve_first,host_header", func(t *testing.T) {
+		testRequestHostAndURL(t, false, "test-target.com", "test-target.com", "test-host")
+	})
+
+	t.Run("resolve_first,no_host_header", func(t *testing.T) {
+		testRequestHostAndURL(t, true, "localhost", "127.0.0.1", "")
+	})
+
+	t.Run("resolve_first,host_header", func(t *testing.T) {
+		testRequestHostAndURL(t, true, "localhost", "127.0.0.1", "test-host")
+	})
 }