From 751b30b73010580eabb1d31edba0773df6210c3e Mon Sep 17 00:00:00 2001 From: neuronori Date: Thu, 25 Jun 2026 17:52:29 +0000 Subject: [PATCH] fix(jitsi): accept pre-latch broadcast so WaitForPeer unblocks (issue #95) WaitForPeer makes the client block before it sends its SYN, so the server cannot yet know the client epoch and its first welcome arrives as a broadcast (receiverEpoch=0). The same happens on every reconnect: peerEpoch is reset to 0 and the peer re-announces via a broadcast Send(nil). The strict requireTargetedPeer guard dropped those broadcasts while unlatched, so peerEpoch never latched and WaitForPeer hung for its whole timeout - the link connected and pinged but no data ever flowed ("ping works, no connection"), most visibly when a peer left and reconnected. Accept a broadcast while unlatched (peerEpoch==0) and keep rejecting third-party broadcasts once latched. --- internal/engine/jitsi/jitsi.go | 27 +++++++++----- internal/engine/jitsi/jitsi_test.go | 55 ++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/internal/engine/jitsi/jitsi.go b/internal/engine/jitsi/jitsi.go index 9ed98f6..5bc8933 100644 --- a/internal/engine/jitsi/jitsi.go +++ b/internal/engine/jitsi/jitsi.go @@ -1270,17 +1270,26 @@ func (s *Session) acceptEpochFrame(payload []byte) ([]byte, bool) { receiverEpoch, s.localEpoch.Load()) return nil, false } - // Drop untargeted (broadcast) frames unless they come from the peer we - // have already latched onto. The server's first reply to a client is - // always targeted (it latches the client's localEpoch from the client's - // initial SYN frame before smux ever emits a reply), so a legitimate - // welcome carries receiverEpoch == localEpoch and never reaches this - // branch. Untargeted frames here are therefore either a third-party - // olcrtc instance broadcasting before our peer does, or post-latch - // broadcasts from our own peer (which we keep accepting). + // Untargeted (broadcast) frame handling. A broadcast carries + // receiverEpoch==0 because the sender does not yet know our localEpoch. + // + // We MUST accept a broadcast while we are still unlatched (peerEpoch==0): + // the client blocks in WaitForPeer until peerEpoch latches, and that latch + // can only come from the peer's first frame. On both initial connect and + // after a reconnect (which resets peerEpoch to 0 and re-announces via a + // broadcast Send(nil)), that first frame is a broadcast - the sender has + // not learned our epoch yet. Dropping it here wedges WaitForPeer for its + // whole timeout and the link never comes up ("ping works, no connection"). + // + // Once we ARE latched (peerEpoch!=0) we only accept broadcasts from that + // same latched sender; a broadcast from a different senderEpoch is a + // third-party olcrtc instance or a stale ghost in a polluted room and is + // dropped. A genuine peer reconnect arrives as a fresh-epoch broadcast + // only after peerEpoch was reset to 0, so it bootstraps via the unlatched + // branch above. if s.requireTargetedPeer && s.onPeerData == nil && receiverEpoch != s.localEpoch.Load() { knownPeerEpoch := s.peerEpoch.Load() - if knownPeerEpoch == 0 || senderEpoch != knownPeerEpoch { + if knownPeerEpoch != 0 && senderEpoch != knownPeerEpoch { logger.Debugf("jitsi: drop untargeted bridge frame senderEpoch=0x%08x localEpoch=0x%08x", senderEpoch, s.localEpoch.Load()) return nil, false diff --git a/internal/engine/jitsi/jitsi_test.go b/internal/engine/jitsi/jitsi_test.go index ec33888..16a84a0 100644 --- a/internal/engine/jitsi/jitsi_test.go +++ b/internal/engine/jitsi/jitsi_test.go @@ -343,8 +343,20 @@ func TestReconnectEpochAnnounceWithZeroPeerEpochIsAccepted(t *testing.T) { } } +// TestRequireTargetedPeerLatchesFirstBroadcastThenRejectsOthers codifies the +// field-verified handshake order under WaitForPeer. The client blocks in +// WaitForPeer until peerEpoch latches, and the latch can only come from the +// peer's first bridge frame. Because the client waits before sending its own +// SYN, the server has not learned the client's localEpoch yet, so that first +// frame necessarily arrives as a broadcast (receiverEpoch==0). The guard must +// therefore accept an unlatched broadcast (or WaitForPeer wedges and the link +// never comes up - "ping works, no connection"). Once latched, broadcasts from +// a different senderEpoch (a third-party olcrtc instance or a stale ghost in a +// polluted room) are dropped, while further frames from the latched peer keep +// flowing. +// //nolint:cyclop // setup asserts latch, epoch, and delivery state -func TestRequireTargetedPeerDropsOtherClientBroadcastBeforeLatch(t *testing.T) { +func TestRequireTargetedPeerLatchesFirstBroadcastThenRejectsOthers(t *testing.T) { var received [][]byte sess, err := New(context.Background(), engine.Config{ URL: testHost, @@ -365,28 +377,37 @@ func TestRequireTargetedPeerDropsOtherClientBroadcastBeforeLatch(t *testing.T) { } js.localEpoch.Store(0x3333) - otherClient := makeBridgeFrameForEpoch(t, 0x2222, 0, []byte("CLIENT_HELLO")) - js.deliverBridgeMessage(makeBridgeMessageFrom("clientB", map[string]any{rawFieldKey: otherClient}), true) - if len(received) != 0 { - t.Fatalf("received other client broadcast = %q, want none", received) - } - if got := js.peerEpoch.Load(); got != 0 { - t.Fatalf("peerEpoch after other client broadcast = 0x%08x, want 0", got) - } - if got := js.peerEndpoint.Load(); got != nil { - t.Fatalf("peerEndpoint after other client broadcast = %q, want nil", *got) - } - - serverWelcome := makeBridgeFrameForEpoch(t, 0x1111, 0x3333, []byte("SERVER_WELCOME")) + // Server welcome arrives as a broadcast (receiverEpoch==0) because the + // server has not learned our epoch yet. It must latch us and be delivered + // so WaitForPeer can unblock. + serverWelcome := makeBridgeFrameForEpoch(t, 0x1111, 0, []byte("SERVER_WELCOME")) js.deliverBridgeMessage(makeBridgeMessageFrom("server", map[string]any{rawFieldKey: serverWelcome}), true) if len(received) != 1 || string(received[0]) != "SERVER_WELCOME" { - t.Fatalf("received = %q, want targeted server welcome", received) + t.Fatalf("received = %q, want server welcome", received) } if got := js.peerEpoch.Load(); got != 0x1111 { - t.Fatalf("peerEpoch = 0x%08x, want server epoch", got) + t.Fatalf("peerEpoch after welcome = 0x%08x, want server epoch", got) } if got := js.peerEndpoint.Load(); got == nil || *got != "server" { - t.Fatalf("peerEndpoint = %v, want server", got) + t.Fatalf("peerEndpoint after welcome = %v, want server", got) + } + + // A broadcast from a different senderEpoch after we are latched is a + // third-party/ghost and must be dropped. + otherClient := makeBridgeFrameForEpoch(t, 0x2222, 0, []byte("CLIENT_HELLO")) + js.deliverBridgeMessage(makeBridgeMessageFrom("clientB", map[string]any{rawFieldKey: otherClient}), true) + if len(received) != 1 { + t.Fatalf("received after third-party broadcast = %q, want only server welcome", received) + } + if got := js.peerEpoch.Load(); got != 0x1111 { + t.Fatalf("peerEpoch after third-party broadcast = 0x%08x, want latched server epoch", got) + } + + // A further frame from the latched peer keeps flowing. + more := makeBridgeFrameForEpoch(t, 0x1111, 0, []byte("MORE")) + js.deliverBridgeMessage(makeBridgeMessageFrom("server", map[string]any{rawFieldKey: more}), true) + if len(received) != 2 || string(received[1]) != "MORE" { + t.Fatalf("received = %q, want server welcome + MORE", received) } }