cmd/trace: rewrite unspecified address to localhost in URL

The previous CL makes -http=:6060 listen on localhost instead of all
addresses, so this case will print http://127.0.0.1:6060.

However, when user explicitly pass the unspecified address (e.g.,
-http=0.0.0.0:6060), we will print http://0.0.0.0:6060.

Clicking this link to connect unspecified address doesn't make much
sense. On Linux, the kernel happens to treat connect to the unspecified
address as a connect to loopback, so this works fine. On the other hand,
Windows treats connect to the unspecified address as an explicit error,
so this URL doesn't work.

Instead, rewrite the unspecified address to localhost when printing the
URL to ensure it works on all OSes. To avoid hiding that we are
listening all addresses, we now print the full listen address in
addition to the potentially rewritten URL.

Fixes #78921.

Change-Id: If99d5126c13059aa58c870dcc52476716a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/770462
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>

src/cmd/trace/main.go

@@ -19,6 +19,7 @@ import (
 	"net"
 	"net/http"
 	_ "net/http/pprof" // Required to use pprof
+	"net/netip"
 	"os"
 	"slices"
 	"sync/atomic"
@@ -157,7 +158,12 @@ func main() {
 	if err != nil {
 		logAndDie(fmt.Errorf("failed to create server socket: %w", err))
 	}
-	url := "http://" + ln.Addr().String()
+
+	addr = ln.Addr().String()
+	url, simplified, err := addrURL(addr)
+	if err != nil {
+		logAndDie(fmt.Errorf("failed to compute server URL: %v", err))
+	}
 
 	log.Print("Preparing trace for viewer...")
 	parsed, err := parseTraceInteractive(tracef, traceSize)
@@ -183,6 +189,13 @@ func main() {
 		logAndDie(err)
 	}
 
+	if simplified {
+		// Warn that the URL below is simplified. i.e., we are actually
+		// listening on more addresses than the URL implies.
+		log.Printf("Full server listen address: %s", addr)
+	}
+	// N.B. gopls depends on the format of this log message. See
+	// golang.org/x/tools/gopls/internal/debug.startFlightRecorder.
 	log.Printf("Opening browser. Trace viewer is listening on %s", url)
 	browser.Open(addr)
 
@@ -261,6 +274,47 @@ func listenAddr(addr string) (string, error) {
 	return net.JoinHostPort(host, port), nil
 }
 
+// addrURL returns an HTTP URL that may be used to connect to addr.
+//
+// It also returns a bool indicating if the returned URL uses a rewritten address.
+func addrURL(addr string) (string, bool, error) {
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return "", false, err
+	}
+
+	if host == "" {
+		// No host implies unspecified address, so rewrite to
+		// localhost, as below.
+		//
+		// addr should come from a net.Listener and thus always include
+		// a host, but handle this just in case.
+		host = "localhost"
+		return "http://" + net.JoinHostPort(host, port), true, nil
+	}
+
+	ipaddr, err := netip.ParseAddr(host)
+	if err != nil {
+		// Not an IP address, no change required.
+		return "http://" + net.JoinHostPort(host, port), false, nil
+	}
+
+	if ipaddr.IsUnspecified() {
+		// An unspecified address means (e.g., 0.0.0.0) this addr is
+		// listening on all addresses. It doesn't make sense to connect
+		// to the unspecified address [1], so rewrite to localhost. A
+		// connection to localhost with route to the same place.
+		//
+		// [1] Linux happens to treat connect to the unspecified
+		// address as loopback, but other OSes, such as Windows, treat
+		// it as an error.
+		host = "localhost"
+		return "http://" + net.JoinHostPort(host, port), true, nil
+	}
+
+	return "http://" + net.JoinHostPort(host, port), false, nil
+}
+
 func parseTraceInteractive(tr io.Reader, size int64) (parsed *parsedTrace, err error) {
 	done := make(chan struct{})
 	cr := countingReader{r: tr}

src/cmd/trace/main_test.go

@@ -58,3 +58,58 @@ func TestListenAddr(t *testing.T) {
 		})
 	}
 }
+
+func TestAddrURL(t *testing.T) {
+	tests := []struct {
+		name           string
+		addr           string
+		wantURL        string
+		wantSimplified bool
+	}{
+		{
+			name:           "empty host",
+			addr:           ":8080",
+			wantURL:        "http://localhost:8080",
+			wantSimplified: true,
+		},
+		{
+			name:           "with host",
+			addr:           "localhost:8080",
+			wantURL:        "http://localhost:8080",
+			wantSimplified: false,
+		},
+		{
+			name:           "with ip",
+			addr:           "10.10.10.10:8080",
+			wantURL:        "http://10.10.10.10:8080",
+			wantSimplified: false,
+		},
+		{
+			name:           "unspecified ipv4",
+			addr:           "0.0.0.0:8080",
+			wantURL:        "http://localhost:8080",
+			wantSimplified: true,
+		},
+		{
+			name:           "unspecified ipv6",
+			addr:           "[::]:8080",
+			wantURL:        "http://localhost:8080",
+			wantSimplified: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotURL, gotSimplified, err := addrURL(tt.addr)
+			if err != nil {
+				t.Fatalf("addrURL(%q) got err %v want nil", tt.addr, err)
+			}
+			if gotURL != tt.wantURL {
+				t.Errorf("addrURL(%q) = %q, want %q", tt.addr, gotURL, tt.wantURL)
+			}
+			if gotSimplified != tt.wantSimplified {
+				t.Errorf("addrURL(%q) simplified = %v, want %v", tt.addr, gotSimplified, tt.wantSimplified)
+			}
+		})
+	}
+}