From 366757d59dd224ef8d8cec428bc343c8c16c8bed Mon Sep 17 00:00:00 2001 From: Woodrow Douglass Date: Fri, 6 Aug 2021 12:42:05 -0400 Subject: [PATCH] Remove unknown constant This constant tends to cause collisions with enumerations, and is in general a source of bugs --- AUTHORS.txt | 2 ++ candidate_base.go | 2 ++ candidatepair_state.go | 6 +++++- candidatepair_test.go | 8 ++++++++ candidatetype.go | 7 +++++-- gather.go | 1 + ice.go | 6 ++++++ ice_test.go | 4 ++-- url.go | 32 ++++++++++++++++++-------------- url_test.go | 1 + 10 files changed, 50 insertions(+), 19 deletions(-) diff --git a/AUTHORS.txt b/AUTHORS.txt index 68666a4..7846911 100644 --- a/AUTHORS.txt +++ b/AUTHORS.txt @@ -17,6 +17,7 @@ Chao Yuan David Hamilton David Zhao Henry +hn8 <10730886+hn8@users.noreply.github.com> Hugo Arregui Hugo Arregui Jason Maldonis @@ -41,6 +42,7 @@ Sean DuBois Sebastian Waisbrot Sidney San Martín Will Forcey +Woodrow Douglass Yutaka Takeda ZHENK Zizheng Tai diff --git a/candidate_base.go b/candidate_base.go index 63231e6..45229d2 100644 --- a/candidate_base.go +++ b/candidate_base.go @@ -172,6 +172,8 @@ func (c *candidateBase) LocalPreference() uint16 { } case CandidateTypeUnspecified: return 0 + default: + return 0 } return 0 }() diff --git a/candidatepair_state.go b/candidatepair_state.go index 28c7187..99a89f4 100644 --- a/candidatepair_state.go +++ b/candidatepair_state.go @@ -20,6 +20,9 @@ const ( // CandidatePairStateSucceeded means a check for this pair was already // done and produced a successful result. CandidatePairStateSucceeded + + // CandidatePairStateUnknown probably indicates an error + CandidatePairStateUnknown ) func (c CandidatePairState) String() string { @@ -32,6 +35,7 @@ func (c CandidatePairState) String() string { return "failed" case CandidatePairStateSucceeded: return "succeeded" + default: + return "Unknown candidate pair state" } - return "Unknown candidate pair state" } diff --git a/candidatepair_test.go b/candidatepair_test.go index 119ab01..ea85515 100644 --- a/candidatepair_test.go +++ b/candidatepair_test.go @@ -131,3 +131,11 @@ func TestNilCandidatePairString(t *testing.T) { var nilCandidatePair *CandidatePair assert.Equal(t, nilCandidatePair.String(), "") } + +func TestInvalidCandidatePairStateString(t *testing.T) { + state := CandidatePairState(0) + assert.NotEqual(t, "waiting", state.String()) + assert.NotEqual(t, "in-progress", state.String()) + assert.NotEqual(t, "failed", state.String()) + assert.NotEqual(t, "succeeded", state.String()) +} diff --git a/candidatetype.go b/candidatetype.go index 376c408..382887a 100644 --- a/candidatetype.go +++ b/candidatetype.go @@ -10,6 +10,7 @@ const ( CandidateTypeServerReflexive CandidateTypePeerReflexive CandidateTypeRelay + CandidateTypeUnknown ) // String makes CandidateType printable @@ -25,8 +26,9 @@ func (c CandidateType) String() string { return "relay" case CandidateTypeUnspecified: return "Unknown candidate type" + default: + return "Unknown candidate type" } - return "Unknown candidate type" } // Preference returns the preference weight of a CandidateType @@ -45,8 +47,9 @@ func (c CandidateType) Preference() uint16 { return 100 case CandidateTypeRelay, CandidateTypeUnspecified: return 0 + default: + return 0 } - return 0 } func containsCandidateType(candidateType CandidateType, candidateTypeList []CandidateType) bool { diff --git a/gather.go b/gather.go index 6a6588a..7c6931f 100644 --- a/gather.go +++ b/gather.go @@ -114,6 +114,7 @@ func (a *Agent) gatherCandidates(ctx context.Context) { wg.Done() }() case CandidateTypePeerReflexive, CandidateTypeUnspecified: + default: } } // Block until all STUN and TURN URLs have been gathered (or timed out) diff --git a/ice.go b/ice.go index d7094f6..1630fcb 100644 --- a/ice.go +++ b/ice.go @@ -25,6 +25,9 @@ const ( // ConnectionStateClosed ICE agent has finished and is no longer handling requests ConnectionStateClosed + + // Unknown State + ConnectionStateUnknown ) func (c ConnectionState) String() string { @@ -60,6 +63,9 @@ const ( // GatheringStateComplete indicates candidate gatering has been completed GatheringStateComplete + + // GatheringStateUnknown probably indicates an error + GatheringStateUnknown ) func (t GatheringState) String() string { diff --git a/ice_test.go b/ice_test.go index 727ae5e..f0d67b3 100644 --- a/ice_test.go +++ b/ice_test.go @@ -11,7 +11,7 @@ func TestConnectedState_String(t *testing.T) { connectionState ConnectionState expectedString string }{ - {ConnectionState(Unknown), "Invalid"}, + {ConnectionStateUnknown, "Invalid"}, {ConnectionStateNew, "New"}, {ConnectionStateChecking, "Checking"}, {ConnectionStateConnected, "Connected"}, @@ -35,7 +35,7 @@ func TestGatheringState_String(t *testing.T) { gatheringState GatheringState expectedString string }{ - {GatheringState(Unknown), ErrUnknownType.Error()}, + {GatheringStateUnknown, ErrUnknownType.Error()}, {GatheringStateNew, "new"}, {GatheringStateGathering, "gathering"}, {GatheringStateComplete, "complete"}, diff --git a/url.go b/url.go index 390591e..263eea6 100644 --- a/url.go +++ b/url.go @@ -9,10 +9,6 @@ import ( // SchemeType indicates the type of server used in the ice.URL structure. type SchemeType int -// Unknown defines default public constant to use for "enum" like struct -// comparisons when no value was defined. -const Unknown = iota - const ( // SchemeTypeSTUN indicates the URL represents a STUN server. SchemeTypeSTUN SchemeType = iota + 1 @@ -25,6 +21,9 @@ const ( // SchemeTypeTURNS indicates the URL represents a TURNS (secure) server. SchemeTypeTURNS + + // SchemeTypeUnknown indicated an unknown scheme type + SchemeTypeUnknown ) // NewSchemeType defines a procedure for creating a new SchemeType from a raw @@ -40,7 +39,7 @@ func NewSchemeType(raw string) SchemeType { case "turns": return SchemeTypeTURNS default: - return SchemeType(Unknown) + return SchemeTypeUnknown } } @@ -69,6 +68,9 @@ const ( // ProtoTypeTCP indicates the URL uses a TCP transport. ProtoTypeTCP + + // ProtoTypeUnknown indicates an unknown protocol type, which is probably an error + ProtoTypeUnknown ) // NewProtoType defines a procedure for creating a new ProtoType from a raw @@ -80,7 +82,7 @@ func NewProtoType(raw string) ProtoType { case "tcp": return ProtoTypeTCP default: - return ProtoType(Unknown) + return ProtoTypeUnknown } } @@ -116,7 +118,7 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit var u URL u.Scheme = NewSchemeType(rawParts.Scheme) - if u.Scheme == SchemeType(Unknown) { + if u.Scheme == SchemeTypeUnknown { return nil, ErrSchemeType } @@ -172,7 +174,7 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit } u.Proto = proto - if u.Proto == ProtoType(Unknown) { + if u.Proto == ProtoTypeUnknown { u.Proto = ProtoTypeUDP } case SchemeTypeTURNS: @@ -182,9 +184,11 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit } u.Proto = proto - if u.Proto == ProtoType(Unknown) { + if u.Proto == ProtoTypeUnknown { u.Proto = ProtoTypeTCP } + default: + return nil, ErrSTUNQuery } return &u, nil @@ -193,19 +197,19 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit func parseProto(raw string) (ProtoType, error) { qArgs, err := url.ParseQuery(raw) if err != nil || len(qArgs) > 1 { - return ProtoType(Unknown), ErrInvalidQuery + return ProtoTypeUnknown, ErrInvalidQuery } - var proto ProtoType + proto := ProtoTypeUnknown if rawProto := qArgs.Get("transport"); rawProto != "" { - if proto = NewProtoType(rawProto); proto == ProtoType(0) { - return ProtoType(Unknown), ErrProtoType + if proto = NewProtoType(rawProto); proto == ProtoTypeUnknown { + return ProtoTypeUnknown, ErrProtoType } return proto, nil } if len(qArgs) > 0 { - return ProtoType(Unknown), ErrInvalidQuery + return ProtoTypeUnknown, ErrInvalidQuery } return proto, nil diff --git a/url_test.go b/url_test.go index 2527a10..a21a11a 100644 --- a/url_test.go +++ b/url_test.go @@ -56,6 +56,7 @@ func TestParseURL(t *testing.T) { {"stun:[::1]:123a", ErrPort}, {"google.de", ErrSchemeType}, {"stun:", ErrHost}, + {"stan:", ErrSchemeType}, {"stun:google.de:abc", ErrPort}, {"stun:google.de?transport=udp", ErrSTUNQuery}, {"stuns:google.de?transport=udp", ErrSTUNQuery},