From bbc09979ebb4a1c022a549cc75432a8ea9a65b4d Mon Sep 17 00:00:00 2001
From: Aniket <42694864+aniketatgithub@users.noreply.github.com>
Date: Mon, 2 Nov 2026 14:35:16 -0800
Subject: [PATCH] net/portmapper: allow slower routers to answer the UPnP root
 device fetch (#21877)

The root device fetch timed out after one second, but some router firmware only starts answering after just over a second, so UPnP never worked behind it. Allow three seconds.

Updates #21860

Signed-off-by: Aniket <aniketkomailkro@gmail.com>
---
 net/portmapper/upnp.go      |  9 +++++----
 net/portmapper/upnp_test.go | 40 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/net/portmapper/upnp.go b/net/portmapper/upnp.go
index 777035605..7203dead8 100644
--- a/net/portmapper/upnp.go
+++ b/net/portmapper/upnp.go
@@ -272,10 +272,11 @@ func getUPnPRootDevice(ctx context.Context, logf logger.Logf, debug DebugKnobs,
 	}
 
 	// We're fetching a smallish XML document over plain HTTP
-	// across the local LAN, without using DNS.  There should be
-	// very few round trips and low latency, so one second is a
-	// long time.
-	ctx, cancel := context.WithTimeout(ctx, time.Second)
+	// across the local LAN, without using DNS. There should be
+	// very few round trips and low latency, but some router
+	// firmware (e.g. the NETGEAR RS700) only answers after just
+	// over a second, so allow a few seconds before giving up.
+	ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
 	defer cancel()
 
 	// This part does a network fetch.
diff --git a/net/portmapper/upnp_test.go b/net/portmapper/upnp_test.go
index 6358bf932..746baf259 100644
--- a/net/portmapper/upnp_test.go
+++ b/net/portmapper/upnp_test.go
@@ -15,8 +15,11 @@ import (
 	"reflect"
 	"regexp"
 	"slices"
+	"strings"
 	"sync/atomic"
 	"testing"
+	"testing/synctest"
+	"time"
 
 	"tailscale.com/net/portmapper/portmappertype"
 	"tailscale.com/tstest"
@@ -533,6 +536,43 @@ func TestGetUPnPClient(t *testing.T) {
 	}
 }
 
+// TestGetUPnPRootDeviceSlowResponse verifies that a router whose HTTP
+// server takes over a second to answer the root device fetch (as the
+// NETGEAR RS700 firmware does) still yields a root device, instead of
+// failing with a context deadline error.
+func TestGetUPnPRootDeviceSlowResponse(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		slowRT := roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			select {
+			case <-time.After(1500 * time.Millisecond):
+			case <-req.Context().Done():
+				return nil, req.Context().Err()
+			}
+			return &http.Response{
+				Status:     "200 OK",
+				StatusCode: http.StatusOK,
+				Header: http.Header{
+					"Content-Type": []string{"text/xml"},
+				},
+				Body:    io.NopCloser(strings.NewReader(pfSenseRootDescXML)),
+				Request: req,
+			}, nil
+		})
+		ctx := upnpHTTPClientKey.WithValue(t.Context(), &http.Client{Transport: slowRT})
+
+		var logBuf tstest.MemLogger
+		dev, _, err := getUPnPRootDevice(ctx, logBuf.Logf, DebugKnobs{}, netip.MustParseAddr("192.168.1.1"), uPnPDiscoResponse{
+			Location: "http://192.168.1.1:5000/rootDesc.xml",
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if dev == nil {
+			t.Fatal("got nil root device")
+		}
+	})
+}
+
 func TestGetUPnPPortMapping(t *testing.T) {
 	igd, err := NewTestIGD(t, TestIGDOptions{UPnP: true})
 	if err != nil {