From b0736fe6f7b3f0ecb7f6c6606dea2a180b065b71 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Thu, 17 Nov 2022 00:07:21 +0500 Subject: [PATCH] ipn/ipnlocal: move selfNode from peerAPIServer to peerAPIHandler The peerAPIHandler is instantiated per PeerAPI call so it is guaranteed to have the latest selfNode. Signed-off-by: Maisem Ali --- ipn/ipnlocal/local.go | 1 - ipn/ipnlocal/peerapi.go | 22 +++++++++++----------- ipn/ipnlocal/peerapi_test.go | 18 +++++++++--------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 147c9810e..f3a8518bd 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2784,7 +2784,6 @@ func (b *LocalBackend) initPeerAPIListener() { ps := &peerAPIServer{ b: b, rootDir: fileRoot, - selfNode: selfNode, directFileMode: b.directFileRoot != "", directFileDoFinalRename: b.directFileDoFinalRename, } diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 902e3cc3b..382b4cadc 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -60,7 +60,6 @@ var addH2C func(*http.Server) type peerAPIServer struct { b *LocalBackend rootDir string // empty means file receiving unavailable - selfNode *tailcfg.Node knownEmpty atomic.Bool resolver *resolver.Resolver @@ -514,10 +513,17 @@ func (pln *peerAPIListener) ServeConn(src netip.AddrPort, c net.Conn) { c.Close() return } + nm := pln.lb.NetMap() + if nm == nil || nm.SelfNode == nil { + logf("peerapi: no netmap") + c.Close() + return + } h := &peerAPIHandler{ ps: pln.ps, - isSelf: pln.ps.selfNode.User == peerNode.User, + isSelf: nm.SelfNode.User == peerNode.User, remoteAddr: src, + selfNode: nm.SelfNode, peerNode: peerNode, peerUser: peerUser, } @@ -535,6 +541,7 @@ type peerAPIHandler struct { ps *peerAPIServer remoteAddr netip.AddrPort isSelf bool // whether peerNode is owned by same user as this node + selfNode *tailcfg.Node // this node; always non-nil peerNode *tailcfg.Node // peerNode is who's making the request peerUser tailcfg.UserProfile // profile of peerNode } @@ -552,7 +559,7 @@ func (h *peerAPIHandler) validateHost(r *http.Request) error { return err } hostIPPfx := netip.PrefixFrom(ap.Addr(), ap.Addr().BitLen()) - if !slices.Contains(h.ps.selfNode.Addresses, hostIPPfx) { + if !slices.Contains(h.selfNode.Addresses, hostIPPfx) { return fmt.Errorf("%v not found in self addresses", hostIPPfx) } return nil @@ -778,14 +785,7 @@ func (h *peerAPIHandler) canPutFile() bool { // canDebug reports whether h can debug this node (goroutines, metrics, // magicsock internal state, etc). func (h *peerAPIHandler) canDebug() bool { - // Reread the selfNode as it may have changed since the peerAPIServer - // was created. - // TODO(maisem): handle this in other places too. - nm := h.ps.b.NetMap() - if nm == nil || nm.SelfNode == nil { - return false - } - if !slices.Contains(nm.SelfNode.Capabilities, tailcfg.CapabilityDebug) { + if !slices.Contains(h.selfNode.Capabilities, tailcfg.CapabilityDebug) { // This node does not expose debug info. return false } diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 83fd01163..2da8d7a95 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -449,6 +449,9 @@ func TestHandlePeerAPI(t *testing.T) { netip.MustParsePrefix("100.100.100.101/32"), }, } + if tt.debugCap { + selfNode.Capabilities = append(selfNode.Capabilities, tailcfg.CapabilityDebug) + } var e peerAPITestEnv lb := &LocalBackend{ logf: e.logBuf.Logf, @@ -456,18 +459,15 @@ func TestHandlePeerAPI(t *testing.T) { netMap: &netmap.NetworkMap{SelfNode: selfNode}, } e.ph = &peerAPIHandler{ - isSelf: tt.isSelf, + isSelf: tt.isSelf, + selfNode: selfNode, peerNode: &tailcfg.Node{ ComputedName: "some-peer-name", }, ps: &peerAPIServer{ - b: lb, - selfNode: selfNode, + b: lb, }, } - if tt.debugCap { - e.ph.ps.selfNode.Capabilities = append(e.ph.ps.selfNode.Capabilities, tailcfg.CapabilityDebug) - } var rootDir string if !tt.omitRoot { rootDir = t.TempDir() @@ -507,9 +507,6 @@ func TestFileDeleteRace(t *testing.T) { logf: t.Logf, capFileSharing: true, }, - selfNode: &tailcfg.Node{ - Addresses: []netip.Prefix{netip.MustParsePrefix("100.100.100.101/32")}, - }, rootDir: dir, } ph := &peerAPIHandler{ @@ -517,6 +514,9 @@ func TestFileDeleteRace(t *testing.T) { peerNode: &tailcfg.Node{ ComputedName: "some-peer-name", }, + selfNode: &tailcfg.Node{ + Addresses: []netip.Prefix{netip.MustParsePrefix("100.100.100.101/32")}, + }, ps: ps, } buf := make([]byte, 2<<20)