Candidate priority is incorrectly calculated

Some constants were being incorrectly generated using 2^N (XOR)
instead of 1 << N. This resulted in incorrect candidate priorities
being used to evaluate candidate pairs.

Resolves #14.
This commit is contained in:
Michael MacDonald
2019-04-11 13:40:39 -04:00
committed by Michael MacDonald
parent 7245ac0866
commit ad7c8697fd
5 changed files with 181 additions and 21 deletions
+2 -2
View File
@@ -368,7 +368,7 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
&stun.Username{Username: a.remoteUfrag + ":" + a.localUfrag},
&stun.UseCandidate{},
&stun.IceControlling{TieBreaker: a.tieBreaker},
&stun.Priority{Priority: uint32(local.Priority())},
&stun.Priority{Priority: local.Priority()},
&stun.MessageIntegrity{
Key: []byte(a.remotePwd),
},
@@ -378,7 +378,7 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
msg, err = stun.Build(stun.ClassRequest, stun.MethodBinding, stun.GenerateTransactionID(),
&stun.Username{Username: a.remoteUfrag + ":" + a.localUfrag},
&stun.IceControlled{TieBreaker: a.tieBreaker},
&stun.Priority{Priority: uint32(local.Priority())},
&stun.Priority{Priority: local.Priority()},
&stun.MessageIntegrity{
Key: []byte(a.remotePwd),
},
+4 -4
View File
@@ -202,16 +202,16 @@ func (c *Candidate) writeTo(raw []byte, dst *Candidate) (int, error) {
}
// Priority computes the priority for this ICE Candidate
func (c *Candidate) Priority() uint16 {
func (c *Candidate) Priority() uint32 {
// The local preference MUST be an integer from 0 (lowest preference) to
// 65535 (highest preference) inclusive. When there is only a single IP
// address, this value SHOULD be set to 65535. If there are multiple
// candidates for a particular component for a particular data stream
// that have the same type, the local preference MUST be unique for each
// one.
return (2^24)*c.Type.Preference() +
(2^8)*c.LocalPreference +
(2^0)*(256-c.Component)
return (1<<24)*uint32(c.Type.Preference()) +
(1<<8)*uint32(c.LocalPreference) +
uint32(256-c.Component)
}
// Equal is used to compare two CandidateBases
+47
View File
@@ -0,0 +1,47 @@
package ice
import "testing"
func TestCandidatePriority(t *testing.T) {
for _, test := range []struct {
Candidate *Candidate
WantPriority uint32
}{
{
Candidate: &Candidate{
Type: CandidateTypeHost,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
},
WantPriority: 2130706431,
},
{
Candidate: &Candidate{
Type: CandidateTypePeerReflexive,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
},
WantPriority: 1862270975,
},
{
Candidate: &Candidate{
Type: CandidateTypeServerReflexive,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
},
WantPriority: 1694498815,
},
{
Candidate: &Candidate{
Type: CandidateTypeRelay,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
},
WantPriority: 16777215,
},
} {
if got, want := test.Candidate.Priority(), test.WantPriority; got != want {
t.Fatalf("Candidate(%v).Priority() = %d, want %d", test.Candidate, got, want)
}
}
}
+17 -15
View File
@@ -41,39 +41,41 @@ func (p *candidatePair) Equal(other *candidatePair) bool {
// agent. Let D be the priority for the candidate provided by the
// controlled agent.
// pair priority = 2^32*MIN(G,D) + 2*MAX(G,D) + (G>D?1:0)
func (p *candidatePair) Priority() uint32 {
func (p *candidatePair) Priority() uint64 {
var g uint32
var d uint32
if p.iceRoleControlling {
g = uint32(p.local.Priority())
d = uint32(p.remote.Priority())
g = p.local.Priority()
d = p.remote.Priority()
} else {
g = uint32(p.remote.Priority())
d = uint32(p.local.Priority())
g = p.remote.Priority()
d = p.local.Priority()
}
// Just implement these here rather
// than fooling around with the math package
min := func(x, y uint32) uint32 {
min := func(x, y uint32) uint64 {
if x < y {
return x
return uint64(x)
}
return y
return uint64(y)
}
max := func(x, y uint32) uint32 {
max := func(x, y uint32) uint64 {
if x > y {
return x
return uint64(x)
}
return y
return uint64(y)
}
cmp := func(x, y uint32) uint32 {
cmp := func(x, y uint32) uint64 {
if x > y {
return 1
return uint64(1)
}
return 0
return uint64(0)
}
return (2^32)*min(g, d) + 2*max(g, d) + cmp(g, d)
// 1<<32 overflows uint32; and if both g && d are
// maxUint32, this result would overflow uint64
return (1<<32-1)*min(g, d) + 2*max(g, d) + cmp(g, d)
}
func (p *candidatePair) Write(b []byte) (int, error) {
+111
View File
@@ -0,0 +1,111 @@
package ice
import "testing"
var (
hostCandidate = &Candidate{
Type: CandidateTypeHost,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
}
prflxCandidate = &Candidate{
Type: CandidateTypePeerReflexive,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
}
srflxCandidate = &Candidate{
Type: CandidateTypeServerReflexive,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
}
relayCandidate = &Candidate{
Type: CandidateTypeRelay,
LocalPreference: defaultLocalPreference,
Component: ComponentRTP,
}
)
func TestCandidatePairPriority(t *testing.T) {
for _, test := range []struct {
Pair *candidatePair
WantPriority uint64
}{
{
Pair: newCandidatePair(
hostCandidate,
hostCandidate,
false,
),
WantPriority: 9151314440652587007,
},
{
Pair: newCandidatePair(
hostCandidate,
hostCandidate,
true,
),
WantPriority: 9151314440652587007,
},
{
Pair: newCandidatePair(
hostCandidate,
prflxCandidate,
true,
),
WantPriority: 7998392936314175488,
},
{
Pair: newCandidatePair(
hostCandidate,
prflxCandidate,
false,
),
WantPriority: 7998392936314175487,
},
{
Pair: newCandidatePair(
hostCandidate,
srflxCandidate,
true,
),
WantPriority: 7277816996102668288,
},
{
Pair: newCandidatePair(
hostCandidate,
srflxCandidate,
false,
),
WantPriority: 7277816996102668287,
},
{
Pair: newCandidatePair(
hostCandidate,
relayCandidate,
true,
),
WantPriority: 72057593987596288,
},
{
Pair: newCandidatePair(
hostCandidate,
relayCandidate,
false,
),
WantPriority: 72057593987596287,
},
} {
if got, want := test.Pair.Priority(), test.WantPriority; got != want {
t.Fatalf("CandidatePair(%v).Priority() = %d, want %d", test.Pair, got, want)
}
}
}
func TestCandidatePairEquality(t *testing.T) {
pairA := newCandidatePair(hostCandidate, srflxCandidate, true)
pairB := newCandidatePair(hostCandidate, srflxCandidate, false)
if !pairA.Equal(pairB) {
t.Fatalf("Expected %v to equal %v", pairA, pairB)
}
}