From a046e0b335421248c8e59d6ec8aae1b8b63d83fe Mon Sep 17 00:00:00 2001 From: Joe Turki Date: Tue, 14 Jan 2025 21:16:51 -0600 Subject: [PATCH] Fixes and Refactor --- candidate.go | 10 +- candidate_base.go | 267 ++++++++----------- candidate_host.go | 2 - candidate_peer_reflexive.go | 2 - candidate_relay.go | 2 - candidate_server_reflexive.go | 2 - candidate_test.go | 477 ++++++++++++++++++++++++---------- gather.go | 7 +- 8 files changed, 461 insertions(+), 308 deletions(-) diff --git a/candidate.go b/candidate.go index 81ab44ee..e2548d95 100644 --- a/candidate.go +++ b/candidate.go @@ -56,12 +56,7 @@ type Candidate interface { // In the order of insertion, *(key value). // Extension attributes are defined in RFC 5245, Section 15.1: // https://datatracker.ietf.org/doc/html/rfc5245#section-15.1 - Extensions() []string - - // GetExtension retrieves the value of a specific extension attribute for the ICECandidate. - // Extension attributes are defined in RFC 5245, Section 15.1: - // https://datatracker.ietf.org/doc/html/rfc5245#section-15.1 - GetExtension(name string) (value string, ok bool) + Extensions() CandidateExtensions String() string Type() CandidateType @@ -69,6 +64,9 @@ type Candidate interface { Equal(other Candidate) bool + // DeepEqual same as Equal, But it also compares the candidate extensions. + DeepEqual(other Candidate) bool + Marshal() string addr() net.Addr diff --git a/candidate_base.go b/candidate_base.go index d5ef9e0b..f6d5b276 100644 --- a/candidate_base.go +++ b/candidate_base.go @@ -10,7 +10,6 @@ import ( "hash/crc32" "io" "net" - "reflect" "strconv" "strings" "sync" @@ -46,8 +45,7 @@ type candidateBase struct { remoteCandidateCaches map[AddrPort]Candidate isLocationTracked bool - - extensions []string + extensions *CandidateExtensions } // Done implements context.Context @@ -414,11 +412,13 @@ func (c *candidateBase) Equal(other Candidate) bool { c.Type() == other.Type() && c.Address() == other.Address() && c.Port() == other.Port() && - // Keeping this for the sake of backwards compatibility - // We don't need to compare TCPType anymore, because it's included in the extensions c.TCPType() == other.TCPType() && - c.RelatedAddress().Equal(other.RelatedAddress()) && - reflect.DeepEqual(c.Extensions(), other.Extensions()) + c.RelatedAddress().Equal(other.RelatedAddress()) +} + +// DeepEqual is same as Equal but also compares the extensions +func (c *candidateBase) DeepEqual(other Candidate) bool { + return c.Equal(other) && c.Extensions().Equal(other.Extensions()) } // String makes the candidateBase printable @@ -510,66 +510,37 @@ func (c *candidateBase) Marshal() string { r.Port) } - Extensions := c.Extensions() + extensions := c.Extensions().Marshal() - if len(Extensions) > 0 { - val = fmt.Sprintf("%s %s", val, strings.Join(c.Extensions(), " ")) + if extensions != "" { + val = fmt.Sprintf("%s %s", val, extensions) } return val } -func (c *candidateBase) Extensions() []string { - extensions := make([]string, 0, len(c.extensions)) - hasTCPTYPE := false - - for i := 0; i < len(c.extensions); i += 2 { - key := c.extensions[i] - - if i+1 >= len(c.extensions) { - // should never happen - break - } +func (c *candidateBase) Extensions() CandidateExtensions { + if c.extensions == nil { + ext := CandidateExtensions{} - value := c.extensions[i+1] + // Extensions were not parsed using UnmarshalCandidate + // For backwards compatibility we set TCPType value - if key == "tcptype" { //nolint:goconst - hasTCPTYPE = true + if c.TCPType() != TCPTypeUnspecified { + ext.extensions = append(ext.extensions, CandidateExtension{ + key: "tcptype", + value: c.TCPType().String(), + }) } - extensions = append(extensions, key, value) + return ext } - // for backwards compatibility, if the candidates is not parsed using UnmarshalCandidate - if !hasTCPTYPE && c.tcpType != TCPTypeUnspecified { - extensions = append(extensions, "tcptype", c.tcpType.String()) - } - - return extensions + return *c.extensions } -func (c *candidateBase) GetExtension(key string) (string, bool) { - for i := 0; i < len(c.extensions); i += 2 { - extKey := c.extensions[i] - - if i+1 >= len(c.extensions) { - // should never happen - break - } - - value := c.extensions[i+1] - - if extKey == key { - return value, true - } - } - - // for backwards compatibility, if the candidates is not parsed using UnmarshalCandidate - if key == "tcptype" && c.tcpType != TCPTypeUnspecified { - return c.tcpType.String(), true - } - - return "", false +func (c *candidateBase) setExtensions(extensions *CandidateExtensions) { + c.extensions = extensions } // UnmarshalCandidate Parses a candidate from a string @@ -605,7 +576,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { } // transport ( "UDP" / transport-extension ; from RFC 3261 ) SP - protocol, pos := readCandidatetringToken(raw, pos) + protocol, pos := readCandidateStringToken(raw, pos) if pos >= len(raw) { return nil, fmt.Errorf("%w: expected priority in %s", errAttributeTooShortICECandidate, raw) @@ -622,7 +593,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { } // connection-address SP ;from RFC 4566 - address, pos := readCandidatetringToken(raw, pos) + address, pos := readCandidateStringToken(raw, pos) // Remove IPv6 ZoneID: https://github.com/pion/ice/pull/704 address = removeZoneIDFromAddress(address) @@ -638,7 +609,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { } // "typ" SP - typeKey, pos := readCandidatetringToken(raw, pos) + typeKey, pos := readCandidateStringToken(raw, pos) if typeKey != "typ" { return nil, fmt.Errorf("%w (%s)", ErrUnknownCandidateTyp, typeKey) } @@ -648,22 +619,37 @@ func UnmarshalCandidate(raw string) (Candidate, error) { } // SP cand-type ("host" / "srflx" / "prflx" / "relay") - typ, pos := readCandidatetringToken(raw, pos) + typ, pos := readCandidateStringToken(raw, pos) - parserExt, err := readCandidateExtensions(raw, pos) + raddr, rport, pos, err := tryReadRelativeAddrs(raw, pos) if err != nil { return nil, err } tcpType := TCPTypeUnspecified + var extensions *CandidateExtensions + + if pos < len(raw) { + extensions, err = UnmarshalCandidateExtensions(raw[pos:]) + if err != nil { + return nil, fmt.Errorf("%w: %v", errParseExtension, err) //nolint:errorlint // we wrap the error + } - if parserExt.tcpType != "" { - tcpType = NewTCPType(parserExt.tcpType) + tcpTypeRaw, ok := extensions.Get("tcptype") + + if ok { + tcpType = NewTCPType(tcpTypeRaw) + if tcpType == TCPTypeUnspecified { + return nil, fmt.Errorf("%w: invalid or unsupported TCPtype %s", errParseTCPType, tcpTypeRaw) + } + } } + // this code is ugly because we can't break backwards compatibility + // with the old way of parsing candidates switch typ { case "host": - return NewCandidateHost(&CandidateHostConfig{ + candidate, err := NewCandidateHost(&CandidateHostConfig{ "", protocol, address, @@ -673,10 +659,16 @@ func UnmarshalCandidate(raw string) (Candidate, error) { foundation, tcpType, false, - parserExt.extensions, }) + if err != nil { + return nil, err + } + + candidate.setExtensions(extensions) + + return candidate, nil case "srflx": - return NewCandidateServerReflexive(&CandidateServerReflexiveConfig{ + candidate, err := NewCandidateServerReflexive(&CandidateServerReflexiveConfig{ "", protocol, address, @@ -684,12 +676,18 @@ func UnmarshalCandidate(raw string) (Candidate, error) { uint16(component), //nolint:gosec // G115 no overflow we read 5 digits uint32(priority), //nolint:gosec // G115 no overflow we read 5 digits foundation, - parserExt.raddr, - parserExt.rport, - parserExt.extensions, + raddr, + rport, }) + if err != nil { + return nil, err + } + + candidate.setExtensions(extensions) + + return candidate, nil case "prflx": - return NewCandidatePeerReflexive(&CandidatePeerReflexiveConfig{ + candidate, err := NewCandidatePeerReflexive(&CandidatePeerReflexiveConfig{ "", protocol, address, @@ -697,12 +695,18 @@ func UnmarshalCandidate(raw string) (Candidate, error) { uint16(component), //nolint:gosec // G115 no overflow we read 5 digits uint32(priority), //nolint:gosec // G115 no overflow we read 5 digits foundation, - parserExt.raddr, - parserExt.rport, - parserExt.extensions, + raddr, + rport, }) + if err != nil { + return nil, err + } + + candidate.setExtensions(extensions) + + return candidate, nil case "relay": - return NewCandidateRelay(&CandidateRelayConfig{ + candidate, err := NewCandidateRelay(&CandidateRelayConfig{ "", protocol, address, @@ -710,16 +714,21 @@ func UnmarshalCandidate(raw string) (Candidate, error) { uint16(component), //nolint:gosec // G115 no overflow we read 5 digits uint32(priority), //nolint:gosec // G115 no overflow we read 5 digits foundation, - parserExt.raddr, - parserExt.rport, + raddr, + rport, "", nil, - parserExt.extensions, }) + if err != nil { + return nil, err + } + + candidate.setExtensions(extensions) + + return candidate, nil default: + return nil, fmt.Errorf("%w (%s)", ErrUnknownCandidateTyp, typ) } - - return nil, fmt.Errorf("%w (%s)", ErrUnknownCandidateTyp, typ) } // Read an ice-char token from the raw string @@ -746,29 +755,9 @@ func readCandidateCharToken(raw string, start int, limit int) (string, int, erro return raw[start:], len(raw), nil } -// Read a byte-string token from the raw string -// As defined in RFC 4566 1*(%x01-09/%x0B-0C/%x0E-FF) ;any byte except NUL, CR, or LF -// we imply that extensions byte-string are UTF-8 encoded -func readCandidateByteString(raw string, start int) (string, int, error) { - for i, char := range raw[start:] { - if char == 0x20 { // SP - return raw[start : start+i], start + i + 1, nil - } - - // 1*(%x01-09/%x0B-0C/%x0E-FF) - if !(char >= 0x01 && char <= 0x09 || - char >= 0x0B && char <= 0x0C || - char >= 0x0E && char <= 0xFF) { - return "", 0, fmt.Errorf("invalid byte-string character: %c", char) //nolint: err113 // handled by caller - } - } - - return raw[start:], len(raw), nil -} - // Read an ice string token from the raw string until a space is encountered // Or the end of the string, we imply that ice string are UTF-8 encoded -func readCandidatetringToken(raw string, start int) (string, int) { +func readCandidateStringToken(raw string, start int) (string, int) { for i, char := range raw[start:] { if char == 0x20 { // SP return raw[start : start+i], start + i + 1 @@ -815,70 +804,40 @@ func readCandidatePort(raw string, start int) (int, int, error) { return port, pos, nil } -type candidateParserExt struct { - raddr string - rport int - // while tcpType is part of the extensions, we cache it because it's required to create the candidate - tcpType string - - extensions []string -} - -// Read and validate ICE extensions from the raw string -func readCandidateExtensions(raw string, pos int) (candidateExt *candidateParserExt, err error) { - extensionStarted := false - candidateExt = &candidateParserExt{} - - var key, value string - - for pos < len(raw) { - key, pos, err = readCandidateByteString(raw, pos) - if err != nil { - return nil, fmt.Errorf("%w: %v", errParseExtension, err) //nolint:errorlint // we wrap the error - } - - if !extensionStarted && key == "raddr" { - if pos >= len(raw) { - return nil, fmt.Errorf("%w: expected raddr value in %s", errParseRelatedAddr, raw) - } - - candidateExt.raddr, pos = readCandidatetringToken(raw, pos) +// Read and validate raddr and rport from the raw string +// [SP rel-addr] [SP rel-port] +// defined in https://datatracker.ietf.org/doc/html/rfc5245#section-15.1 +// . +func tryReadRelativeAddrs(raw string, start int) (raddr string, rport, pos int, err error) { + key, pos := readCandidateStringToken(raw, start) - if pos >= len(raw) { - return nil, fmt.Errorf("%w: expected rport in %s", errParseRelatedAddr, raw) - } - - key, pos = readCandidatetringToken(raw, pos) - if key != "rport" { - return nil, fmt.Errorf("%w: expected rport in %s", errParseRelatedAddr, raw) - } - - if pos >= len(raw) { - return nil, fmt.Errorf("%w: expected rport value in %s", errParseRelatedAddr, raw) - } + if key != "raddr" { + return "", 0, start, nil + } - candidateExt.rport, pos, err = readCandidatePort(raw, pos) - if err != nil { - return nil, fmt.Errorf("%w: %v", errParseRelatedAddr, err) //nolint:errorlint // we wrap the error - } + if pos >= len(raw) { + return "", 0, 0, fmt.Errorf("%w: expected raddr value in %s", errParseRelatedAddr, raw) + } - continue - } + raddr, pos = readCandidateStringToken(raw, pos) - // (SP extension-att-name SP extension-att-value) - extensionStarted = true + if pos >= len(raw) { + return "", 0, 0, fmt.Errorf("%w: expected rport in %s", errParseRelatedAddr, raw) + } - value, pos, err = readCandidateByteString(raw, pos) - if err != nil { - return nil, fmt.Errorf("%w: %v", errParseExtension, err) //nolint:errorlint // we wrap the error - } + key, pos = readCandidateStringToken(raw, pos) + if key != "rport" { + return "", 0, 0, fmt.Errorf("%w: expected rport in %s", errParseRelatedAddr, raw) + } - if key == "tcptype" { - candidateExt.tcpType = value - } + if pos >= len(raw) { + return "", 0, 0, fmt.Errorf("%w: expected rport value in %s", errParseRelatedAddr, raw) + } - candidateExt.extensions = append(candidateExt.extensions, key, value) + rport, pos, err = readCandidatePort(raw, pos) + if err != nil { + return "", 0, 0, fmt.Errorf("%w: %v", errParseRelatedAddr, err) //nolint:errorlint // we wrap the error } - return candidateExt, nil + return raddr, rport, pos, nil } diff --git a/candidate_host.go b/candidate_host.go index 60797fa2..c14b6a7c 100644 --- a/candidate_host.go +++ b/candidate_host.go @@ -26,7 +26,6 @@ type CandidateHostConfig struct { Foundation string TCPType TCPType IsLocationTracked bool - Extensions []string } // NewCandidateHost creates a new host candidate @@ -49,7 +48,6 @@ func NewCandidateHost(config *CandidateHostConfig) (*CandidateHost, error) { priorityOverride: config.Priority, remoteCandidateCaches: map[AddrPort]Candidate{}, isLocationTracked: config.IsLocationTracked, - extensions: config.Extensions, }, network: config.Network, } diff --git a/candidate_peer_reflexive.go b/candidate_peer_reflexive.go index 87fab481..b28e9a76 100644 --- a/candidate_peer_reflexive.go +++ b/candidate_peer_reflexive.go @@ -26,7 +26,6 @@ type CandidatePeerReflexiveConfig struct { Foundation string RelAddr string RelPort int - Extensions []string } // NewCandidatePeerReflexive creates a new peer reflective candidate @@ -62,7 +61,6 @@ func NewCandidatePeerReflexive(config *CandidatePeerReflexiveConfig) (*Candidate Port: config.RelPort, }, remoteCandidateCaches: map[AddrPort]Candidate{}, - extensions: config.Extensions, }, }, nil } diff --git a/candidate_relay.go b/candidate_relay.go index 56ac1e62..faf281b0 100644 --- a/candidate_relay.go +++ b/candidate_relay.go @@ -29,7 +29,6 @@ type CandidateRelayConfig struct { RelPort int RelayProtocol string OnClose func() error - Extensions []string } // NewCandidateRelay creates a new relay candidate @@ -70,7 +69,6 @@ func NewCandidateRelay(config *CandidateRelayConfig) (*CandidateRelay, error) { Port: config.RelPort, }, remoteCandidateCaches: map[AddrPort]Candidate{}, - extensions: config.Extensions, }, relayProtocol: config.RelayProtocol, onClose: config.OnClose, diff --git a/candidate_server_reflexive.go b/candidate_server_reflexive.go index d34533c0..85d613e2 100644 --- a/candidate_server_reflexive.go +++ b/candidate_server_reflexive.go @@ -24,7 +24,6 @@ type CandidateServerReflexiveConfig struct { Foundation string RelAddr string RelPort int - Extensions []string } // NewCandidateServerReflexive creates a new server reflective candidate @@ -64,7 +63,6 @@ func NewCandidateServerReflexive(config *CandidateServerReflexiveConfig) (*Candi Port: config.RelPort, }, remoteCandidateCaches: map[AddrPort]Candidate{}, - extensions: config.Extensions, }, }, nil } diff --git a/candidate_test.go b/candidate_test.go index c6be0f89..4fc4e904 100644 --- a/candidate_test.go +++ b/candidate_test.go @@ -274,6 +274,19 @@ func mustCandidateHost(conf *CandidateHostConfig) Candidate { return cand } +func mustCandidateHostWithExtensions(t *testing.T, conf *CandidateHostConfig, extensions *CandidateExtensions) Candidate { + t.Helper() + + cand, err := NewCandidateHost(conf) + if err != nil { + panic(err) + } + + cand.setExtensions(extensions) + + return cand +} + func mustCandidateRelay(conf *CandidateRelayConfig) Candidate { cand, err := NewCandidateRelay(conf) if err != nil { @@ -282,6 +295,19 @@ func mustCandidateRelay(conf *CandidateRelayConfig) Candidate { return cand } +func mustCandidateRelayWithExtensions(t *testing.T, conf *CandidateRelayConfig, extensions *CandidateExtensions) Candidate { + t.Helper() + + cand, err := NewCandidateRelay(conf) + if err != nil { + panic(err) + } + + cand.setExtensions(extensions) + + return cand +} + func mustCandidateServerReflexive(conf *CandidateServerReflexiveConfig) Candidate { cand, err := NewCandidateServerReflexive(conf) if err != nil { @@ -290,7 +316,20 @@ func mustCandidateServerReflexive(conf *CandidateServerReflexiveConfig) Candidat return cand } -func mustCandidatePeerReflexive(t *testing.T, conf *CandidatePeerReflexiveConfig) Candidate { +func mustCandidateServerReflexiveWithExtensions(t *testing.T, conf *CandidateServerReflexiveConfig, extensions *CandidateExtensions) Candidate { + t.Helper() + + cand, err := NewCandidateServerReflexive(conf) + if err != nil { + panic(err) + } + + cand.setExtensions(extensions) + + return cand +} + +func mustCandidatePeerReflexiveWithExtensions(t *testing.T, conf *CandidatePeerReflexiveConfig, extensions *CandidateExtensions) Candidate { t.Helper() cand, err := NewCandidatePeerReflexive(conf) @@ -298,6 +337,8 @@ func mustCandidatePeerReflexive(t *testing.T, conf *CandidatePeerReflexiveConfig panic(err) } + cand.setExtensions(extensions) + return cand } @@ -339,21 +380,21 @@ func TestCandidateMarshal(t *testing.T) { false, }, { - mustCandidatePeerReflexive(t, &CandidatePeerReflexiveConfig{ - Network: NetworkTypeTCP4.String(), - Address: "192.0.2.15", - Port: 50000, - RelAddr: "10.0.0.1", - RelPort: 12345, - Extensions: []string{ - "generation", - "0", - "network-id", - "2", - "network-cost", - "10", + mustCandidatePeerReflexiveWithExtensions( + t, + &CandidatePeerReflexiveConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.0.2.15", + Port: 50000, + RelAddr: "10.0.0.1", + RelPort: 12345, }, - }), + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }}, + ), "4207374052 1 tcp 1685790463 192.0.2.15 50000 typ prflx raddr 10.0.0.1 rport 12345 generation 0 network-id 2 network-cost 10", false, }, @@ -436,6 +477,9 @@ func TestCandidateMarshal(t *testing.T) { {nil, "4207374051 1 INVALID 2130706431 10.0.75.1 53634 typ host", true}, {nil, "4207374051 1 INVALID 2130706431 10.0.75.1 53634 typ", true}, {nil, "4207374051 1 INVALID 2130706431 10.0.75.1 53634", true}, + {nil, "848194626 1 udp 16777215 50.0.0.^^1 5000 typ relay raddr 192.168.0.1 rport 5001", true}, + {nil, "4207374052 1 tcp 1685790463 192.0#.2.15 50000 typ prflx raddr 10.0.0.1 rport 12345 rport 5001", true}, + {nil, "647372371 1 udp 1694498815 191.228.2@338.68 53991 typ srflx raddr 192.168.0.274 rport 53991", true}, // invalid foundion; longer than 32 characters {nil, "111111111111111111111111111111111 1 udp 500 " + localhostIPStr + " 80 typ host", true}, // Invalid ice-char @@ -450,12 +494,16 @@ func TestCandidateMarshal(t *testing.T) { {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 999999", true}, // bad byte-string in extension value + {nil, "750 1 udp 500 fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a 53987 typ host ext valu\nu", true}, {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 654 ext valu\nu", true}, {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 654 ext valu\000e", true}, // bad byte-string in extension key {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 654 ext\r value", true}, + // invalid tcptype + {nil, "1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype INVALID", true}, + // expect rport after raddr {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 extension 322", true}, {nil, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport", true}, @@ -555,55 +603,75 @@ func TestMarshalUnmarshalCandidateWithZoneID(t *testing.T) { func TestCandidateExtensionsMarshal(t *testing.T) { testCases := []struct { - Extensions []string + Extensions CandidateExtensions candidate string }{ { - []string{ - "generation", "0", - "ufrag", "QNvE", - "network-id", "4", + CandidateExtensions{ + []CandidateExtension{ + {"generation", "0"}, + {"ufrag", "QNvE"}, + {"network-id", "4"}, + }, }, "1299692247 1 udp 2122134271 fdc8:cc8:c835:e400:343c:feb:32c8:17b9 58240 typ host generation 0 ufrag QNvE network-id 4", }, { - []string{ - "generation", "1", - "network-id", "2", - "network-cost", "50", + CandidateExtensions{ + []CandidateExtension{ + {"generation", "1"}, + {"network-id", "2"}, + {"network-cost", "50"}, + }, }, "647372371 1 udp 1694498815 191.228.238.68 53991 typ srflx raddr 192.168.0.274 rport 53991 generation 1 network-id 2 network-cost 50", }, { - []string{ - "generation", "0", - "network-id", "1", - "network-cost", "20", - "ufrag", "frag42abcdef", - "password", "abc123exp123", + CandidateExtensions{ + []CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }, + }, + "4207374052 1 tcp 1685790463 192.0.2.15 50000 typ prflx raddr 10.0.0.1 rport 12345 generation 0 network-id 2 network-cost 10", + }, + { + CandidateExtensions{ + []CandidateExtension{ + {"generation", "0"}, + {"network-id", "1"}, + {"network-cost", "20"}, + {"ufrag", "frag42abcdef"}, + {"password", "abc123exp123"}, + }, }, "848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 5001 generation 0 network-id 1 network-cost 20 ufrag frag42abcdef password abc123exp123", }, { - []string{ - "tcptype", "active", - "generation", "0", + CandidateExtensions{ + []CandidateExtension{ + {"tcptype", "active"}, + {"generation", "0"}, + }, }, "1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype active generation 0", }, { - []string{ - "tcptype", "active", - "generation", "0", + CandidateExtensions{ + []CandidateExtension{ + {"tcptype", "active"}, + {"generation", "0"}, + }, }, "1052353102 1 tcp 2128609279 192.168.0.196 0 typ host tcptype active generation 0", }, { - []string{}, + CandidateExtensions{}, "1052353102 1 tcp 2128609279 192.168.0.196 0 typ host", }, { - []string{}, + CandidateExtensions{}, "1052353102 1 tcp 2128609279 192.168.0.196 0 typ host", }, } @@ -611,17 +679,17 @@ func TestCandidateExtensionsMarshal(t *testing.T) { for _, tc := range testCases { candidate, err := UnmarshalCandidate(tc.candidate) require.NoError(t, err) - require.Equal(t, tc.Extensions, candidate.Extensions()) + require.Equal(t, tc.Extensions.Equal(candidate.Extensions()), true, "Extensions should be equal", tc.candidate) valueStr := candidate.Marshal() candidate2, err := UnmarshalCandidate(valueStr) require.NoError(t, err) - require.Equal(t, tc.Extensions, candidate2.Extensions(), "Marshal() should preserve extensions") + require.Equal(t, tc.Extensions.Equal(candidate2.Extensions()), true, "Marshal() should preserve extensions") } } -func TestCandidateExtensionsEqual(t *testing.T) { +func TestCandidateExtensionsDeepEqual(t *testing.T) { noExt, err := UnmarshalCandidate("750 0 udp 500 fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a 53987 typ host") require.NoError(t, err) @@ -629,13 +697,12 @@ func TestCandidateExtensionsEqual(t *testing.T) { ufrag := "QNvE" networkID := "4" - extensions := []string{ - "generation", - generation, - "ufrag", - ufrag, - "network-id", - networkID, + extensions := &CandidateExtensions{ + []CandidateExtension{ + {"generation", generation}, + {"ufrag", ufrag}, + {"network-id", networkID}, + }, } candidate, err := UnmarshalCandidate( @@ -666,135 +733,273 @@ func TestCandidateExtensionsEqual(t *testing.T) { true, }, { - mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeUDP4.String(), - Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", - Port: 53987, - Priority: 500, - Foundation: "750", - Extensions: []string{}, - }), + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeUDP4.String(), + Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", + Port: 53987, + Priority: 500, + Foundation: "750", + }, + &CandidateExtensions{[]CandidateExtension{}}, + ), noExt, true, }, { - mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeUDP4.String(), - Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", - Port: 53987, - Priority: 500, - Foundation: "750", - Extensions: extensions, - }), + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeUDP4.String(), + Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", + Port: 53987, + Priority: 500, + Foundation: "750", + }, + extensions, + ), candidate, true, }, { - mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeUDP4.String(), - Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", - Port: 53987, - Priority: 500, - Foundation: "750", - Extensions: []string{ - "generation", - "5", - "ufrag", - ufrag, - "network-id", - networkID, + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.196", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, }, - }), - candidate, - false, + &CandidateExtensions{[]CandidateExtension{ + {"tcptype", TCPTypeActive.String()}, + {"generation", "0"}, + }}, + ), + candidate2, + true, }, { - mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeTCP4.String(), - Address: "192.168.0.196", - Port: 0, - Priority: 2128609279, - Foundation: "1052353102", - TCPType: TCPTypeActive, - Extensions: []string{ - "tcptype", - TCPTypeActive.String(), - "generation", - "0", + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.196", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, }, - }), + &CandidateExtensions{[]CandidateExtension{ + {"tcptype", TCPTypeActive.String()}, + {"generation", "0"}, + }}, + ), candidate2, true, }, { - mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeTCP4.String(), - Address: "192.168.0.196", - Port: 0, - Priority: 2128609279, - Foundation: "1052353102", - TCPType: TCPTypeActive, - Extensions: []string{ - "tcptype", - TCPTypeActive.String(), - "generation", - "0", - "INVALID", // non-even number of elements should be ignored + mustCandidateRelayWithExtensions( + t, + &CandidateRelayConfig{ + Network: NetworkTypeUDP4.String(), + Address: "10.0.0.10", + Port: 5000, + RelAddr: "10.0.0.2", + RelPort: 5001, }, - }), - candidate2, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "1"}, + }}, + ), + mustCandidateRelayWithExtensions( + t, + &CandidateRelayConfig{ + Network: NetworkTypeUDP4.String(), + Address: "10.0.0.10", + Port: 5000, + RelAddr: "10.0.0.2", + RelPort: 5001, + }, + &CandidateExtensions{[]CandidateExtension{ + {"network-id", "1"}, + {"generation", "0"}, + }}, + ), + true, + }, + { + mustCandidatePeerReflexiveWithExtensions( + t, + &CandidatePeerReflexiveConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.0.2.15", + Port: 50000, + RelAddr: "10.0.0.1", + RelPort: 12345, + }, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }}, + ), + mustCandidatePeerReflexiveWithExtensions( + t, + &CandidatePeerReflexiveConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.0.2.15", + Port: 50000, + RelAddr: "10.0.0.1", + RelPort: 12345, + }, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }}, + ), + true, + }, + { + mustCandidateServerReflexiveWithExtensions( + t, + &CandidateServerReflexiveConfig{ + Network: NetworkTypeUDP4.String(), + Address: "191.228.238.68", + Port: 53991, + RelAddr: "192.168.0.274", + RelPort: 53991, + }, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }}, + ), + mustCandidateServerReflexiveWithExtensions( + t, + &CandidateServerReflexiveConfig{ + Network: NetworkTypeUDP4.String(), + Address: "191.228.238.68", + Port: 53991, + RelAddr: "192.168.0.274", + RelPort: 53991, + }, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "0"}, + {"network-id", "2"}, + {"network-cost", "10"}, + }}, + ), true, }, + { + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeUDP4.String(), + Address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", + Port: 53987, + Priority: 500, + Foundation: "750", + }, + &CandidateExtensions{[]CandidateExtension{ + {"generation", "5"}, + {"ufrag", ufrag}, + {"network-id", networkID}, + }}, + ), + candidate, + false, + }, + { + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.196", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, + }, + &CandidateExtensions{[]CandidateExtension{ + {"tcptype", TCPTypeActive.String()}, + {"generation", "0"}, + }}, + ), + mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.197", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, + }, + &CandidateExtensions{[]CandidateExtension{ + {"tcptype", TCPTypeActive.String()}, + {"generation", "0"}, + }}, + ), + false, + }, } for _, tc := range testCases { - require.Equal(t, tc.equal, tc.a.Equal(tc.b), "a: %s, b: %s", tc.a.String(), tc.b.String()) + require.Equal(t, tc.a.DeepEqual(tc.b), tc.equal, "a: %s, b: %s", tc.a.Marshal(), tc.b.Marshal()) } } func TestCandidateGetExtension(t *testing.T) { t.Run("valid", func(t *testing.T) { - candidate := mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeTCP4.String(), - Address: "192.168.0.196", - Port: 0, - Priority: 2128609279, - Foundation: "1052353102", - TCPType: TCPTypeActive, - Extensions: []string{ - "tcptype", - TCPTypeActive.String(), - "generation", - "0", - "INVALID", // non-even number of elements should be ignored + candidate := mustCandidateHostWithExtensions( + t, + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.196", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, }, - }) + &CandidateExtensions{[]CandidateExtension{ + {"tcptype", TCPTypeActive.String()}, + {"generation", "0"}, + }}, + ) - value, ok := candidate.GetExtension("tcptype") + value, ok := candidate.Extensions().Get("tcptype") require.Equal(t, TCPTypeActive.String(), value) require.True(t, ok) - value, ok = candidate.GetExtension("generation") + value, ok = candidate.Extensions().Get("generation") require.Equal(t, "0", value) require.True(t, ok) - value, ok = candidate.GetExtension("INVALID") + value, ok = candidate.Extensions().Get("INVALID") require.Equal(t, "", value) require.False(t, ok) }) t.Run("tcptype value no extenstion", func(t *testing.T) { - candidate := mustCandidateHost(&CandidateHostConfig{ - Network: NetworkTypeTCP4.String(), - Address: "192.168.0.196", - Port: 0, - Priority: 2128609279, - Foundation: "1052353102", - TCPType: TCPTypeActive, - Extensions: []string{}, - }) + candidate := mustCandidateHost( + &CandidateHostConfig{ + Network: NetworkTypeTCP4.String(), + Address: "192.168.0.196", + Port: 0, + Priority: 2128609279, + Foundation: "1052353102", + TCPType: TCPTypeActive, + }, + ) - value, ok := candidate.GetExtension("tcptype") + value, ok := candidate.Extensions().Get("tcptype") require.Equal(t, TCPTypeActive.String(), value) require.True(t, ok) }) diff --git a/gather.go b/gather.go index f6be3ede..99b5c069 100644 --- a/gather.go +++ b/gather.go @@ -296,7 +296,7 @@ func (a *Agent) gatherCandidatesLocalUDPMux(ctx context.Context) error { //nolin } localAddresses := a.udpMux.GetListenAddresses() - existingConfigs := make(map[string]struct{}) + existingConfigs := make(map[CandidateHostConfig]struct{}) for _, addr := range localAddresses { udpAddr, ok := addr.(*net.UDPAddr) @@ -342,13 +342,12 @@ func (a *Agent) gatherCandidatesLocalUDPMux(ctx context.Context) error { //nolin Component: ComponentRTP, IsLocationTracked: isLocationTracked, } - hostConfigStr := fmt.Sprintf("%#v", hostConfig) // Detect a duplicate candidate before calling addCandidate(). // otherwise, addCandidate() detects the duplicate candidate // and close its connection, invalidating all candidates // that share the same connection. - if _, ok := existingConfigs[hostConfigStr]; ok { + if _, ok := existingConfigs[hostConfig]; ok { continue } @@ -372,7 +371,7 @@ func (a *Agent) gatherCandidatesLocalUDPMux(ctx context.Context) error { //nolin continue } - existingConfigs[hostConfigStr] = struct{}{} + existingConfigs[hostConfig] = struct{}{} } return nil