Skip to content

Commit

Permalink
Don't drop Binding Requests in Controlled Agent
Browse files Browse the repository at this point in the history
A controlled Agent would discard incoming Binding Requests if it didn't
cause the pair to be selected. For UDP Candidate this would be
interpreted as packet loss. For TCP Candidates not responding with a
Binding Success could be interpreted as a failure.

Firefox's ICE Agent would disconnect TCP Candidates because of this
behavior.

Resolves to pion/webrtc#2125
Resolves to pion/webrtc#1356
See https://bugzilla.mozilla.org/show_bug.cgi?id=1756460
  • Loading branch information
Sean-Der committed Feb 22, 2022
1 parent fb4760c commit e0db6d2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
13 changes: 7 additions & 6 deletions candidatepair.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ func newCandidatePair(local, remote Candidate, controlling bool) *CandidatePair
// CandidatePair is a combination of a
// local and remote candidate
type CandidatePair struct {
iceRoleControlling bool
Remote Candidate
Local Candidate
bindingRequestCount uint16
state CandidatePairState
nominated bool
iceRoleControlling bool
Remote Candidate
Local Candidate
bindingRequestCount uint16
state CandidatePairState
nominated bool
nominateOnBindingSuccess bool
}

func (p *CandidatePair) String() string {
Expand Down
15 changes: 9 additions & 6 deletions selection.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,17 @@ func (s *controlledSelector) HandleSuccessResponse(m *stun.Message, local, remot

p.state = CandidatePairStateSucceeded
s.log.Tracef("Found valid candidate pair: %s", p)
if p.nominateOnBindingSuccess {
if selectedPair := s.agent.getSelectedPair(); selectedPair == nil {
s.agent.setSelectedPair(p)
}
}
}

func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote Candidate) {
useCandidate := m.Contains(stun.AttrUseCandidate)

p := s.agent.findPair(local, remote)

if p == nil {
p = s.agent.addPair(local, remote)
}
Expand All @@ -251,7 +255,6 @@ func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote
if selectedPair := s.agent.getSelectedPair(); selectedPair == nil {
s.agent.setSelectedPair(p)
}
s.agent.sendBindingSuccess(m, local, remote)
} else {
// If the received Binding request triggered a new check to be
// enqueued in the triggered-check queue (Section 7.3.1.4), once the
Expand All @@ -261,12 +264,12 @@ func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote
// MUST remove the candidate pair from the valid list, set the
// candidate pair state to Failed, and set the checklist state to
// Failed.
s.PingCandidate(local, remote)
p.nominateOnBindingSuccess = true
}
} else {
s.agent.sendBindingSuccess(m, local, remote)
s.PingCandidate(local, remote)
}

s.agent.sendBindingSuccess(m, local, remote)
s.PingCandidate(local, remote)
}

type liteSelector struct {
Expand Down

0 comments on commit e0db6d2

Please sign in to comment.