From 53004dded1e574d5ccf86023dd19b57982c0a5a1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 28 Oct 2025 08:34:34 -0700 Subject: [PATCH] wgengine/magicsock: fix js/wasm crash regression loading non-existent portmapper Thanks for the report, @Need-an-AwP! Fixes #17681 Updates #9394 Change-Id: I2e0b722ef9b460bd7e79499192d1a315504ca84c Signed-off-by: Brad Fitzpatrick (cherry picked from commit edb11e0e60ce702ebe62e7bfca345f167ac5efad) --- client/local/local.go | 13 +++++++++++++ client/tailscale/apitype/apitype.go | 10 ++++++++++ feature/feature.go | 6 ++++++ feature/portmapper/portmapper.go | 2 ++ ipn/localapi/debug.go | 10 ++++++++++ tstest/integration/integration_test.go | 22 ++++++++++++++++++++++ wgengine/magicsock/magicsock.go | 8 ++++++-- 7 files changed, 69 insertions(+), 2 deletions(-) diff --git a/client/local/local.go b/client/local/local.go index 582c7b848..2382a1225 100644 --- a/client/local/local.go +++ b/client/local/local.go @@ -596,6 +596,19 @@ func (lc *Client) DebugResultJSON(ctx context.Context, action string) (any, erro return x, nil } +// QueryOptionalFeatures queries the optional features supported by the Tailscale daemon. +func (lc *Client) QueryOptionalFeatures(ctx context.Context) (*apitype.OptionalFeatures, error) { + body, err := lc.send(ctx, "POST", "/localapi/v0/debug-optional-features", 200, nil) + if err != nil { + return nil, fmt.Errorf("error %w: %s", err, body) + } + var x apitype.OptionalFeatures + if err := json.Unmarshal(body, &x); err != nil { + return nil, err + } + return &x, nil +} + // SetDevStoreKeyValue set a statestore key/value. It's only meant for development. // The schema (including when keys are re-read) is not a stable interface. func (lc *Client) SetDevStoreKeyValue(ctx context.Context, key, value string) error { diff --git a/client/tailscale/apitype/apitype.go b/client/tailscale/apitype/apitype.go index 58cdcecc7..6d239d082 100644 --- a/client/tailscale/apitype/apitype.go +++ b/client/tailscale/apitype/apitype.go @@ -94,3 +94,13 @@ type DNSQueryResponse struct { // Resolvers is the list of resolvers that the forwarder deemed able to resolve the query. Resolvers []*dnstype.Resolver } + +// OptionalFeatures describes which optional features are enabled in the build. +type OptionalFeatures struct { + // Features is the map of optional feature names to whether they are + // enabled. + // + // Disabled features may be absent from the map. (That is, false values + // are not guaranteed to be present.) + Features map[string]bool +} diff --git a/feature/feature.go b/feature/feature.go index 0d383b398..110b104da 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -13,6 +13,12 @@ var ErrUnavailable = errors.New("feature not included in this build") var in = map[string]bool{} +// Registered reports the set of registered features. +// +// The returned map should not be modified by the caller, +// not accessed concurrently with calls to Register. +func Registered() map[string]bool { return in } + // Register notes that the named feature is linked into the binary. func Register(name string) { if _, ok := in[name]; ok { diff --git a/feature/portmapper/portmapper.go b/feature/portmapper/portmapper.go index e7be00ad1..d1b903cb6 100644 --- a/feature/portmapper/portmapper.go +++ b/feature/portmapper/portmapper.go @@ -6,6 +6,7 @@ package portmapper import ( + "tailscale.com/feature" "tailscale.com/net/netmon" "tailscale.com/net/portmapper" "tailscale.com/net/portmapper/portmappertype" @@ -14,6 +15,7 @@ import ( ) func init() { + feature.Register("portmapper") portmappertype.HookNewPortMapper.Set(newPortMapper) } diff --git a/ipn/localapi/debug.go b/ipn/localapi/debug.go index b3b919d31..8aca7f009 100644 --- a/ipn/localapi/debug.go +++ b/ipn/localapi/debug.go @@ -19,6 +19,7 @@ import ( "sync" "time" + "tailscale.com/client/tailscale/apitype" "tailscale.com/feature" "tailscale.com/feature/buildfeatures" "tailscale.com/ipn" @@ -39,6 +40,7 @@ func init() { Register("debug-packet-filter-matches", (*Handler).serveDebugPacketFilterMatches) Register("debug-packet-filter-rules", (*Handler).serveDebugPacketFilterRules) Register("debug-peer-endpoint-changes", (*Handler).serveDebugPeerEndpointChanges) + Register("debug-optional-features", (*Handler).serveDebugOptionalFeatures) } func (h *Handler) serveDebugPeerEndpointChanges(w http.ResponseWriter, r *http.Request) { @@ -463,3 +465,11 @@ func (h *Handler) serveDebugLog(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } + +func (h *Handler) serveDebugOptionalFeatures(w http.ResponseWriter, r *http.Request) { + of := &apitype.OptionalFeatures{ + Features: feature.Registered(), + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(of) +} diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 234bb8c6e..64f49c7b8 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -175,6 +175,28 @@ func TestControlKnobs(t *testing.T) { } } +func TestExpectedFeaturesLinked(t *testing.T) { + tstest.Shard(t) + tstest.Parallel(t) + env := NewTestEnv(t) + n1 := NewTestNode(t, env) + + d1 := n1.StartDaemon() + n1.AwaitResponding() + lc := n1.LocalClient() + got, err := lc.QueryOptionalFeatures(t.Context()) + if err != nil { + t.Fatal(err) + } + if !got.Features["portmapper"] { + t.Errorf("optional feature portmapper unexpectedly not found: got %v", got.Features) + } + + d1.MustCleanShutdown(t) + + t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests()) +} + func TestCollectPanic(t *testing.T) { flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/15865") tstest.Shard(t) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index e3c2d478e..658478901 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -719,9 +719,13 @@ func NewConn(opts Options) (*Conn, error) { newPortMapper, ok := portmappertype.HookNewPortMapper.GetOk() if ok { c.portMapper = newPortMapper(portmapperLogf, opts.EventBus, opts.NetMon, disableUPnP, c.onlyTCP443.Load) - } else if !testenv.InTest() { - panic("unexpected: HookNewPortMapper not set") } + // If !ok, the HookNewPortMapper hook is not set (so feature/portmapper + // isn't linked), but the build tag to explicitly omit the portmapper + // isn't set either. This should only happen to js/wasm builds, where + // the portmapper is a no-op even if linked (but it's no longer linked, + // since the move to feature/portmapper), or if people are wiring up + // their own Tailscale build from pieces. } c.netMon = opts.NetMon