Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
13 changes: 13 additions & 0 deletions agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,19 @@ func (a *Agent) AddRemoteCandidate(cand Candidate) error {
return nil
}

// AddLocalCandidate adds a local candidate and the packet connection used to
// send and receive packets for it.
func (a *Agent) AddLocalCandidate(cand Candidate, candidateConn net.PacketConn) error {
if cand == nil {
return nil
}
if candidateConn == nil {
return ErrCandidatePacketConnNil
}

return a.addCandidate(a.loop, cand, candidateConn)
}
Comment on lines +1032 to +1041

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make sure that a user error with calling this function with the same candidate twice doesn't cause the agent to close the candidate during duplication checks and just returns an error?

ice/agent.go

Lines 1358 to 1364 in c649265

a.log.Debugf("Ignore duplicate candidate: %s", cand)
if err := cand.close(); err != nil {
a.log.Warnf("Failed to close duplicate candidate: %v", err)
}
if err := candidateConn.Close(); err != nil {
a.log.Warnf("Failed to close duplicate candidate connection: %v", err)
}

Maybe we can just add a new parameter to addCandidate so it errors and returns if it detects duplication, while keeping the close behavior for normal path?


func (a *Agent) resolveAndAddMulticastCandidate(cand *CandidateHost) {
if a.mDNSConn == nil {
return
Expand Down
4 changes: 4 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ var (
// ErrNoOnCandidateHandler indicates agent was started without OnCandidate.
ErrNoOnCandidateHandler = errors.New("no OnCandidate provided")

// ErrCandidatePacketConnNil indicates a local candidate was added without
// a packet connection.
ErrCandidatePacketConnNil = errors.New("candidate packet connection is nil")

// ErrMultipleGatherAttempted indicates GatherCandidates has been called multiple times.
ErrMultipleGatherAttempted = errors.New("attempting to gather candidates during gathering state")

Expand Down
137 changes: 137 additions & 0 deletions local_candidate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// SPDX-FileCopyrightText: 2026 The Pion community <https://pion.ly>
// SPDX-License-Identifier: MIT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please merge this test file with an existing test file? we're trying to limit how much files we add to /


package ice

import (
"io"
"net"
"sync/atomic"
"testing"
"time"

"github.com/stretchr/testify/require"
)

type localCandidatePacketConn struct {
addr net.Addr
closeCount atomic.Int32
}

func (c *localCandidatePacketConn) ReadFrom([]byte) (int, net.Addr, error) {
return 0, c.addr, io.EOF
}

func (c *localCandidatePacketConn) WriteTo(payload []byte, _ net.Addr) (int, error) {
return len(payload), nil
}

func (c *localCandidatePacketConn) Close() error {
c.closeCount.Add(1)

return nil
}

func (c *localCandidatePacketConn) LocalAddr() net.Addr { return c.addr }
func (c *localCandidatePacketConn) SetDeadline(time.Time) error { return nil }
func (c *localCandidatePacketConn) SetReadDeadline(time.Time) error { return nil }
func (c *localCandidatePacketConn) SetWriteDeadline(time.Time) error { return nil }

func TestAddLocalCandidateRegistersExternalRelay(t *testing.T) {
agent, err := NewAgentWithOptions(
WithCandidateTypes([]CandidateType{}),
WithMulticastDNSMode(MulticastDNSModeDisabled),
)
require.NoError(t, err)
defer func() { require.NoError(t, agent.Close()) }()

gatheringComplete := make(chan struct{})
candidates := make(chan Candidate, 1)
require.NoError(t, agent.OnCandidate(func(candidate Candidate) {
if candidate == nil {
close(gatheringComplete)

return
}
candidates <- candidate
}))

candidate, err := NewCandidateRelay(&CandidateRelayConfig{
Network: NetworkTypeUDP4.String(),
Address: "192.0.2.10",
Port: 5000,
Component: ComponentRTP,
RelayProtocol: "custom",
})
require.NoError(t, err)
packetConn := &localCandidatePacketConn{
addr: &net.UDPAddr{IP: net.IPv4(192, 0, 2, 10), Port: 5000},
}

require.NoError(t, agent.AddLocalCandidate(candidate, packetConn))
localCandidates, err := agent.GetLocalCandidates()
require.NoError(t, err)
require.Len(t, localCandidates, 1)
relayCandidate, ok := localCandidates[0].(*CandidateRelay)
require.True(t, ok)
require.Equal(t, "custom", relayCandidate.RelayProtocol())

select {
case got := <-candidates:
require.Equal(t, candidate, got)
case <-time.After(time.Second):
require.FailNow(t, "timed out waiting for local candidate callback")
}

require.NoError(t, agent.GatherCandidates())
select {
case <-gatheringComplete:
case <-time.After(time.Second):
require.FailNow(t, "timed out waiting for gathering completion")
}
}

func TestAddLocalCandidateRejectsNilPacketConn(t *testing.T) {
agent, err := NewAgentWithOptions(WithMulticastDNSMode(MulticastDNSModeDisabled))
require.NoError(t, err)
defer func() { require.NoError(t, agent.Close()) }()

candidate, err := NewCandidateRelay(&CandidateRelayConfig{
Network: NetworkTypeUDP4.String(),
Address: "192.0.2.11",
Port: 5001,
Component: ComponentRTP,
})
require.NoError(t, err)
require.Error(t, agent.AddLocalCandidate(candidate, nil))
}

func TestAddLocalCandidateClosesDuplicateConnection(t *testing.T) {
agent, err := NewAgentWithOptions(WithMulticastDNSMode(MulticastDNSModeDisabled))
require.NoError(t, err)
defer func() { require.NoError(t, agent.Close()) }()

require.NoError(t, agent.OnCandidate(func(Candidate) {}))
config := CandidateRelayConfig{
Network: NetworkTypeUDP4.String(),
Address: "192.0.2.12",
Port: 5002,
Component: ComponentRTP,
}
first, err := NewCandidateRelay(&config)
require.NoError(t, err)
second, err := NewCandidateRelay(&config)
require.NoError(t, err)
firstConn := &localCandidatePacketConn{addr: &net.UDPAddr{IP: net.IPv4(192, 0, 2, 12), Port: 5002}}
secondConn := &localCandidatePacketConn{addr: &net.UDPAddr{IP: net.IPv4(192, 0, 2, 12), Port: 5002}}

require.NoError(t, agent.AddLocalCandidate(first, firstConn))
require.NoError(t, agent.AddLocalCandidate(second, secondConn))
require.Eventually(t, func() bool {
return secondConn.closeCount.Load() == 1
}, time.Second, time.Millisecond)

localCandidates, err := agent.GetLocalCandidates()
require.NoError(t, err)
require.Len(t, localCandidates, 1)
}
Loading