tsnet: base HTTPClient's transport on http.DefaultTransport

Previously Server.HTTPClient returned a client whose transport set
only DialContext, losing http.DefaultTransport's defaults such as
ForceAttemptHTTP2, MaxIdleConns, IdleConnTimeout, TLSHandshakeTimeout,
and ExpectContinueTimeout. Now it clones http.DefaultTransport and
overrides DialContext to dial over the tailnet, so it picks up those
settings and any future defaults.

Proxy is explicitly nil: an HTTP proxy from the environment would be
dialed through the tailnet, where it's unlikely to be reachable.

The new test compares the transport field by field against a clone of
http.DefaultTransport and fails on any unknown future field, forcing a
decision about how HTTPClient should handle it.

Updates tailscale/corp#47393 (as motivation)

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I1e800982ee11d386699dd4f588c1fd2ce392f229

tsnet/tsnet.go

@@ -413,12 +413,16 @@ func (s *Server) awaitRunning(ctx context.Context) error {
 //
 // This is useful if you need to have your tsnet services connect to other devices on
 // your tailnet.
+//
+// The returned client's transport has the same settings as
+// [http.DefaultTransport], except its DialContext dials over Tailscale
+// and it has no Proxy set, as HTTP proxies from the environment are
+// unlikely to be reachable over the tailnet.
 func (s *Server) HTTPClient() *http.Client {
-	return &http.Client{
-		Transport: &http.Transport{
-			DialContext: s.Dial,
-		},
-	}
+	tr := http.DefaultTransport.(*http.Transport).Clone()
+	tr.DialContext = s.Dial
+	tr.Proxy = nil
+	return &http.Client{Transport: tr}
 }
 
 // LocalClient returns a LocalClient that speaks to s.

tsnet/tsnet_test.go

@@ -4046,3 +4046,55 @@ func TestCloseBeforeStart(t *testing.T) {
 		t.Fatalf("Close: %v", err)
 	}
 }
+
+// TestHTTPClientDefaultTransport verifies that the transport returned by
+// HTTPClient matches http.DefaultTransport's settings, except for the
+// fields that HTTPClient intentionally overrides.
+func TestHTTPClientDefaultTransport(t *testing.T) {
+	s := &Server{}
+	tr, ok := s.HTTPClient().Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("Transport is %T; want *http.Transport", s.HTTPClient().Transport)
+	}
+	if tr.DialContext == nil {
+		t.Error("DialContext is nil; want it set to Server.Dial")
+	}
+	if tr.Proxy != nil {
+		t.Error("Proxy is non-nil; want nil, as environment proxies are unreachable over the tailnet")
+	}
+
+	want := http.DefaultTransport.(*http.Transport).Clone()
+	gotv := reflect.ValueOf(tr).Elem()
+	wantv := reflect.ValueOf(want).Elem()
+	for i := range gotv.NumField() {
+		f := gotv.Type().Field(i)
+		if !f.IsExported() {
+			continue
+		}
+		switch f.Name {
+		case "DialContext", "Proxy":
+			// Intentionally different; checked above.
+		case "TLSNextProto":
+			// A map containing funcs, which reflect.DeepEqual can't
+			// compare, so just check that the sizes match.
+			if got, want := gotv.Field(i).Len(), wantv.Field(i).Len(); got != want {
+				t.Errorf("TLSNextProto has %d entries; want %d", got, want)
+			}
+		case "OnProxyConnectResponse", "Dial", "DialTLSContext", "DialTLS",
+			"TLSClientConfig", "TLSHandshakeTimeout",
+			"DisableKeepAlives", "DisableCompression",
+			"MaxIdleConns", "MaxIdleConnsPerHost", "MaxConnsPerHost",
+			"IdleConnTimeout", "ResponseHeaderTimeout", "ExpectContinueTimeout",
+			"ProxyConnectHeader", "GetProxyConnectHeader",
+			"MaxResponseHeaderBytes", "WriteBufferSize", "ReadBufferSize",
+			"ForceAttemptHTTP2", "HTTP2", "Protocols":
+			// Expected to be equal from Clone.
+			g, w := gotv.Field(i).Interface(), wantv.Field(i).Interface()
+			if !reflect.DeepEqual(g, w) {
+				t.Errorf("field %s = %v; want %v (as in http.DefaultTransport)", f.Name, g, w)
+			}
+		default:
+			t.Errorf("unexpected http.Transport field %q; decide how HTTPClient should handle it and update this test", f.Name)
+		}
+	}
+}