diff --git a/.golangci.yml b/.golangci.yml index 570f17b..4213697 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,6 +3,80 @@ linters-settings: check-shadowing: true misspell: locale: US + exhaustive: + default-signifies-exhaustive: true + +linters: + enable: + - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers + - bodyclose # checks whether HTTP response body is closed successfully + - deadcode # Finds unused code + - depguard # Go linter that checks if package imports are in a list of acceptable packages + - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + - dupl # Tool for code clone detection + - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases + - exhaustive # check exhaustiveness of enum switch statements + - exportloopref # checks for pointers to enclosing loop variables + - gci # Gci control golang package import order and make it always deterministic. + - gochecknoglobals # Checks that no globals are present in Go code + - gochecknoinits # Checks that no init functions are present in Go code + - gocognit # Computes and checks the cognitive complexity of functions + - goconst # Finds repeated strings that could be replaced by a constant + - gocritic # The most opinionated Go source code linter + - godox # Tool for detection of FIXME, TODO and other comment keywords + - goerr113 # Golang linter to check the errors handling expressions + - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification + - gofumpt # Gofumpt checks whether code was gofumpt-ed. + - goheader # Checks is file header matches to pattern + - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports + - golint # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes + - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + - goprintffuncname # Checks that printf-like functions are named with `f` at the end + - gosec # Inspects source code for security problems + - gosimple # Linter for Go source code that specializes in simplifying a code + - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # Detects when assignments to existing variables are not used + - misspell # Finds commonly misspelled English words in comments + - nakedret # Finds naked returns in functions greater than a specified function length + - noctx # noctx finds sending http request without context.Context + - scopelint # Scopelint checks for unpinned variables in go programs + - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks + - structcheck # Finds unused struct fields + - stylecheck # Stylecheck is a replacement for golint + - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code + - unconvert # Remove unnecessary type conversions + - unparam # Reports unused function parameters + - unused # Checks Go code for unused constants, variables, functions and types + - varcheck # Finds unused global variables and constants + - whitespace # Tool for detection of leading and trailing whitespace + disable: + - funlen # Tool for detection of long functions + - gocyclo # Computes and checks the cyclomatic complexity of functions + - godot # Check if comments end in a period + - gomnd # An analyzer to detect magic numbers. + - lll # Reports long lines + - maligned # Tool to detect Go structs that would take less memory if their fields were sorted + - nestif # Reports deeply nested if statements + - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity + - nolintlint # Reports ill-formed or insufficient nolint directives + - prealloc # Finds slice declarations that could potentially be preallocated + - rowserrcheck # checks whether Err of rows is checked successfully + - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. + - testpackage # linter that makes you use a separate _test package + - wsl # Whitespace Linter - Forces you to use empty lines! + +issues: + exclude-rules: + # Allow complex tests, better to be self contained + - path: _test\.go + linters: + - gocognit + + # Allow complex main function in examples + - path: examples + text: "of func `main` is high" + linters: + - gocognit run: skip-dirs-use-default: false diff --git a/README.md b/README.md index f754631..dcb8853 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu * [Sidney San Martín](https://github.com/s4y) * [JooYoung Lim](https://github.com/DevRockstarZ) * [Kory Miller](https://github.com/korymiller1489) +* [ZHENK](https://github.com/scorpionknifes) ### License MIT License - see [LICENSE](LICENSE) for full text diff --git a/agent.go b/agent.go index a81db93..c9788b1 100644 --- a/agent.go +++ b/agent.go @@ -224,7 +224,7 @@ func (a *Agent) taskLoop() { } // NewAgent creates a new Agent -func NewAgent(config *AgentConfig) (*Agent, error) { +func NewAgent(config *AgentConfig) (*Agent, error) { //nolint:gocognit var err error if config.PortMax < config.PortMin { return nil, ErrPort @@ -748,7 +748,7 @@ func (a *Agent) resolveAndAddMulticastCandidate(c *CandidateHost) { return } - ip, _, _, _ := parseAddr(src) + ip, _, _, _ := parseAddr(src) //nolint:dogsled if ip == nil { a.log.Warnf("Failed to discover mDNS candidate %s: failed to parse IP", c.Address()) return @@ -1009,7 +1009,7 @@ func (a *Agent) handleInboundBindingSuccess(id [stun.TransactionIDSize]byte) (bo } // handleInbound processes STUN traffic from a remote candidate -func (a *Agent) handleInbound(m *stun.Message, local Candidate, remote net.Addr) { +func (a *Agent) handleInbound(m *stun.Message, local Candidate, remote net.Addr) { //nolint:gocognit var err error if m == nil || local == nil { return @@ -1074,7 +1074,6 @@ func (a *Agent) handleInbound(m *stun.Message, local Candidate, remote net.Addr) Component: local.Component(), RelAddr: "", RelPort: 0, - // TODO set TCPType } prflxCandidate, err := NewCandidatePeerReflexive(&prflxCandidateConfig) diff --git a/agent_config.go b/agent_config.go index 6003026..1523d12 100644 --- a/agent_config.go +++ b/agent_config.go @@ -42,9 +42,9 @@ const ( maxBindingRequestTimeout = 4000 * time.Millisecond ) -var ( - defaultCandidateTypes = []CandidateType{CandidateTypeHost, CandidateTypeServerReflexive, CandidateTypeRelay} -) +func defaultCandidateTypes() []CandidateType { + return []CandidateType{CandidateTypeHost, CandidateTypeServerReflexive, CandidateTypeRelay} +} // AgentConfig collects the arguments to ice.Agent construction into // a single structure, for future-proofness of the interface @@ -202,7 +202,7 @@ func (config *AgentConfig) initWithDefaults(a *Agent) { } if config.CandidateTypes == nil || len(config.CandidateTypes) == 0 { - a.candidateTypes = defaultCandidateTypes + a.candidateTypes = defaultCandidateTypes() } else { a.candidateTypes = config.CandidateTypes } diff --git a/agent_test.go b/agent_test.go index 966c71e..39da38c 100644 --- a/agent_test.go +++ b/agent_test.go @@ -4,6 +4,7 @@ package ice import ( "context" + "errors" "net" "strconv" "sync" @@ -39,7 +40,6 @@ func TestPairSearch(t *testing.T) { var config AgentConfig a, err := NewAgent(&config) - if err != nil { t.Fatalf("Error constructing ice.Agent") } @@ -208,13 +208,13 @@ type BadAddr struct{} func (ba *BadAddr) Network() string { return "xxx" } + func (ba *BadAddr) String() string { return "yyy" } func runAgentTest(t *testing.T, config *AgentConfig, task func(ctx context.Context, a *Agent)) { a, err := NewAgent(config) - if err != nil { t.Fatalf("Error constructing ice.Agent") } @@ -255,7 +255,7 @@ func TestHandlePeerReflexive(t *testing.T) { msg, err := stun.Build(stun.BindingRequest, stun.TransactionID, stun.NewUsername(a.localUfrag+":"+a.remoteUfrag), - UseCandidate, + UseCandidate(), AttrControlling(a.tieBreaker), PriorityAttr(local.Priority()), stun.NewShortTermIntegrity(a.localPwd), @@ -325,7 +325,7 @@ func TestHandlePeerReflexive(t *testing.T) { runAgentTest(t, &config, func(ctx context.Context, a *Agent) { a.selector = &controllingSelector{agent: a, log: a.log} tID := [stun.TransactionIDSize]byte{} - copy(tID[:], []byte("ABC")) + copy(tID[:], "ABC") a.pendingBindingRequests = []bindingRequest{ {time.Now(), tID, &net.UDPAddr{}, false}, } @@ -392,7 +392,7 @@ func TestConnectivityOnStartup(t *testing.T) { KeepaliveInterval := time.Hour cfg0 := &AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net0, @@ -405,7 +405,7 @@ func TestConnectivityOnStartup(t *testing.T) { require.NoError(t, aAgent.OnConnectionStateChange(aNotifier)) cfg1 := &AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net1, KeepaliveInterval: &KeepaliveInterval, @@ -498,7 +498,7 @@ func TestConnectivityLite(t *testing.T) { cfg0 := &AgentConfig{ Urls: []*URL{stunServerURL}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: v.net0, } @@ -511,7 +511,7 @@ func TestConnectivityLite(t *testing.T) { Urls: []*URL{}, Lite: true, CandidateTypes: []CandidateType{CandidateTypeHost}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: v.net1, } @@ -666,7 +666,7 @@ func TestInboundValidity(t *testing.T) { remote := &net.UDPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999} tID := [stun.TransactionIDSize]byte{} - copy(tID[:], []byte("ABC")) + copy(tID[:], "ABC") msg, err := stun.Build(stun.BindingSuccess, stun.NewTransactionIDSetter(tID), stun.NewShortTermIntegrity(a.remotePwd), stun.Fingerprint, @@ -693,19 +693,19 @@ func TestInvalidAgentStarts(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) defer cancel() - if _, err = a.Dial(ctx, "", "bar"); err != nil && err != ErrRemoteUfragEmpty { + if _, err = a.Dial(ctx, "", "bar"); err != nil && !errors.Is(err, ErrRemoteUfragEmpty) { t.Fatal(err) } - if _, err = a.Dial(ctx, "foo", ""); err != nil && err != ErrRemotePwdEmpty { + if _, err = a.Dial(ctx, "foo", ""); err != nil && !errors.Is(err, ErrRemotePwdEmpty) { t.Fatal(err) } - if _, err = a.Dial(ctx, "foo", "bar"); err != nil && err != ErrCanceledByCaller { + if _, err = a.Dial(ctx, "foo", "bar"); err != nil && !errors.Is(err, ErrCanceledByCaller) { t.Fatal(err) } - if _, err = a.Dial(context.TODO(), "foo", "bar"); err != nil && err != ErrMultipleStart { + if _, err = a.Dial(context.TODO(), "foo", "bar"); err != nil && !errors.Is(err, ErrMultipleStart) { t.Fatal(err) } @@ -726,7 +726,7 @@ func TestConnectionStateCallback(t *testing.T) { cfg := &AgentConfig{ Urls: []*URL{}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), DisconnectedTimeout: &disconnectedDuration, FailedTimeout: &failedDuration, KeepaliveInterval: &KeepaliveInterval, @@ -786,7 +786,7 @@ func TestInvalidGather(t *testing.T) { } err = a.GatherCandidates() - if err != ErrNoOnCandidateHandler { + if !errors.Is(err, ErrNoOnCandidateHandler) { t.Fatal("trickle GatherCandidates succeeded without OnCandidate") } assert.NoError(t, a.Close()) @@ -1161,7 +1161,7 @@ func TestInitExtIPMapping(t *testing.T) { NAT1To1IPCandidateType: CandidateTypeHost, CandidateTypes: []CandidateType{CandidateTypeRelay}, }) - if err != ErrIneffectiveNAT1To1IPMappingHost { + if !errors.Is(err, ErrIneffectiveNAT1To1IPMappingHost) { t.Fatalf("Unexpected error: %v", err) } @@ -1172,7 +1172,7 @@ func TestInitExtIPMapping(t *testing.T) { NAT1To1IPCandidateType: CandidateTypeServerReflexive, CandidateTypes: []CandidateType{CandidateTypeRelay}, }) - if err != ErrIneffectiveNAT1To1IPMappingSrflx { + if !errors.Is(err, ErrIneffectiveNAT1To1IPMappingSrflx) { t.Fatalf("Unexpected error: %v", err) } @@ -1183,7 +1183,7 @@ func TestInitExtIPMapping(t *testing.T) { NAT1To1IPCandidateType: CandidateTypeHost, MulticastDNSMode: MulticastDNSModeQueryAndGather, }) - if err != ErrMulticastDNSWithNAT1To1IPMapping { + if !errors.Is(err, ErrMulticastDNSWithNAT1To1IPMapping) { t.Fatalf("Unexpected error: %v", err) } @@ -1192,7 +1192,7 @@ func TestInitExtIPMapping(t *testing.T) { NAT1To1IPs: []string{"bad.2.3.4"}, // bad IP NAT1To1IPCandidateType: CandidateTypeHost, }) - if err != ErrInvalidNAT1To1IPMapping { + if !errors.Is(err, ErrInvalidNAT1To1IPMapping) { t.Fatalf("Unexpected error: %v", err) } } @@ -1269,7 +1269,7 @@ func TestConnectionStateFailedDeleteAllCandidates(t *testing.T) { KeepaliveInterval := time.Duration(0) cfg := &AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), DisconnectedTimeout: &oneSecond, FailedTimeout: &oneSecond, KeepaliveInterval: &KeepaliveInterval, @@ -1496,7 +1496,7 @@ func TestCloseInConnectionStateCallback(t *testing.T) { cfg := &AgentConfig{ Urls: []*URL{}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), DisconnectedTimeout: &disconnectedDuration, FailedTimeout: &failedDuration, KeepaliveInterval: &KeepaliveInterval, @@ -1547,7 +1547,7 @@ func TestRunTaskInConnectionStateCallback(t *testing.T) { cfg := &AgentConfig{ Urls: []*URL{}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), DisconnectedTimeout: &oneSecond, FailedTimeout: &oneSecond, KeepaliveInterval: &KeepaliveInterval, @@ -1591,7 +1591,7 @@ func TestRunTaskInSelectedCandidatePairChangeCallback(t *testing.T) { cfg := &AgentConfig{ Urls: []*URL{}, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), DisconnectedTimeout: &oneSecond, FailedTimeout: &oneSecond, KeepaliveInterval: &KeepaliveInterval, diff --git a/candidate_base.go b/candidate_base.go index 93c4d64..ca00180 100644 --- a/candidate_base.go +++ b/candidate_base.go @@ -156,6 +156,8 @@ func (c *candidateBase) LocalPreference() uint16 { return 4 case TCPTypeSimultaneousOpen: return 2 + case TCPTypeUnspecified: + return 0 } case CandidateTypePeerReflexive, CandidateTypeServerReflexive: switch c.tcpType { @@ -165,9 +167,12 @@ func (c *candidateBase) LocalPreference() uint16 { return 4 case TCPTypePassive: return 2 + case TCPTypeUnspecified: + return 0 } + case CandidateTypeUnspecified: + return 0 } - return 0 }() @@ -295,7 +300,7 @@ func (c *candidateBase) close() error { func (c *candidateBase) writeTo(raw []byte, dst Candidate) (int, error) { n, err := c.conn.WriteTo(raw, dst.addr()) if err != nil { - return n, fmt.Errorf("failed to send packet: %v", err) + return n, fmt.Errorf("%w: %v", errSendPacket, err) } c.seen(true) return n, nil @@ -410,7 +415,7 @@ func (c candidateBase) Marshal() string { func UnmarshalCandidate(raw string) (Candidate, error) { split := strings.Fields(raw) if len(split) < 8 { - return nil, fmt.Errorf("attribute not long enough to be ICE candidate (%d)", len(split)) + return nil, fmt.Errorf("%w (%d)", errAttributeTooShortICECandidate, len(split)) } // Foundation @@ -419,7 +424,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { // Component rawComponent, err := strconv.ParseUint(split[1], 10, 16) if err != nil { - return nil, fmt.Errorf("could not parse component: %v", err) + return nil, fmt.Errorf("%w: %v", errParseComponent, err) } component := uint16(rawComponent) @@ -429,7 +434,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { // Priority priorityRaw, err := strconv.ParseUint(split[3], 10, 32) if err != nil { - return nil, fmt.Errorf("could not parse priority: %v", err) + return nil, fmt.Errorf("%w: %v", errParsePriority, err) } priority := uint32(priorityRaw) @@ -439,7 +444,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { // Port rawPort, err := strconv.ParseUint(split[5], 10, 16) if err != nil { - return nil, fmt.Errorf("could not parse port: %v", err) + return nil, fmt.Errorf("%w: %v", errParsePort, err) } port := int(rawPort) typ := split[7] @@ -453,7 +458,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) { if split[0] == "raddr" { if len(split) < 4 { - return nil, fmt.Errorf("could not parse related addresses: incorrect length") + return nil, fmt.Errorf("%w: incorrect length", errParseRelatedAddr) } // RelatedAddress @@ -462,12 +467,12 @@ func UnmarshalCandidate(raw string) (Candidate, error) { // RelatedPort rawRelatedPort, parseErr := strconv.ParseUint(split[3], 10, 16) if parseErr != nil { - return nil, fmt.Errorf("could not parse port: %v", parseErr) + return nil, fmt.Errorf("%w: %v", errParsePort, parseErr) } relatedPort = int(rawRelatedPort) } else if split[0] == "tcptype" { if len(split) < 2 { - return nil, fmt.Errorf("could not parse typtype: incorrect length") + return nil, fmt.Errorf("%w: incorrect length", errParseTypType) } tcpType = NewTCPType(split[1]) @@ -476,15 +481,15 @@ func UnmarshalCandidate(raw string) (Candidate, error) { switch typ { case "host": - return NewCandidateHost(&CandidateHostConfig{"", protocol, address, port, uint16(component), priority, foundation, tcpType}) + return NewCandidateHost(&CandidateHostConfig{"", protocol, address, port, component, priority, foundation, tcpType}) case "srflx": - return NewCandidateServerReflexive(&CandidateServerReflexiveConfig{"", protocol, address, port, uint16(component), priority, foundation, relatedAddress, relatedPort}) + return NewCandidateServerReflexive(&CandidateServerReflexiveConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort}) case "prflx": - return NewCandidatePeerReflexive(&CandidatePeerReflexiveConfig{"", protocol, address, port, uint16(component), priority, foundation, relatedAddress, relatedPort}) + return NewCandidatePeerReflexive(&CandidatePeerReflexiveConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort}) case "relay": - return NewCandidateRelay(&CandidateRelayConfig{"", protocol, address, port, uint16(component), priority, foundation, relatedAddress, relatedPort, nil}) + return NewCandidateRelay(&CandidateRelayConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort, nil}) default: } - return nil, fmt.Errorf("Unknown candidate typ(%s)", typ) + return nil, fmt.Errorf("%w (%s)", errUnknownCandidateTyp, typ) } diff --git a/candidate_peer_reflexive.go b/candidate_peer_reflexive.go index dda5d75..0b330d1 100644 --- a/candidate_peer_reflexive.go +++ b/candidate_peer_reflexive.go @@ -35,8 +35,9 @@ func NewCandidatePeerReflexive(config *CandidatePeerReflexiveConfig) (*Candidate } candidateID := config.CandidateID + candidateIDGenerator := newCandidateIDGenerator() if candidateID == "" { - candidateID = globalCandidateIDGenerator.Generate() + candidateID = candidateIDGenerator.Generate() } return &CandidatePeerReflexive{ diff --git a/candidate_relay_test.go b/candidate_relay_test.go index c2df366..9170a5c 100644 --- a/candidate_relay_test.go +++ b/candidate_relay_test.go @@ -42,7 +42,7 @@ func TestRelayOnlyConnection(t *testing.T) { assert.NoError(t, err) cfg := &AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), Urls: []*URL{ { Scheme: SchemeTypeTURN, diff --git a/candidate_test.go b/candidate_test.go index 23f3a9e..ed4e159 100644 --- a/candidate_test.go +++ b/candidate_test.go @@ -212,80 +212,87 @@ func TestCandidateMarshal(t *testing.T) { marshaled string expectError bool }{ - {&CandidateHost{ - candidateBase{ - networkType: NetworkTypeUDP6, - candidateType: CandidateTypeHost, - address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", - port: 53987, - priorityOverride: 500, - foundationOverride: "750", + { + &CandidateHost{ + candidateBase{ + networkType: NetworkTypeUDP6, + candidateType: CandidateTypeHost, + address: "fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a", + port: 53987, + priorityOverride: 500, + foundationOverride: "750", + }, + "", }, - "", - }, "750 1 udp 500 fcd9:e3b8:12ce:9fc5:74a5:c6bb:d8b:e08a 53987 typ host", false, }, - {&CandidateHost{ - candidateBase{ - networkType: NetworkTypeUDP4, - candidateType: CandidateTypeHost, - address: "10.0.75.1", - port: 53634, + { + &CandidateHost{ + candidateBase{ + networkType: NetworkTypeUDP4, + candidateType: CandidateTypeHost, + address: "10.0.75.1", + port: 53634, + }, + "", }, - "", - }, "4273957277 1 udp 2130706431 10.0.75.1 53634 typ host", false, }, - {&CandidateServerReflexive{ - candidateBase{ - networkType: NetworkTypeUDP4, - candidateType: CandidateTypeServerReflexive, - address: "191.228.238.68", - port: 53991, - relatedAddress: &CandidateRelatedAddress{"192.168.0.274", 53991}, + { + &CandidateServerReflexive{ + candidateBase{ + networkType: NetworkTypeUDP4, + candidateType: CandidateTypeServerReflexive, + address: "191.228.238.68", + port: 53991, + relatedAddress: &CandidateRelatedAddress{"192.168.0.274", 53991}, + }, }, - }, "647372371 1 udp 1694498815 191.228.238.68 53991 typ srflx raddr 192.168.0.274 rport 53991", false, }, - {&CandidateRelay{ - candidateBase{ - networkType: NetworkTypeUDP4, - candidateType: CandidateTypeRelay, - address: "50.0.0.1", - port: 5000, - relatedAddress: &CandidateRelatedAddress{"192.168.0.1", 5001}, + { + &CandidateRelay{ + candidateBase{ + networkType: NetworkTypeUDP4, + candidateType: CandidateTypeRelay, + address: "50.0.0.1", + port: 5000, + relatedAddress: &CandidateRelatedAddress{"192.168.0.1", 5001}, + }, + nil, }, - nil, - }, "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, + { + &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, }, - {&CandidateHost{ - candidateBase{ - networkType: NetworkTypeUDP4, - candidateType: CandidateTypeHost, - address: "e2494022-4d9a-4c1e-a750-cc48d4f8d6ee.local", - port: 60542, + { + &CandidateHost{ + candidateBase{ + networkType: NetworkTypeUDP4, + candidateType: CandidateTypeHost, + address: "e2494022-4d9a-4c1e-a750-cc48d4f8d6ee.local", + port: 60542, + }, + "", }, - "", + "1380287402 1 udp 2130706431 e2494022-4d9a-4c1e-a750-cc48d4f8d6ee.local 60542 typ host", false, }, - "1380287402 1 udp 2130706431 e2494022-4d9a-4c1e-a750-cc48d4f8d6ee.local 60542 typ host", false}, // Invalid candidates {nil, "", true}, diff --git a/candidatepair_test.go b/candidatepair_test.go index 636cad2..12a4c38 100644 --- a/candidatepair_test.go +++ b/candidatepair_test.go @@ -2,32 +2,41 @@ package ice import "testing" -var ( - hostCandidate = &CandidateHost{ +func hostCandidate() *CandidateHost { + return &CandidateHost{ candidateBase: candidateBase{ candidateType: CandidateTypeHost, component: ComponentRTP, }, } - prflxCandidate = &CandidatePeerReflexive{ +} + +func prflxCandidate() *CandidatePeerReflexive { + return &CandidatePeerReflexive{ candidateBase: candidateBase{ candidateType: CandidateTypePeerReflexive, component: ComponentRTP, }, } - srflxCandidate = &CandidateServerReflexive{ +} + +func srflxCandidate() *CandidateServerReflexive { + return &CandidateServerReflexive{ candidateBase: candidateBase{ candidateType: CandidateTypeServerReflexive, component: ComponentRTP, }, } - relayCandidate = &CandidateRelay{ +} + +func relayCandidate() *CandidateRelay { + return &CandidateRelay{ candidateBase: candidateBase{ candidateType: CandidateTypeRelay, component: ComponentRTP, }, } -) +} func TestCandidatePairPriority(t *testing.T) { for _, test := range []struct { @@ -36,64 +45,64 @@ func TestCandidatePairPriority(t *testing.T) { }{ { Pair: newCandidatePair( - hostCandidate, - hostCandidate, + hostCandidate(), + hostCandidate(), false, ), WantPriority: 9151314440652587007, }, { Pair: newCandidatePair( - hostCandidate, - hostCandidate, + hostCandidate(), + hostCandidate(), true, ), WantPriority: 9151314440652587007, }, { Pair: newCandidatePair( - hostCandidate, - prflxCandidate, + hostCandidate(), + prflxCandidate(), true, ), WantPriority: 7998392936314175488, }, { Pair: newCandidatePair( - hostCandidate, - prflxCandidate, + hostCandidate(), + prflxCandidate(), false, ), WantPriority: 7998392936314175487, }, { Pair: newCandidatePair( - hostCandidate, - srflxCandidate, + hostCandidate(), + srflxCandidate(), true, ), WantPriority: 7277816996102668288, }, { Pair: newCandidatePair( - hostCandidate, - srflxCandidate, + hostCandidate(), + srflxCandidate(), false, ), WantPriority: 7277816996102668287, }, { Pair: newCandidatePair( - hostCandidate, - relayCandidate, + hostCandidate(), + relayCandidate(), true, ), WantPriority: 72057593987596288, }, { Pair: newCandidatePair( - hostCandidate, - relayCandidate, + hostCandidate(), + relayCandidate(), false, ), WantPriority: 72057593987596287, @@ -106,8 +115,8 @@ func TestCandidatePairPriority(t *testing.T) { } func TestCandidatePairEquality(t *testing.T) { - pairA := newCandidatePair(hostCandidate, srflxCandidate, true) - pairB := newCandidatePair(hostCandidate, srflxCandidate, false) + pairA := newCandidatePair(hostCandidate(), srflxCandidate(), true) + pairB := newCandidatePair(hostCandidate(), srflxCandidate(), false) if !pairA.Equal(pairB) { t.Fatalf("Expected %v to equal %v", pairA, pairB) diff --git a/candidatetype.go b/candidatetype.go index db12af8..376c408 100644 --- a/candidatetype.go +++ b/candidatetype.go @@ -23,6 +23,8 @@ func (c CandidateType) String() string { return "prflx" case CandidateTypeRelay: return "relay" + case CandidateTypeUnspecified: + return "Unknown candidate type" } return "Unknown candidate type" } @@ -41,6 +43,8 @@ func (c CandidateType) Preference() uint16 { return 110 case CandidateTypeServerReflexive: return 100 + case CandidateTypeRelay, CandidateTypeUnspecified: + return 0 } return 0 } diff --git a/connectivity_vnet_test.go b/connectivity_vnet_test.go index 45ab488..0a81147 100644 --- a/connectivity_vnet_test.go +++ b/connectivity_vnet_test.go @@ -228,7 +228,7 @@ func pipeWithVNet(v *virtualNet, a0TestConfig, a1TestConfig *agentTestConfig) (* cfg0 := &AgentConfig{ Urls: a0TestConfig.urls, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, NAT1To1IPs: nat1To1IPs, NAT1To1IPCandidateType: a0TestConfig.nat1To1IPCandidateType, @@ -251,7 +251,7 @@ func pipeWithVNet(v *virtualNet, a0TestConfig, a1TestConfig *agentTestConfig) (* } cfg1 := &AgentConfig{ Urls: a1TestConfig.urls, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, NAT1To1IPs: nat1To1IPs, NAT1To1IPCandidateType: a1TestConfig.nat1To1IPCandidateType, @@ -491,7 +491,7 @@ func TestDisconnectedToConnected(t *testing.T) { // Create two agents and connect them controllingAgent, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net0, DisconnectedTimeout: &disconnectTimeout, @@ -501,7 +501,7 @@ func TestDisconnectedToConnected(t *testing.T) { assert.NoError(t, err) controlledAgent, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net1, DisconnectedTimeout: &disconnectTimeout, @@ -594,14 +594,14 @@ func TestWriteUseValidPair(t *testing.T) { // Create two agents and connect them controllingAgent, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net0, }) assert.NoError(t, err) controlledAgent, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, Net: net1, }) diff --git a/errors.go b/errors.go index 5504328..e7dd625 100644 --- a/errors.go +++ b/errors.go @@ -108,4 +108,25 @@ var ( // ErrTCPRemoteAddrAlreadyExists indicates we already have the connection with same remote addr. ErrTCPRemoteAddrAlreadyExists = errors.New("conn with same remote addr already exists") + + errSendPacket = errors.New("failed to send packet") + errAttributeTooShortICECandidate = errors.New("attribute not long enough to be ICE candidate") + errParseComponent = errors.New("could not parse component") + errParsePriority = errors.New("could not parse priority") + errParsePort = errors.New("could not parse port") + errParseRelatedAddr = errors.New("could not parse related addresses") + errParseTypType = errors.New("could not parse typtype") + errUnknownCandidateTyp = errors.New("unknown candidate typ") + errGetXorMappedAddrResponse = errors.New("failed to get XOR-MAPPED-ADDRESS response") + errConnectionAddrAlreadyExist = errors.New("connection with same remote address already exists") + errReadingStreamingPacket = errors.New("error reading streaming packet") + errWriting = errors.New("error writing to") + errClosingConnection = errors.New("error closing connection") + errDetermineNetworkType = errors.New("unable to determine networkType") + errMissingProtocolScheme = errors.New("missing protocol scheme") + errTooManyColonsAddr = errors.New("too many colons in address") + errRead = errors.New("unexpected error trying to read") + errUnknownRole = errors.New("unknown role") + errMismatchUsername = errors.New("username mismatch") + errICEWriteSTUNMessage = errors.New("the ICE conn can't write STUN messages") ) diff --git a/external_ip_mapper.go b/external_ip_mapper.go index fdb1c95..5310cc0 100644 --- a/external_ip_mapper.go +++ b/external_ip_mapper.go @@ -65,7 +65,7 @@ type externalIPMapper struct { candidateType CandidateType } -func newExternalIPMapper(candidateType CandidateType, ips []string) (*externalIPMapper, error) { +func newExternalIPMapper(candidateType CandidateType, ips []string) (*externalIPMapper, error) { //nolint:gocognit if len(ips) == 0 { return nil, nil } diff --git a/gather.go b/gather.go index d081cf5..1db2732 100644 --- a/gather.go +++ b/gather.go @@ -3,6 +3,7 @@ package ice import ( "context" "crypto/tls" + "errors" "fmt" "net" "reflect" @@ -112,6 +113,7 @@ func (a *Agent) gatherCandidates(ctx context.Context) { a.gatherCandidatesRelay(ctx, a.urls) wg.Done() }() + case CandidateTypePeerReflexive, CandidateTypeUnspecified: } } // Block until all STUN and TURN URLs have been gathered (or timed out) @@ -122,7 +124,7 @@ func (a *Agent) gatherCandidates(ctx context.Context) { } } -func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []NetworkType) { +func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []NetworkType) { //nolint:gocognit networks := map[string]struct{}{} for _, networkType := range networkTypes { if networkType.IsTCP() { @@ -162,20 +164,18 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ switch network { case tcp: // Handle ICE TCP passive mode - // TODO active mode - // TODO S-O mode a.log.Debugf("GetConn by ufrag: %s\n", a.localUfrag) conn, err = a.tcpMux.GetConnByUfrag(a.localUfrag) if err != nil { - if err != ErrTCPMuxNotInitialized { + if !errors.Is(err, ErrTCPMuxNotInitialized) { a.log.Warnf("error getting tcp conn by ufrag: %s %s %s\n", network, ip, a.localUfrag) } continue } port = conn.LocalAddr().(*net.TCPAddr).Port tcpType = TCPTypePassive - // TODO is there a way to verify that the listen address is even + // is there a way to verify that the listen address is even // accessible from the current interface. case udp: conn, err = listenUDPInPortRange(a.net, a.log, int(a.portmax), int(a.portmin), network, &net.UDPAddr{IP: ip, Port: 0}) @@ -332,11 +332,11 @@ func (a *Agent) gatherCandidatesSrflx(ctx context.Context, urls []*URL, networkT } } -func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { +func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //nolint:gocognit var wg sync.WaitGroup defer wg.Wait() - network := NetworkTypeUDP4.String() // TODO IPv6 + network := NetworkTypeUDP4.String() for i := range urls { switch { case urls[i].Scheme != SchemeTypeTURN && urls[i].Scheme != SchemeTypeTURNS: diff --git a/gather_test.go b/gather_test.go index d1d939d..8e953b6 100644 --- a/gather_test.go +++ b/gather_test.go @@ -127,7 +127,7 @@ func TestSTUNConcurrency(t *testing.T) { }() a, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), Urls: urls, CandidateTypes: []CandidateType{CandidateTypeHost, CandidateTypeServerReflexive}, TCPMux: NewTCPMuxDefault( @@ -212,7 +212,7 @@ func TestTURNConcurrency(t *testing.T) { a, err := NewAgent(&AgentConfig{ CandidateTypes: []CandidateType{CandidateTypeRelay}, InsecureSkipVerify: true, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), Urls: urls, }) assert.NoError(t, err) @@ -252,7 +252,7 @@ func TestTURNConcurrency(t *testing.T) { assert.NoError(t, genErr) serverPort := randomPort(t) - serverListener, err := tls.Listen("tcp", "127.0.0.1:"+strconv.Itoa(serverPort), &tls.Config{ + serverListener, err := tls.Listen("tcp", "127.0.0.1:"+strconv.Itoa(serverPort), &tls.Config{ //nolint:gosec Certificates: []tls.Certificate{certificate}, }) assert.NoError(t, err) @@ -316,7 +316,7 @@ func TestSTUNTURNConcurrency(t *testing.T) { }) a, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), Urls: urls, CandidateTypes: []CandidateType{CandidateTypeServerReflexive, CandidateTypeRelay}, }) @@ -380,7 +380,7 @@ func TestTURNSrflx(t *testing.T) { }} a, err := NewAgent(&AgentConfig{ - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), Urls: urls, CandidateTypes: []CandidateType{CandidateTypeServerReflexive, CandidateTypeRelay}, }) diff --git a/gather_vnet_test.go b/gather_vnet_test.go index 1d5d312..211cea4 100644 --- a/gather_vnet_test.go +++ b/gather_vnet_test.go @@ -4,6 +4,7 @@ package ice import ( "context" + "errors" "fmt" "net" "testing" @@ -19,7 +20,7 @@ func TestVNetGather(t *testing.T) { defer report() loggerFactory := logging.NewDefaultLoggerFactory() - //log := loggerFactory.NewLogger("test") + // log := loggerFactory.NewLogger("test") t.Run("No local IP address", func(t *testing.T) { a, err := NewAgent(&AgentConfig{ @@ -131,7 +132,7 @@ func TestVNetGather(t *testing.T) { } _, err = listenUDPInPortRange(a.net, a.log, 4999, 5000, udp, &net.UDPAddr{IP: ip, Port: 0}) - if err != ErrPort { + if !errors.Is(err, ErrPort) { t.Fatal("listenUDP with invalid port range did not return ErrPort") } @@ -443,7 +444,7 @@ func TestVNetGather_TURNConnectionLeak(t *testing.T) { Urls: []*URL{ turnServerURL, }, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), MulticastDNSMode: MulticastDNSModeDisabled, NAT1To1IPs: []string{vnetGlobalIPA}, Net: v.net0, diff --git a/icecontrol.go b/icecontrol.go index 017d50e..ede2e09 100644 --- a/icecontrol.go +++ b/icecontrol.go @@ -1,6 +1,10 @@ package ice -import "github.com/pion/stun" +import ( + "encoding/binary" + + "github.com/pion/stun" +) // tiebreaker is common helper for ICE-{CONTROLLED,CONTROLLING} // and represents the so-called tiebreaker number. @@ -11,7 +15,7 @@ const tiebreakerSize = 8 // 64 bit // AddToAs adds tiebreaker value to m as t attribute. func (a tiebreaker) AddToAs(m *stun.Message, t stun.AttrType) error { v := make([]byte, tiebreakerSize) - bin.PutUint64(v, uint64(a)) + binary.BigEndian.PutUint64(v, uint64(a)) m.Add(t, v) return nil } @@ -25,7 +29,7 @@ func (a *tiebreaker) GetFromAs(m *stun.Message, t stun.AttrType) error { if err = stun.CheckSize(t, len(v), tiebreakerSize); err != nil { return err } - *a = tiebreaker(bin.Uint64(v)) + *a = tiebreaker(binary.BigEndian.Uint64(v)) return nil } diff --git a/icecontrol_test.go b/icecontrol_test.go index 9254c71..3443ccb 100644 --- a/icecontrol_test.go +++ b/icecontrol_test.go @@ -1,15 +1,16 @@ package ice import ( + "errors" "testing" "github.com/pion/stun" ) -func TestControlled_GetFrom(t *testing.T) { +func TestControlled_GetFrom(t *testing.T) { //nolint:dupl m := new(stun.Message) var c AttrControlled - if err := c.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := c.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } if err := m.Build(stun.BindingRequest, &c); err != nil { @@ -36,10 +37,10 @@ func TestControlled_GetFrom(t *testing.T) { }) } -func TestControlling_GetFrom(t *testing.T) { +func TestControlling_GetFrom(t *testing.T) { //nolint:dupl m := new(stun.Message) var c AttrControlling - if err := c.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := c.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } if err := m.Build(stun.BindingRequest, &c); err != nil { @@ -70,14 +71,14 @@ func TestControl_GetFrom(t *testing.T) { t.Run("Blank", func(t *testing.T) { m := new(stun.Message) var c AttrControl - if err := c.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := c.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } }) - t.Run("Controlling", func(t *testing.T) { + t.Run("Controlling", func(t *testing.T) { //nolint:dupl m := new(stun.Message) var c AttrControl - if err := c.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := c.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } c.Role = Controlling @@ -105,10 +106,10 @@ func TestControl_GetFrom(t *testing.T) { } }) }) - t.Run("Controlled", func(t *testing.T) { + t.Run("Controlled", func(t *testing.T) { //nolint:dupl m := new(stun.Message) var c AttrControl - if err := c.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := c.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } c.Role = Controlled diff --git a/networktype.go b/networktype.go index 7b501dc..462ff2d 100644 --- a/networktype.go +++ b/networktype.go @@ -11,11 +11,13 @@ const ( tcp = "tcp" ) -var supportedNetworkTypes = []NetworkType{ - NetworkTypeUDP4, - NetworkTypeUDP6, - NetworkTypeTCP4, - NetworkTypeTCP6, +func supportedNetworkTypes() []NetworkType { + return []NetworkType{ + NetworkTypeUDP4, + NetworkTypeUDP6, + NetworkTypeTCP4, + NetworkTypeTCP6, + } } // NetworkType represents the type of network @@ -124,5 +126,5 @@ func determineNetworkType(network string, ip net.IP) (NetworkType, error) { return NetworkTypeTCP6, nil } - return NetworkType(0), fmt.Errorf("unable to determine networkType from %s %s", network, ip) + return NetworkType(0), fmt.Errorf("%w from %s %s", errDetermineNetworkType, network, ip) } diff --git a/priority.go b/priority.go index 727c550..4218299 100644 --- a/priority.go +++ b/priority.go @@ -1,6 +1,10 @@ package ice -import "github.com/pion/stun" +import ( + "encoding/binary" + + "github.com/pion/stun" +) // PriorityAttr represents PRIORITY attribute. type PriorityAttr uint32 @@ -10,7 +14,7 @@ const prioritySize = 4 // 32 bit // AddTo adds PRIORITY attribute to message. func (p PriorityAttr) AddTo(m *stun.Message) error { v := make([]byte, prioritySize) - bin.PutUint32(v, uint32(p)) + binary.BigEndian.PutUint32(v, uint32(p)) m.Add(stun.AttrPriority, v) return nil } @@ -24,6 +28,6 @@ func (p *PriorityAttr) GetFrom(m *stun.Message) error { if err = stun.CheckSize(stun.AttrPriority, len(v), prioritySize); err != nil { return err } - *p = PriorityAttr(bin.Uint32(v)) + *p = PriorityAttr(binary.BigEndian.Uint32(v)) return nil } diff --git a/priority_test.go b/priority_test.go index 6930d8a..16f6b10 100644 --- a/priority_test.go +++ b/priority_test.go @@ -1,15 +1,16 @@ package ice import ( + "errors" "testing" "github.com/pion/stun" ) -func TestPriority_GetFrom(t *testing.T) { +func TestPriority_GetFrom(t *testing.T) { //nolint:dupl m := new(stun.Message) var p PriorityAttr - if err := p.GetFrom(m); err != stun.ErrAttributeNotFound { + if err := p.GetFrom(m); !errors.Is(err, stun.ErrAttributeNotFound) { t.Error("unexpected error") } if err := m.Build(stun.BindingRequest, &p); err != nil { diff --git a/rand.go b/rand.go index 5e3c132..918783e 100644 --- a/rand.go +++ b/rand.go @@ -14,8 +14,10 @@ const ( // Seeding random generator each time limits number of generated sequence to 31-bits, // and causes collision on low time accuracy environments. // Use global random generator seeded by crypto grade random. -var globalMathRandomGenerator = randutil.NewMathRandomGenerator() -var globalCandidateIDGenerator = candidateIDGenerator{globalMathRandomGenerator} +var ( + globalMathRandomGenerator = randutil.NewMathRandomGenerator() //nolint:gochecknoglobals + globalCandidateIDGenerator = candidateIDGenerator{globalMathRandomGenerator} //nolint:gochecknoglobals +) // candidateIDGenerator is a random candidate ID generator. // Candidate ID is used in SDP and always shared to the other peer. diff --git a/role.go b/role.go index c4fa420..7a8bc06 100644 --- a/role.go +++ b/role.go @@ -1,6 +1,8 @@ package ice -import "fmt" +import ( + "fmt" +) // Role represents ICE agent role, which can be controlling or controlled. type Role byte @@ -19,7 +21,7 @@ func (r *Role) UnmarshalText(text []byte) error { case "controlled": *r = Controlled default: - return fmt.Errorf("unknown role %q", text) + return fmt.Errorf("%w %q", errUnknownRole, text) } return nil } diff --git a/selection.go b/selection.go index 7f93781..df5d1f9 100644 --- a/selection.go +++ b/selection.go @@ -73,13 +73,12 @@ func (s *controllingSelector) nominatePair(pair *candidatePair) { // request. msg, err := stun.Build(stun.BindingRequest, stun.TransactionID, stun.NewUsername(s.agent.remoteUfrag+":"+s.agent.localUfrag), - UseCandidate, + UseCandidate(), AttrControlling(s.agent.tieBreaker), PriorityAttr(pair.local.Priority()), stun.NewShortTermIntegrity(s.agent.remotePwd), stun.Fingerprint, ) - if err != nil { s.log.Error(err.Error()) return @@ -152,7 +151,6 @@ func (s *controllingSelector) PingCandidate(local, remote Candidate) { stun.NewShortTermIntegrity(s.agent.remotePwd), stun.Fingerprint, ) - if err != nil { s.log.Error(err.Error()) return @@ -188,7 +186,6 @@ func (s *controlledSelector) PingCandidate(local, remote Candidate) { stun.NewShortTermIntegrity(s.agent.remotePwd), stun.Fingerprint, ) - if err != nil { s.log.Error(err.Error()) return @@ -198,6 +195,7 @@ func (s *controlledSelector) PingCandidate(local, remote Candidate) { } func (s *controlledSelector) HandleSuccessResponse(m *stun.Message, local, remote Candidate, remoteAddr net.Addr) { + // nolint:godox // TODO according to the standard we should specifically answer a failed nomination: // https://tools.ietf.org/html/rfc8445#section-7.3.1.5 // If the controlled agent does not accept the request from the @@ -278,6 +276,7 @@ type liteSelector struct { // A lite selector should not contact candidates func (s *liteSelector) ContactCandidates() { if _, ok := s.pairCandidateSelector.(*controllingSelector); ok { + // nolint:godox // pion/ice#96 // TODO: implement lite controlling agent. For now falling back to full agent. // This only happens if both peers are lite. See RFC 8445 S6.1.1 and S6.2 diff --git a/stun.go b/stun.go index 6a3315c..bef7c87 100644 --- a/stun.go +++ b/stun.go @@ -1,22 +1,18 @@ package ice import ( - "encoding/binary" "fmt" "github.com/pion/stun" ) -// bin is shorthand for BigEndian. -var bin = binary.BigEndian - func assertInboundUsername(m *stun.Message, expectedUsername string) error { var username stun.Username if err := username.GetFrom(m); err != nil { return err } if string(username) != expectedUsername { - return fmt.Errorf("username mismatch expected(%x) actual(%x)", expectedUsername, string(username)) + return fmt.Errorf("%w expected(%x) actual(%x)", errMismatchUsername, expectedUsername, string(username)) } return nil diff --git a/tcp_mux.go b/tcp_mux.go index 3bddf63..1a9a797 100644 --- a/tcp_mux.go +++ b/tcp_mux.go @@ -159,7 +159,6 @@ func (m *TCPMuxDefault) handleConn(conn net.Conn) { buf := make([]byte, receiveMTU) n, err := readStreamingPacket(conn, buf) - if err != nil { m.params.Logger.Warnf("Error reading first packet: %s", err) return @@ -254,7 +253,7 @@ const streamingPacketHeaderLen = 2 // | LENGTH | RTP or RTCP packet ... | // ----------------------------------------------------------------- func readStreamingPacket(conn net.Conn, buf []byte) (int, error) { - var header = make([]byte, streamingPacketHeaderLen) + header := make([]byte, streamingPacketHeaderLen) var bytesRead, n int var err error @@ -288,7 +287,6 @@ func writeStreamingPacket(conn net.Conn, buf []byte) (int, error) { copy(bufferCopy[2:], buf) n, err := conn.Write(bufferCopy) - if err != nil { return 0, err } diff --git a/tcp_mux_test.go b/tcp_mux_test.go index 6d2f0c5..8340b94 100644 --- a/tcp_mux_test.go +++ b/tcp_mux_test.go @@ -12,8 +12,10 @@ import ( "github.com/stretchr/testify/require" ) -var _ TCPMux = &TCPMuxDefault{} -var _ TCPMux = &invalidTCPMux{} +var ( + _ TCPMux = &TCPMuxDefault{} + _ TCPMux = &invalidTCPMux{} +) func TestTCPMux_Recv(t *testing.T) { report := test.CheckRoutines(t) diff --git a/tcp_packet_conn.go b/tcp_packet_conn.go index 264dea4..dc4eaf0 100644 --- a/tcp_packet_conn.go +++ b/tcp_packet_conn.go @@ -62,7 +62,7 @@ func (t *tcpPacketConn) AddConn(conn net.Conn, firstPacketData []byte) error { } if _, ok := t.conns[conn.RemoteAddr().String()]; ok { - return fmt.Errorf("connection with same remote address already exists: %s", conn.RemoteAddr().String()) + return fmt.Errorf("%w: %s", errConnectionAddrAlreadyExist, conn.RemoteAddr().String()) } t.conns[conn.RemoteAddr().String()] = conn @@ -85,9 +85,8 @@ func (t *tcpPacketConn) startReading(conn net.Conn) { for { n, err := readStreamingPacket(conn, buf) // t.params.Logger.Infof("readStreamingPacket read %d bytes", n) - if err != nil { - t.params.Logger.Infof("Error reading streaming packet: %s\n", err) + t.params.Logger.Infof("%w: %s\n", errReadingStreamingPacket, err) t.handleRecv(streamingPacket{nil, conn.RemoteAddr(), err}) t.removeConn(conn) return @@ -168,7 +167,7 @@ func (t *tcpPacketConn) WriteTo(buf []byte, raddr net.Addr) (n int, err error) { n, err = writeStreamingPacket(conn, buf) if err != nil { - t.params.Logger.Tracef("Error writing to %s\n", raddr) + t.params.Logger.Tracef("%w %s\n", errWriting, raddr) return n, err } @@ -178,7 +177,7 @@ func (t *tcpPacketConn) WriteTo(buf []byte, raddr net.Addr) (n int, err error) { func (t *tcpPacketConn) closeAndLogError(closer io.Closer) { err := closer.Close() if err != nil { - t.params.Logger.Warnf("Error closing connection: %s", err) + t.params.Logger.Warnf("%w: %s", errClosingConnection, err) } } diff --git a/transport.go b/transport.go index 56bb12d..d1c82ff 100644 --- a/transport.go +++ b/transport.go @@ -2,7 +2,6 @@ package ice import ( "context" - "errors" "net" "sync/atomic" "time" @@ -55,7 +54,6 @@ func (a *Agent) connect(ctx context.Context, isControlling bool, remoteUfrag, re case <-a.done: return nil, a.getErr() case <-ctx.Done(): - // TODO: Stop connectivity checks? return nil, ErrCanceledByCaller case <-a.onConnected: } @@ -85,7 +83,7 @@ func (c *Conn) Write(p []byte) (int, error) { } if stun.IsMessage(p) { - return 0, errors.New("the ICE conn can't write STUN messages") + return 0, errICEWriteSTUNMessage } pair := c.agent.getSelectedPair() @@ -111,8 +109,6 @@ func (c *Conn) Close() error { return c.agent.Close() } -// TODO: Maybe just switch to using io.ReadWriteCloser? - // LocalAddr returns the local address of the current selected pair or nil if there is none. func (c *Conn) LocalAddr() net.Addr { pair := c.agent.getSelectedPair() diff --git a/transport_test.go b/transport_test.go index ee34649..20b65e6 100644 --- a/transport_test.go +++ b/transport_test.go @@ -4,7 +4,6 @@ package ice import ( "context" - "errors" "net" "sync" "testing" @@ -82,7 +81,6 @@ func TestTimeout(t *testing.T) { t.Run("WithoutDisconnectTimeout", func(t *testing.T) { ca, cb := pipe(nil) err := cb.Close() - if err != nil { // we should never get here. panic(err) @@ -94,7 +92,6 @@ func TestTimeout(t *testing.T) { t.Run("WithDisconnectTimeout", func(t *testing.T) { ca, cb := pipeWithTimeout(5*time.Second, 3*time.Second) err := cb.Close() - if err != nil { // we should never get here. panic(err) @@ -254,7 +251,7 @@ func pipe(defaultConfig *AgentConfig) (*Conn, *Conn) { } cfg.Urls = urls - cfg.NetworkTypes = supportedNetworkTypes + cfg.NetworkTypes = supportedNetworkTypes() aAgent, err := NewAgent(cfg) check(err) @@ -285,7 +282,7 @@ func pipeWithTimeout(disconnectTimeout time.Duration, iceKeepalive time.Duration Urls: urls, DisconnectedTimeout: &disconnectTimeout, KeepaliveInterval: &iceKeepalive, - NetworkTypes: supportedNetworkTypes, + NetworkTypes: supportedNetworkTypes(), } aAgent, err := NewAgent(cfg) @@ -397,7 +394,7 @@ func TestConnStats(t *testing.T) { go func() { buf := make([]byte, 10) if _, err := cb.Read(buf); err != nil { - panic(errors.New("unexpected error trying to read")) + panic(errRead) } wg.Done() }() diff --git a/url.go b/url.go index 1b6cf13..390591e 100644 --- a/url.go +++ b/url.go @@ -6,8 +6,6 @@ import ( "strconv" ) -// TODO: Migrate address parsing to STUN/TURN - // SchemeType indicates the type of server used in the ice.URL structure. type SchemeType int @@ -110,7 +108,7 @@ type URL struct { // ParseURL parses a STUN or TURN urls following the ABNF syntax described in // https://tools.ietf.org/html/rfc7064 and https://tools.ietf.org/html/rfc7065 // respectively. -func ParseURL(raw string) (*URL, error) { +func ParseURL(raw string) (*URL, error) { //nolint:gocognit rawParts, err := url.Parse(raw) if err != nil { return nil, err diff --git a/url_test.go b/url_test.go index 021ba5c..2527a10 100644 --- a/url_test.go +++ b/url_test.go @@ -1,7 +1,6 @@ package ice import ( - "errors" "fmt" "net" "net/url" @@ -52,8 +51,8 @@ func TestParseURL(t *testing.T) { expectedErr error }{ {"", ErrSchemeType}, - {":::", errors.New("missing protocol scheme")}, - {"stun:[::1]:123:", errors.New("too many colons in address")}, + {":::", errMissingProtocolScheme}, + {"stun:[::1]:123:", errTooManyColonsAddr}, {"stun:[::1]:123a", ErrPort}, {"google.de", ErrSchemeType}, {"stun:", ErrHost}, @@ -72,7 +71,7 @@ func TestParseURL(t *testing.T) { case *url.Error: err = e.Err case *net.AddrError: - err = fmt.Errorf("%v", e.Err) + err = fmt.Errorf(e.Err) //nolint:goerr113 } assert.EqualError(t, err, testCase.expectedErr.Error(), "testCase: %d %v", i, testCase) } diff --git a/usecandidate.go b/usecandidate.go index 21aa374..f168c08 100644 --- a/usecandidate.go +++ b/usecandidate.go @@ -18,4 +18,6 @@ func (UseCandidateAttr) IsSet(m *stun.Message) bool { } // UseCandidate is shorthand for UseCandidateAttr. -var UseCandidate UseCandidateAttr +func UseCandidate() UseCandidateAttr { + return UseCandidateAttr{} +} diff --git a/usecandidate_test.go b/usecandidate_test.go index 8d8406a..56cd62f 100644 --- a/usecandidate_test.go +++ b/usecandidate_test.go @@ -8,17 +8,17 @@ import ( func TestUseCandidateAttr_AddTo(t *testing.T) { m := new(stun.Message) - if UseCandidate.IsSet(m) { + if UseCandidate().IsSet(m) { t.Error("should not be set") } - if err := m.Build(stun.BindingRequest, UseCandidate); err != nil { + if err := m.Build(stun.BindingRequest, UseCandidate()); err != nil { t.Error(err) } m1 := new(stun.Message) if _, err := m1.Write(m.Raw); err != nil { t.Error(err) } - if !UseCandidate.IsSet(m1) { + if !UseCandidate().IsSet(m1) { t.Error("should be set") } } diff --git a/util.go b/util.go index a75b2bd..7eb13c8 100644 --- a/util.go +++ b/util.go @@ -16,6 +16,7 @@ type atomicError struct{ v atomic.Value } func (a *atomicError) Store(err error) { a.v.Store(struct{ error }{err}) } + func (a *atomicError) Load() error { err, _ := a.v.Load().(struct{ error }) return err.error @@ -105,7 +106,7 @@ func getXORMappedAddr(conn net.PacketConn, serverAddr net.Addr, deadline time.Du } var addr stun.XORMappedAddress if err = addr.GetFrom(resp); err != nil { - return nil, fmt.Errorf("failed to get XOR-MAPPED-ADDRESS response: %v", err) + return nil, fmt.Errorf("%w: %v", errGetXorMappedAddrResponse, err) } return &addr, nil } @@ -131,7 +132,7 @@ func stunRequest(read func([]byte) (int, error), write func([]byte) (int, error) return res, nil } -func localInterfaces(vnet *vnet.Net, interfaceFilter func(string) bool, networkTypes []NetworkType) ([]net.IP, error) { +func localInterfaces(vnet *vnet.Net, interfaceFilter func(string) bool, networkTypes []NetworkType) ([]net.IP, error) { //nolint:gocognit ips := []net.IP{} ifaces, err := vnet.Interfaces() if err != nil {