From acbf5671b715f821a35d7fc5e34bf32a03aba67e Mon Sep 17 00:00:00 2001 From: Sean DuBois Date: Thu, 20 Jun 2019 14:12:59 -0700 Subject: [PATCH] IP -> Address in public API ICE RFC specifies this value as `Address` and not IP. We need to fix this divergence to implement mDNS Relates to pion/webrtc#699 --- agent.go | 8 ++++---- agent_test.go | 28 +++++++++++++--------------- candidate.go | 4 ++-- candidate_base.go | 23 ++++++++++++----------- candidate_host.go | 30 ++++++++++++++++++++---------- candidate_peer_reflexive.go | 10 ++++++++-- candidate_relay.go | 10 ++++++++-- candidate_server_reflexive.go | 10 ++++++++-- errors.go | 3 +++ gather.go | 6 +++--- transport_test.go | 6 +++--- 11 files changed, 84 insertions(+), 54 deletions(-) diff --git a/agent.go b/agent.go index e0ebee4..2ab4700 100644 --- a/agent.go +++ b/agent.go @@ -695,7 +695,7 @@ func (a *Agent) findRemoteCandidate(networkType NetworkType, addr net.Addr) Cand set := a.remoteCandidates[networkType] for _, c := range set { - if c.IP().Equal(ip) && c.Port() == port { + if c.Address() == ip.String() && c.Port() == port { return c } } @@ -725,8 +725,8 @@ func (a *Agent) sendBindingSuccess(m *stun.Message, local, remote Candidate) { base := remote if out, err := stun.Build(m, stun.BindingSuccess, &stun.XORMappedAddress{ - IP: base.IP(), - Port: base.Port(), + IP: base.addr().IP, + Port: base.addr().Port, }, stun.NewShortTermIntegrity(a.localPwd), stun.Fingerprint, @@ -809,7 +809,7 @@ func (a *Agent) handleInbound(m *stun.Message, local Candidate, remote net.Addr) return } - prflxCandidate, err := NewCandidatePeerReflexive(networkType.String(), ip, port, local.Component(), "", 0) + prflxCandidate, err := NewCandidatePeerReflexive(networkType.String(), ip.String(), port, local.Component(), "", 0) if err != nil { a.log.Errorf("Failed to create new remote prflx candidate (%s)", err) return diff --git a/agent_test.go b/agent_test.go index 8e96244..830a304 100644 --- a/agent_test.go +++ b/agent_test.go @@ -63,7 +63,7 @@ func TestPairPriority(t *testing.T) { hostLocal, err := NewCandidateHost( "udp", - net.ParseIP("192.168.1.1"), 19216, + "192.168.1.1", 19216, 1, ) if err != nil { @@ -72,7 +72,7 @@ func TestPairPriority(t *testing.T) { relayRemote, err := NewCandidateRelay( "udp", - net.ParseIP("1.2.3.4"), 12340, + "1.2.3.4", 12340, 1, "4.3.2.1", 43210, ) @@ -82,7 +82,7 @@ func TestPairPriority(t *testing.T) { srflxRemote, err := NewCandidateServerReflexive( "udp", - net.ParseIP("10.10.10.2"), 19218, + "10.10.10.2", 19218, 1, "4.3.2.1", 43212, ) @@ -92,7 +92,7 @@ func TestPairPriority(t *testing.T) { prflxRemote, err := NewCandidatePeerReflexive( "udp", - net.ParseIP("10.10.10.2"), 19217, + "10.10.10.2", 19217, 1, "4.3.2.1", 43211, ) @@ -102,7 +102,7 @@ func TestPairPriority(t *testing.T) { hostRemote, err := NewCandidateHost( "udp", - net.ParseIP("1.2.3.5"), 12350, + "1.2.3.5", 12350, 1, ) if err != nil { @@ -145,7 +145,7 @@ func TestOnSelectedCandidatePairChange(t *testing.T) { hostLocal, err := NewCandidateHost( "udp", - net.ParseIP("192.168.1.1"), 19216, + "192.168.1.1", 19216, 1, ) if err != nil { @@ -154,7 +154,7 @@ func TestOnSelectedCandidatePairChange(t *testing.T) { relayRemote, err := NewCandidateRelay( "udp", - net.ParseIP("1.2.3.4"), 12340, + "1.2.3.4", 12340, 1, "4.3.2.1", 43210, ) @@ -204,8 +204,7 @@ func TestHandlePeerReflexive(t *testing.T) { var config AgentConfig runAgentTest(t, &config, func(a *Agent) { a.selector = &controllingSelector{agent: a, log: a.log} - ip := net.ParseIP("192.168.0.2") - local, err := NewCandidateHost("udp", ip, 777, 1) + local, err := NewCandidateHost("udp", "192.168.0.2", 777, 1) local.conn = &mockPacketConn{} if err != nil { t.Fatalf("failed to create a new candidate: %v", err) @@ -244,7 +243,7 @@ func TestHandlePeerReflexive(t *testing.T) { t.Fatal("candidate type must be prflx") } - if !c.IP().Equal(net.ParseIP("172.17.0.3")) { + if c.Address() != "172.17.0.3" { t.Fatal("IP address mismatch") } @@ -263,8 +262,7 @@ func TestHandlePeerReflexive(t *testing.T) { var config AgentConfig runAgentTest(t, &config, func(a *Agent) { a.selector = &controllingSelector{agent: a, log: a.log} - ip := net.ParseIP("192.168.0.2") - local, err := NewCandidateHost("tcp", ip, 777, 1) + local, err := NewCandidateHost("tcp", "192.168.0.2", 777, 1) if err != nil { t.Fatalf("failed to create a new candidate: %v", err) } @@ -294,7 +292,7 @@ func TestHandlePeerReflexive(t *testing.T) { {tID, &net.UDPAddr{}, false}, } - local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1) + local, err := NewCandidateHost("udp", "192.168.0.2", 777, 1) local.conn = &mockPacketConn{} if err != nil { t.Fatalf("failed to create a new candidate: %v", err) @@ -373,7 +371,7 @@ func TestInboundValidity(t *testing.T) { } remote := &net.UDPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999} - local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1) + local, err := NewCandidateHost("udp", "192.168.0.2", 777, 1) local.conn = &mockPacketConn{} if err != nil { t.Fatalf("failed to create a new candidate: %v", err) @@ -464,7 +462,7 @@ func TestInboundValidity(t *testing.T) { t.Fatalf("Error constructing ice.Agent") } - local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1) + local, err := NewCandidateHost("udp", "192.168.0.2", 777, 1) local.conn = &mockPacketConn{} if err != nil { t.Fatalf("failed to create a new candidate: %v", err) diff --git a/candidate.go b/candidate.go index e073c21..2f38a70 100644 --- a/candidate.go +++ b/candidate.go @@ -18,7 +18,7 @@ const ( // Candidate represents an ICE candidate type Candidate interface { Component() uint16 - IP() net.IP + Address() string LastReceived() time.Time LastSent() time.Time NetworkType() NetworkType @@ -30,7 +30,7 @@ type Candidate interface { Equal(other Candidate) bool - addr() net.Addr + addr() *net.UDPAddr agent() *Agent close() error diff --git a/candidate_base.go b/candidate_base.go index 89dad86..7793780 100644 --- a/candidate_base.go +++ b/candidate_base.go @@ -15,10 +15,12 @@ type candidateBase struct { candidateType CandidateType component uint16 - ip net.IP + address string port int relatedAddress *CandidateRelatedAddress + resolvedAddr *net.UDPAddr + lock sync.RWMutex lastSent time.Time lastReceived time.Time @@ -29,9 +31,9 @@ type candidateBase struct { closedCh chan struct{} } -// IP returns Candidate IP -func (c *candidateBase) IP() net.IP { - return c.ip +// Address returns Candidate Address +func (c *candidateBase) Address() string { + return c.address } // Port returns Candidate Port @@ -176,14 +178,14 @@ func (c *candidateBase) Priority() uint32 { func (c *candidateBase) Equal(other Candidate) bool { return c.NetworkType() == other.NetworkType() && c.Type() == other.Type() && - c.IP().Equal(other.IP()) && + c.Address() == other.Address() && c.Port() == other.Port() && c.RelatedAddress().Equal(other.RelatedAddress()) } // String makes the candidateBase printable func (c *candidateBase) String() string { - return fmt.Sprintf("%s %s:%d%s", c.Type(), c.IP(), c.Port(), c.relatedAddress) + return fmt.Sprintf("%s %s:%d%s", c.Type(), c.Address(), c.Port(), c.relatedAddress) } // LastReceived returns a time.Time indicating the last time @@ -222,11 +224,10 @@ func (c *candidateBase) seen(outbound bool) { } } -func (c *candidateBase) addr() net.Addr { - return &net.UDPAddr{ - IP: c.IP(), - Port: c.Port(), - } +func (c *candidateBase) addr() *net.UDPAddr { + c.lock.Lock() + defer c.lock.Unlock() + return c.resolvedAddr } func (c *candidateBase) agent() *Agent { diff --git a/candidate_host.go b/candidate_host.go index 0834bea..1300c7f 100644 --- a/candidate_host.go +++ b/candidate_host.go @@ -2,6 +2,7 @@ package ice import ( "net" + "strings" ) // CandidateHost is a candidate of type host @@ -10,19 +11,28 @@ type CandidateHost struct { } // NewCandidateHost creates a new host candidate -func NewCandidateHost(network string, ip net.IP, port int, component uint16) (*CandidateHost, error) { - networkType, err := determineNetworkType(network, ip) - if err != nil { - return nil, err - } - - return &CandidateHost{ +func NewCandidateHost(network string, address string, port int, component uint16) (*CandidateHost, error) { + c := &CandidateHost{ candidateBase: candidateBase{ - networkType: networkType, candidateType: CandidateTypeHost, - ip: ip, port: port, component: component, }, - }, nil + } + if !strings.HasSuffix(address, ".local") { + ip := net.ParseIP(address) + if ip == nil { + return nil, ErrAddressParseFailed + } + + networkType, err := determineNetworkType(network, ip) + if err != nil { + return nil, err + } + + c.candidateBase.networkType = networkType + c.candidateBase.resolvedAddr = &net.UDPAddr{IP: ip, Port: port} + } + + return c, nil } diff --git a/candidate_peer_reflexive.go b/candidate_peer_reflexive.go index ba26fc7..c6ddd39 100644 --- a/candidate_peer_reflexive.go +++ b/candidate_peer_reflexive.go @@ -8,7 +8,12 @@ type CandidatePeerReflexive struct { } // NewCandidatePeerReflexive creates a new peer reflective candidate -func NewCandidatePeerReflexive(network string, ip net.IP, port int, component uint16, relAddr string, relPort int) (*CandidatePeerReflexive, error) { +func NewCandidatePeerReflexive(network string, address string, port int, component uint16, relAddr string, relPort int) (*CandidatePeerReflexive, error) { + ip := net.ParseIP(address) + if ip == nil { + return nil, ErrAddressParseFailed + } + networkType, err := determineNetworkType(network, ip) if err != nil { return nil, err @@ -18,8 +23,9 @@ func NewCandidatePeerReflexive(network string, ip net.IP, port int, component ui candidateBase: candidateBase{ networkType: networkType, candidateType: CandidateTypePeerReflexive, - ip: ip, + address: address, port: port, + resolvedAddr: &net.UDPAddr{IP: ip, Port: port}, component: component, relatedAddress: &CandidateRelatedAddress{ Address: relAddr, diff --git a/candidate_relay.go b/candidate_relay.go index 37b5d94..92c41c5 100644 --- a/candidate_relay.go +++ b/candidate_relay.go @@ -18,7 +18,12 @@ type CandidateRelay struct { } // NewCandidateRelay creates a new relay candidate -func NewCandidateRelay(network string, ip net.IP, port int, component uint16, relAddr string, relPort int) (*CandidateRelay, error) { +func NewCandidateRelay(network string, address string, port int, component uint16, relAddr string, relPort int) (*CandidateRelay, error) { + ip := net.ParseIP(address) + if ip == nil { + return nil, ErrAddressParseFailed + } + networkType, err := determineNetworkType(network, ip) if err != nil { return nil, err @@ -28,8 +33,9 @@ func NewCandidateRelay(network string, ip net.IP, port int, component uint16, re candidateBase: candidateBase{ networkType: networkType, candidateType: CandidateTypeRelay, - ip: ip, + address: address, port: port, + resolvedAddr: &net.UDPAddr{IP: ip, Port: port}, component: component, relatedAddress: &CandidateRelatedAddress{ Address: relAddr, diff --git a/candidate_server_reflexive.go b/candidate_server_reflexive.go index be27f54..8ff5ec3 100644 --- a/candidate_server_reflexive.go +++ b/candidate_server_reflexive.go @@ -8,7 +8,12 @@ type CandidateServerReflexive struct { } // NewCandidateServerReflexive creates a new server reflective candidate -func NewCandidateServerReflexive(network string, ip net.IP, port int, component uint16, relAddr string, relPort int) (*CandidateServerReflexive, error) { +func NewCandidateServerReflexive(network string, address string, port int, component uint16, relAddr string, relPort int) (*CandidateServerReflexive, error) { + ip := net.ParseIP(address) + if ip == nil { + return nil, ErrAddressParseFailed + } + networkType, err := determineNetworkType(network, ip) if err != nil { return nil, err @@ -18,8 +23,9 @@ func NewCandidateServerReflexive(network string, ip net.IP, port int, component candidateBase: candidateBase{ networkType: networkType, candidateType: CandidateTypeServerReflexive, - ip: ip, + address: address, port: port, + resolvedAddr: &net.UDPAddr{IP: ip, Port: port}, component: component, relatedAddress: &CandidateRelatedAddress{ Address: relAddr, diff --git a/errors.go b/errors.go index e39f4ab..01e5e27 100644 --- a/errors.go +++ b/errors.go @@ -54,4 +54,7 @@ var ( // ErrPasswordEmpty indicates agent was give TURN URL with an empty Password ErrPasswordEmpty = errors.New("password is empty") + + // ErrAddressParseFailed indicates we were unable to parse a candidate address + ErrAddressParseFailed = errors.New("failed to parse address") ) diff --git a/gather.go b/gather.go index 60187b7..188c098 100644 --- a/gather.go +++ b/gather.go @@ -177,7 +177,7 @@ func (a *Agent) gatherCandidatesLocal(networkTypes []NetworkType) { } port := conn.LocalAddr().(*net.UDPAddr).Port - c, err := NewCandidateHost(network, ip, port, ComponentRTP) + c, err := NewCandidateHost(network, ip.String(), port, ComponentRTP) if err != nil { a.log.Warnf("Failed to create host candidate: %s %s %d: %v\n", network, ip, port, err) return @@ -237,7 +237,7 @@ func (a *Agent) gatherCandidatesSrflx(urls []*URL, networkTypes []NetworkType) { port := xoraddr.Port relIP := laddr.IP.String() relPort := laddr.Port - c, err := NewCandidateServerReflexive(network, ip, port, ComponentRTP, relIP, relPort) + c, err := NewCandidateServerReflexive(network, ip.String(), port, ComponentRTP, relIP, relPort) if err != nil { a.log.Warnf("Failed to create server reflexive candidate: %s %s %d: %v\n", network, ip, port, err) continue @@ -302,7 +302,7 @@ func (a *Agent) gatherCandidatesRelay(urls []*URL) error { ip := allocation.Relayed().IP port := allocation.Relayed().Port - candidate, err := NewCandidateRelay(network, ip, port, ComponentRTP, laddr.IP.String(), laddr.Port) + candidate, err := NewCandidateRelay(network, ip.String(), port, ComponentRTP, laddr.IP.String(), laddr.Port) if err != nil { a.log.Warnf("Failed to create server reflexive candidate: %s %s %d: %v\n", network, ip, port, err) continue diff --git a/transport_test.go b/transport_test.go index 5b8dc97..260fab3 100644 --- a/transport_test.go +++ b/transport_test.go @@ -337,7 +337,7 @@ func copyCandidate(o Candidate) Candidate { candidateBase{ candidateType: orig.candidateType, networkType: orig.networkType, - ip: orig.ip, + address: orig.address, port: orig.port, component: orig.component, }, @@ -347,7 +347,7 @@ func copyCandidate(o Candidate) Candidate { candidateBase{ candidateType: orig.candidateType, networkType: orig.networkType, - ip: orig.ip, + address: orig.address, port: orig.port, component: orig.component, relatedAddress: orig.relatedAddress, @@ -359,7 +359,7 @@ func copyCandidate(o Candidate) Candidate { candidateBase{ candidateType: orig.candidateType, networkType: orig.networkType, - ip: orig.ip, + address: orig.address, port: orig.port, component: orig.component, relatedAddress: orig.relatedAddress,