Better handling of HTTP redirects.

prober/http.go

@@ -28,6 +28,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/go-kit/kit/log"
@@ -145,38 +146,63 @@ type roundTripTrace struct {
 
 // transport is a custom transport keeping traces for each HTTP roundtrip.
 type transport struct {
-	Transport http.RoundTripper
-	logger    log.Logger
-	traces    []*roundTripTrace
-	current   *roundTripTrace
+	Transport             http.RoundTripper
+	NoServerNameTransport http.RoundTripper
+	firstHost             string
+	logger                log.Logger
+
+	mu      sync.Mutex
+	traces  []*roundTripTrace
+	current *roundTripTrace
 }
 
-func newTransport(rt http.RoundTripper, logger log.Logger) *transport {
+func newTransport(rt, noServerName http.RoundTripper, logger log.Logger) *transport {
 	return &transport{
-		Transport: rt,
-		logger:    logger,
-		traces:    []*roundTripTrace{},
+		Transport:             rt,
+		NoServerNameTransport: noServerName,
+		logger:                logger,
+		traces:                []*roundTripTrace{},
 	}
 }
 
 // RoundTrip switches to a new trace, then runs embedded RoundTripper.
 func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
+	level.Info(t.logger).Log("msg", "Making HTTP request", "url", req.URL.String(), "host", req.Host)
+
 	trace := &roundTripTrace{}
 	if req.URL.Scheme == "https" {
 		trace.tls = true
 	}
 	t.current = trace
 	t.traces = append(t.traces, trace)
+
+	if t.firstHost == "" {
+		t.firstHost = req.URL.Host
+	}
+
+	if t.firstHost != req.URL.Host {
+		// This is a redirect to something other than the initial host,
+		// so TLS ServerName should not be set.
+		level.Info(t.logger).Log("msg", "Address does not match first address, not sending TLS ServerName", "first", t.firstHost, "address", req.URL.Host)
+		return t.NoServerNameTransport.RoundTrip(req)
+	}
+
 	return t.Transport.RoundTrip(req)
 }
 
 func (t *transport) DNSStart(_ httptrace.DNSStartInfo) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.current.start = time.Now()
 }
 func (t *transport) DNSDone(_ httptrace.DNSDoneInfo) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.current.dnsDone = time.Now()
 }
 func (ts *transport) ConnectStart(_, _ string) {
+	ts.mu.Lock()
+	defer ts.mu.Unlock()
 	t := ts.current
 	// No DNS resolution because we connected to IP directly.
 	if t.dnsDone.IsZero() {
@@ -185,12 +211,18 @@ func (ts *transport) ConnectStart(_, _ string) {
 	}
 }
 func (t *transport) ConnectDone(net, addr string, err error) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.current.connectDone = time.Now()
 }
 func (t *transport) GotConn(_ httptrace.GotConnInfo) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.current.gotConn = time.Now()
 }
 func (t *transport) GotFirstResponseByte() {
+	t.mu.Lock()
+	defer t.mu.Unlock()
 	t.current.responseStart = time.Now()
 }
 
@@ -290,19 +322,27 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr
 		return false
 	}
 
+	httpClientConfig.TLSConfig.ServerName = ""
+	noServerName, err := pconfig.NewRoundTripperFromConfig(httpClientConfig, "http_probe", true)
+	if err != nil {
+		level.Error(logger).Log("msg", "Error generating HTTP client without ServerName", "err", err)
+		return false
+	}
+
 	jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
 	if err != nil {
 		level.Error(logger).Log("msg", "Error generating cookiejar", "err", err)
 		return false
 	}
 	client.Jar = jar
 
-	// Inject transport that tracks trace for each redirect.
-	tt := newTransport(client.Transport, logger)
+	// Inject transport that tracks traces for each redirect,
+	// and does not set TLS ServerNames on redirect if needed.
+	tt := newTransport(client.Transport, noServerName, logger)
 	client.Transport = tt
 
 	client.CheckRedirect = func(r *http.Request, via []*http.Request) error {
-		level.Info(logger).Log("msg", "Received redirect", "url", r.URL.String())
+		level.Info(logger).Log("msg", "Received redirect", "location", r.Response.Header.Get("Location"))
 		redirects = len(via)
 		if redirects > 10 || httpConfig.NoFollowRedirects {
 			level.Info(logger).Log("msg", "Not following redirect")
@@ -350,8 +390,6 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr
 		request.Header.Set(key, value)
 	}
 
-	level.Info(logger).Log("msg", "Making HTTP request", "url", request.URL.String(), "host", request.Host)
-
 	trace := &httptrace.ClientTrace{
 		DNSStart:             tt.DNSStart,
 		DNSDone:              tt.DNSDone,
@@ -450,6 +488,8 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr
 	if resp == nil {
 		resp = &http.Response{}
 	}
+	tt.mu.Lock()
+	defer tt.mu.Unlock()
 	for i, trace := range tt.traces {
 		level.Info(logger).Log(
 			"msg", "Response timings for roundtrip",

prober/http_test.go

@@ -649,6 +649,27 @@ func TestHTTPUsesTargetAsTLSServerName(t *testing.T) {
 	}
 }
 
+func TestRedirectToTLSHostWorks(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping network dependant test")
+	}
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "https://prometheus.io", http.StatusFound)
+	}))
+	defer ts.Close()
+
+	// Follow redirect, should succeed with 200.
+	registry := prometheus.NewRegistry()
+	testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	result := ProbeHTTP(testCTX, ts.URL,
+		config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true}}, registry, log.NewNopLogger())
+	if !result {
+		t.Fatalf("Redirect test failed unexpectedly")
+	}
+
+}
+
 func TestHTTPPhases(t *testing.T) {
 	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 	}))