From 6ee956333fda82b17f6dddbe5eb7388ae9b43b70 Mon Sep 17 00:00:00 2001
From: Irbe Krumina <irbe@tailscale.com>
Date: Tue, 23 Jan 2024 18:12:56 +0000
Subject: [PATCH] ipn/ipnlocal: fix proxy path that matches mount point
 (#10864)

Don't append a trailing slash to a request path
to the reverse proxy that matches the mount point exactly.

Updates tailscale/tailscale#10730

Signed-off-by: Irbe Krumina <irbe@tailscale.com>
---
 ipn/ipnlocal/serve.go      |  13 +++++
 ipn/ipnlocal/serve_test.go | 103 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go
index 02df4eb1f..3d7f32bff 100644
--- a/ipn/ipnlocal/serve.go
+++ b/ipn/ipnlocal/serve.go
@@ -605,7 +605,20 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	p := &httputil.ReverseProxy{Rewrite: func(r *httputil.ProxyRequest) {
+		oldOutPath := r.Out.URL.Path
 		r.SetURL(rp.url)
+
+		// If mount point matches the request path exactly, the outbound
+		// request URL was set to empty string in serveWebHandler which
+		// would have resulted in the outbound path set to <proxy path>
+		// + '/' in SetURL. In that case, if the proxy path was set, we
+		// want to send the request to the <proxy path> (without the
+		// '/') .
+		if oldOutPath == "" && rp.url.Path != "" {
+			r.Out.URL.Path = rp.url.Path
+			r.Out.URL.RawPath = rp.url.RawPath
+		}
+
 		r.Out.Host = r.In.Host
 		addProxyForwardedHeaders(r)
 		rp.lb.addTailscaleIdentityHeaders(r)
diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go
index 4899e1d4c..604cf0b3a 100644
--- a/ipn/ipnlocal/serve_test.go
+++ b/ipn/ipnlocal/serve_test.go
@@ -348,7 +348,108 @@ func TestServeConfigETag(t *testing.T) {
 	}
 }
 
-func TestServeHTTPProxy(t *testing.T) {
+func TestServeHTTPProxyPath(t *testing.T) {
+	b := newTestBackend(t)
+	// Start test serve endpoint.
+	testServ := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			// Set the request URL path to a response header, so the
+			// requested URL path can be checked in tests.
+			t.Logf("adding path %s", r.URL.Path)
+			w.Header().Add("Path", r.URL.Path)
+		},
+	))
+	defer testServ.Close()
+	tests := []struct {
+		name            string
+		mountPoint      string
+		proxyPath       string
+		requestPath     string
+		wantRequestPath string
+	}{
+		{
+			name:            "/foo -> /foo, with mount point and path /foo",
+			mountPoint:      "/foo",
+			proxyPath:       "/foo",
+			requestPath:     "/foo",
+			wantRequestPath: "/foo",
+		},
+		{
+			name:            "/foo/ -> /foo/, with mount point and path /foo",
+			mountPoint:      "/foo",
+			proxyPath:       "/foo",
+			requestPath:     "/foo/",
+			wantRequestPath: "/foo/",
+		},
+		{
+			name:            "/foo -> /foo/, with mount point and path /foo/",
+			mountPoint:      "/foo/",
+			proxyPath:       "/foo/",
+			requestPath:     "/foo",
+			wantRequestPath: "/foo/",
+		},
+		{
+			name:            "/-> /, with mount point and path /",
+			mountPoint:      "/",
+			proxyPath:       "/",
+			requestPath:     "/",
+			wantRequestPath: "/",
+		},
+		{
+			name:            "/foo -> /foo, with mount point and path /",
+			mountPoint:      "/",
+			proxyPath:       "/",
+			requestPath:     "/foo",
+			wantRequestPath: "/foo",
+		},
+		{
+			name:            "/foo/bar -> /foo/bar, with mount point and path /foo",
+			mountPoint:      "/foo",
+			proxyPath:       "/foo",
+			requestPath:     "/foo/bar",
+			wantRequestPath: "/foo/bar",
+		},
+		{
+			name:            "/foo/bar/baz -> /foo/bar/baz, with mount point and path /foo",
+			mountPoint:      "/foo",
+			proxyPath:       "/foo",
+			requestPath:     "/foo/bar/baz",
+			wantRequestPath: "/foo/bar/baz",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			conf := &ipn.ServeConfig{
+				Web: map[ipn.HostPort]*ipn.WebServerConfig{
+					"example.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
+						tt.mountPoint: {Proxy: testServ.URL + tt.proxyPath},
+					}},
+				},
+			}
+			if err := b.SetServeConfig(conf, ""); err != nil {
+				t.Fatal(err)
+			}
+			req := &http.Request{
+				URL: &url.URL{Path: tt.requestPath},
+				TLS: &tls.ConnectionState{ServerName: "example.ts.net"},
+			}
+			req = req.WithContext(context.WithValue(req.Context(), serveHTTPContextKey{}, &serveHTTPContext{
+				DestPort: 443,
+				SrcAddr:  netip.MustParseAddrPort("1.2.3.4:1234"), // random src
+			}))
+
+			w := httptest.NewRecorder()
+			b.serveWebHandler(w, req)
+
+			// Verify what path was requested
+			p := w.Result().Header.Get("Path")
+			if p != tt.wantRequestPath {
+				t.Errorf("wanted request path %s got %s", tt.wantRequestPath, p)
+			}
+		})
+	}
+}
+func TestServeHTTPProxyHeaders(t *testing.T) {
 	b := newTestBackend(t)
 
 	// Start test serve endpoint.