From a8fc58214a727a4e560d2325e5a76f188e125609 Mon Sep 17 00:00:00 2001 From: Manu Garg Date: Mon, 17 Feb 2020 22:27:57 -0800 Subject: [PATCH] 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: https://github.com/google/cloudprober/issues/365 PiperOrigin-RevId: 295666337 --- probes/http/http.go | 2 +- probes/http/request.go | 39 ++++++++++----- probes/http/request_test.go | 94 ++++++++++++++++++++++++++++++------- 3 files changed, 106 insertions(+), 29 deletions(-) diff --git a/probes/http/http.go b/probes/http/http.go index 92f4a6b0..a4547921 100644 --- a/probes/http/http.go +++ b/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 } diff --git a/probes/http/request.go b/probes/http/request.go index 85bd0af4..569e9f83 100644 --- a/probes/http/request.go +++ b/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" { diff --git a/probes/http/request_test.go b/probes/http/request_test.go index 9822cb61..91d90b7b 100644 --- a/probes/http/request_test.go +++ b/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") + }) }