Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion selection.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,15 @@ func (s *controlledSelector) HandleBindingRequest(message *stun.Message, local,
}

s.agent.sendBindingSuccess(message, local, remote)
s.PingCandidate(local, remote)

// Only send a triggered check during ICE checking phase (RFC 8445 §7.3.1.4).
// Once the pair is established (succeeded + selected), sending a triggered check
// on every inbound request creates a ping-pong busy loop: the remote side responds
// and sends its own request, which triggers another check here, repeating at 1/RTT.
// After connection, consent freshness is maintained by checkKeepalive() on a timer.
if pair.state != CandidatePairStateSucceeded || s.agent.getSelectedPair() == nil {
s.PingCandidate(local, remote)
}

if s.agent.userBindingRequestHandler != nil {
if shouldSwitch := s.agent.userBindingRequestHandler(message, local, remote, pair); shouldSwitch {
Expand Down
97 changes: 97 additions & 0 deletions selection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,103 @@ func TestControlledSelector_HandleSuccessResponse_UnknownTxID(t *testing.T) {
require.True(t, logger.warned, "expected Warnf to be called for unknown TransactionID (hitting !ok branch)")
}

// TestControlledSelector_NoTriggeredCheckAfterConnected verifies that once a pair
// is in Succeeded state with a selected pair, HandleBindingRequest does NOT send
// a triggered check (PingCandidate). Before the fix, every inbound Binding Request
// unconditionally called PingCandidate, creating a ping-pong busy loop at 1/RTT.
func TestControlledSelector_NoTriggeredCheckAfterConnected(t *testing.T) {
agent := bareAgentForPing()
agent.log = logging.NewDefaultLoggerFactory().NewLogger("test")
agent.remoteUfrag = selectionTestRemoteUfrag
agent.localUfrag = selectionTestLocalUfrag
agent.remotePwd = selectionTestPassword
agent.localPwd = selectionTestPassword
agent.tieBreaker = 1
agent.isControlling.Store(false)
agent.onConnected = make(chan struct{})
agent.setSelector()

selector, ok := agent.getSelector().(*controlledSelector)
require.True(t, ok, "expected controlledSelector")

local := newPingNoIOCand()
local.candidateBase.networkType = NetworkTypeUDP4
local.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000}

remote := newPingNoIOCand()
remote.candidateBase.networkType = NetworkTypeUDP4
remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000}

pair := agent.addPair(local, remote)
pair.state = CandidatePairStateSucceeded
agent.setSelectedPair(pair)

// Record requests sent before calling HandleBindingRequest.
sentBefore := pair.RequestsSent()

// Build a STUN Binding Request (no USE-CANDIDATE).
msg, err := stun.Build(stun.BindingRequest,
stun.TransactionID,
stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag),
stun.NewShortTermIntegrity(agent.localPwd),
stun.Fingerprint,
)
require.NoError(t, err)

// Call HandleBindingRequest multiple times — simulates repeated inbound requests.
for range 10 {
selector.HandleBindingRequest(msg, local, remote)
}

// No triggered checks should have been sent since the pair is already connected.
assert.Equal(t, sentBefore, pair.RequestsSent(),
"triggered check should not be sent for a succeeded+selected pair")
}

// TestControlledSelector_TriggeredCheckDuringChecking verifies that a triggered
// check IS sent when the pair is not yet in Succeeded state (normal ICE checking).
func TestControlledSelector_TriggeredCheckDuringChecking(t *testing.T) {
agent := bareAgentForPing()
agent.log = logging.NewDefaultLoggerFactory().NewLogger("test")
agent.remoteUfrag = selectionTestRemoteUfrag
agent.localUfrag = selectionTestLocalUfrag
agent.remotePwd = selectionTestPassword
agent.localPwd = selectionTestPassword
agent.tieBreaker = 1
agent.isControlling.Store(false)
agent.onConnected = make(chan struct{})
agent.setSelector()

selector, ok := agent.getSelector().(*controlledSelector)
require.True(t, ok, "expected controlledSelector")

local := newPingNoIOCand()
local.candidateBase.networkType = NetworkTypeUDP4
local.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000}

remote := newPingNoIOCand()
remote.candidateBase.networkType = NetworkTypeUDP4
remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000}

pair := agent.addPair(local, remote)
// Pair is in Waiting state (not Succeeded), no selected pair — normal ICE checking.

sentBefore := pair.RequestsSent()

msg, err := stun.Build(stun.BindingRequest,
stun.TransactionID,
stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag),
stun.NewShortTermIntegrity(agent.localPwd),
stun.Fingerprint,
)
require.NoError(t, err)

selector.HandleBindingRequest(msg, local, remote)

assert.Greater(t, pair.RequestsSent(), sentBefore,
"triggered check should be sent during ICE checking phase")
}

func TestAutomaticRenomination(t *testing.T) { //nolint:maintidx
report := test.CheckRoutines(t)
defer report()
Expand Down
Loading