Repository: pion/ice
Pull request #278 (2020-09-13 03:51:38)

Foundation and TCPType Support
Both of these things were handled in the SDP package, bringing them into ICE.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=h1) Report
> Merging [#278](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=desc) into [master](https://codecov.io/gh/pion/ice/commit/166df5842e51785dba1352fa3ba9fea14deebf15?el=desc) will **decrease** coverage by `51.77%`.
> The diff coverage is `71.42%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/ice/pull/278/graphs/tree.svg?width=650&height=150&src=pr&token=1ivIyHDzva)](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master     #278       +/-   ##
===========================================
- Coverage   80.60%   28.82%   -51.78%     
===========================================
  Files          31       31               
  Lines        2531     2543       +12     
===========================================
- Hits         2040      733     -1307     
- Misses        347     1774     +1427     
+ Partials      144       36      -108     
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `?` | |
| #wasm | `28.82% <71.42%> (+0.77%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [candidate\_base.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-Y2FuZGlkYXRlX2Jhc2UuZ28=) | `50.00% <71.42%> (-31.43%)` | :arrow_down: |
| [agent\_config.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-YWdlbnRfY29uZmlnLmdv) | `0.00% <0.00%> (-85.08%)` | :arrow_down: |
| [agent\_stats.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-YWdlbnRfc3RhdHMuZ28=) | `0.00% <0.00%> (-84.75%)` | :arrow_down: |
| [candidate\_peer\_reflexive.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-Y2FuZGlkYXRlX3BlZXJfcmVmbGV4aXZlLmdv) | `0.00% <0.00%> (-84.00%)` | :arrow_down: |
| [agent.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-YWdlbnQuZ28=) | `0.00% <0.00%> (-83.26%)` | :arrow_down: |
| [transport.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-dHJhbnNwb3J0Lmdv) | `0.00% <0.00%> (-81.54%)` | :arrow_down: |
| [stun.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-c3R1bi5nbw==) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [selection.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-c2VsZWN0aW9uLmdv) | `0.00% <0.00%> (-76.56%)` | :arrow_down: |
| [gather.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-Z2F0aGVyLmdv) | `0.00% <0.00%> (-71.83%)` | :arrow_down: |
| [context.go](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree#diff-Y29udGV4dC5nbw==) | `0.00% <0.00%> (-71.43%)` | :arrow_down: |
| ... and [12 more](https://codecov.io/gh/pion/ice/pull/278/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=footer). Last update [166df58...1e7ea42](https://codecov.io/gh/pion/ice/pull/278?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/candidate.go b/candidate.go
index 237f58e..701f8f8 100644
--- a/candidate.go
+++ b/candidate.go
@@ -18,15 +18,37 @@ const (
 
 // Candidate represents an ICE candidate
 type Candidate interface {
+	// An arbitrary string used in the freezing algorithm to
+	// group similar candidates.  It is the same for two candidates that
+	// have the same type, base IP address, protocol (UDP, TCP, etc.),
+	// and STUN or TURN server.
+	Foundation() string
+
+	// ID is a unique identifier for just this candidate
+	// Unlike the foundation this is different for each candidate
 	ID() string
+
+	// A component is a piece of a data stream.
+	// An example is one for RTP, and one for RTCP
 	Component() uint16
-	Address() string
+	SetComponent(uint16)
+
+	// The last time this candidate received traffic
 	LastReceived() time.Time
+
+	// The last time this candidate sent traffic
 	LastSent() time.Time
+
 	NetworkType() NetworkType
+	Address() string
 	Port() int
+
 	Priority() uint32
+
+	// A transport address related to a
+	//  candidate, which is useful for diagnostics and other purposes
 	RelatedAddress() *CandidateRelatedAddress
+
 	String() string
 	Type() CandidateType
 	TCPType() TCPType
diff --git a/candidate_base.go b/candidate_base.go
index be0fc1b..fa9f9e9 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -3,6 +3,7 @@ package ice
 import (
 	"context"
 	"fmt"
+	"hash/crc32"
 	"net"
 	"strconv"
 	"strings"
@@ -65,6 +66,10 @@ func (c *candidateBase) ID() string {
 	return c.id
 }
 
+func (c *candidateBase) Foundation() string {
+	return fmt.Sprintf("%d", crc32.ChecksumIEEE([]byte(c.Type().String()+c.address+c.networkType.String())))
+}
+
 // Address returns Candidate Address
 func (c *candidateBase) Address() string {
 	return c.address
@@ -90,6 +95,10 @@ func (c *candidateBase) Component() uint16 {
 	return c.component
 }
 
+func (c *candidateBase) SetComponent(component uint16) {
+	c.component = component
+}
+
 // LocalPreference returns the local preference for this candidate
 func (c *candidateBase) LocalPreference() uint16 {
 	if c.NetworkType().IsTCP() {
@@ -304,6 +313,7 @@ func (c *candidateBase) Equal(other Candidate) bool {
 		c.Type() == other.Type() &&
 		c.Address() == other.Address() &&
 		c.Port() == other.Port() &&
+		c.TCPType() == other.TCPType() &&
 		c.RelatedAddress().Equal(other.RelatedAddress())
 }
 
@@ -363,7 +373,7 @@ func (c *candidateBase) context() context.Context {
 // Marshal returns the string representation of the ICECandidate
 func (c candidateBase) Marshal() string {
 	val := fmt.Sprintf("%s %d %s %d %s %d typ %s",
-		c.ID(),
+		c.Foundation(),
 		c.Component(),
 		c.NetworkType().NetworkShort(),
 		c.Priority(),
@@ -371,6 +381,10 @@ func (c candidateBase) Marshal() string {
 		c.Port(),
 		c.Type())
 
+	if c.tcpType != TCPTypeUnspecified {
+		val += fmt.Sprintf(" tcptype %s", c.tcpType.String())
+	}
+
 	if c.RelatedAddress() != nil {
 		val = fmt.Sprintf("%s raddr %s rport %d",
 			val,
@@ -419,6 +433,8 @@ func UnmarshalCandidate(raw string) (Candidate, error) {
 
 	relatedAddress := ""
 	relatedPort := 0
+	tcpType := TCPTypeUnspecified
+
 	if len(split) > 8 {
 		split = split[8:]
 
@@ -436,13 +452,18 @@ func UnmarshalCandidate(raw string) (Candidate, error) {
 				return nil, fmt.Errorf("could not parse port: %v", err)
 			}
 			relatedPort = int(rawRelatedPort)
-		}
+		} else if split[0] == "tcptype" {
+			if len(split) < 2 {
+				return nil, fmt.Errorf("could not parse typtype: incorrect length")
+			}
 
+			tcpType = NewTCPType(split[1])
+		}
 	}
 
 	switch typ {
 	case "host":
-		return NewCandidateHost(&CandidateHostConfig{foundation, protocol, address, port, uint16(component), TCPTypePassive})
+		return NewCandidateHost(&CandidateHostConfig{foundation, protocol, address, port, uint16(component), tcpType})
 	case "srflx":
 		return NewCandidateServerReflexive(&CandidateServerReflexiveConfig{foundation, protocol, address, port, uint16(component), relatedAddress, relatedPort})
 	case "prflx":


Test patch:
diff --git a/candidate_test.go b/candidate_test.go
index 509f807..857cd50 100644
--- a/candidate_test.go
+++ b/candidate_test.go
@@ -137,6 +137,75 @@ func TestCandidateLastReceived(t *testing.T) {
 	assert.Equal(t, candidate.LastReceived(), now)
 }
 
+func TestCandidateFoundation(t *testing.T) {
+	// All fields are the same
+	assert.Equal(t,
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation(),
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation())
+
+	// Different Address
+	assert.NotEqual(t,
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation(),
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "B",
+		}).Foundation())
+
+	// Different networkType
+	assert.NotEqual(t,
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation(),
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP6,
+			address:       "A",
+		}).Foundation())
+
+	// Different candidateType
+	assert.NotEqual(t,
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation(),
+		(&candidateBase{
+			candidateType: CandidateTypePeerReflexive,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+		}).Foundation())
+
+	// Port has no effect
+	assert.Equal(t,
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+			port:          8080,
+		}).Foundation(),
+		(&candidateBase{
+			candidateType: CandidateTypeHost,
+			networkType:   NetworkTypeUDP4,
+			address:       "A",
+			port:          80,
+		}).Foundation())
+}
+
 func TestCandidateMarshal(t *testing.T) {
 	for _, test := range []struct {
 		candidate   Candidate
@@ -152,7 +221,7 @@ func TestCandidateMarshal(t *testing.T) {
 			},
 			"",
 		},
-			"1938809241 1 udp 2130706431 fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a 53987 typ host",
+			"121457893 1 udp 2130706431 fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a 53987 typ host",
 			false,
 		},
 		{&CandidateHost{
@@ -164,7 +233,7 @@ func TestCandidateMarshal(t *testing.T) {
 			},
 			"",
 		},
-			"1986380506 1 udp 2130706431 10.0.75.1 53634 typ host",
+			"4273957277 1 udp 2130706431 10.0.75.1 53634 typ host",
 			false,
 		},
 		{&CandidateServerReflexive{
@@ -176,7 +245,7 @@ func TestCandidateMarshal(t *testing.T) {
 				relatedAddress: &CandidateRelatedAddress{"192.168.0.274", 53991},
 			},
 		},
-			"4207374051 1 udp 1694498815 191.228.238.68 53991 typ srflx raddr 192.168.0.274 rport 53991",
+			"647372371 1 udp 1694498815 191.228.238.68 53991 typ srflx raddr 192.168.0.274 rport 53991",
 			false,
 		},
 		{&CandidateRelay{
@@ -189,7 +258,20 @@ func TestCandidateMarshal(t *testing.T) {
 			},
 			nil,
 		},
-			"4207374051 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 5001",
+			"848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 5001",
+			false,
+		},
+		{&CandidateHost{
+			candidateBase{
+				networkType:   NetworkTypeTCP4,
+				candidateType: CandidateTypeHost,
+				address:       "192.168.0.196",
+				port:          0,
+				tcpType:       TCPTypeActive,
+			},
+			"",
+		},
+			"1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype active",
 			false,
 		},
 
@@ -201,14 +283,19 @@ func TestCandidateMarshal(t *testing.T) {
 		{nil, "4207374051 1 udp 1685790463 191.228.238.68 99999999 typ srflx raddr 192.168.0.278 rport 53991 generation 0 network-id 3", true},
 		{nil, "4207374051 1 udp 1685790463 191.228.238.68 53991 typ srflx raddr", true},
 		{nil, "4207374051 1 udp 1685790463 191.228.238.68 53991 typ srflx raddr 192.168.0.278 rport 99999999 generation 0 network-id 3", true},
+		{nil, "4207374051 INVALID udp 2130706431 10.0.75.1 53634 typ host", true},
+		{nil, "4207374051 1 udp INVALID 10.0.75.1 53634 typ host", true},
+		{nil, "4207374051 INVALID udp 2130706431 10.0.75.1 INVALID typ host", true},
+		{nil, "4207374051 1 udp 2130706431 10.0.75.1 53634 typ INVALID", true},
 	} {
 		actualCandidate, err := UnmarshalCandidate(test.marshaled)
 		if test.expectError {
 			assert.Error(t, err)
-			return
+			continue
 		}
 
 		assert.NoError(t, err)
+
 		assert.True(t, test.candidate.Equal(actualCandidate))
 		assert.Equal(t, test.marshaled, actualCandidate.Marshal())
 	}