Update CI configs to v0.4.7

Update lint scripts and CI configs.

.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

datachannel.go

@@ -96,11 +96,11 @@ func Client(stream *sctp.Stream, config *Config) (*DataChannel, error) {
 	if !config.Negotiated {
 		rawMsg, err := msg.Marshal()
 		if err != nil {
-			return nil, fmt.Errorf("failed to marshal ChannelOpen %v", err)
+			return nil, fmt.Errorf("failed to marshal ChannelOpen %w", err)
 		}
 
 		if _, err = stream.WriteSCTP(rawMsg, sctp.PayloadTypeWebRTCDCEP); err != nil {
-			return nil, fmt.Errorf("failed to send ChannelOpen %v", err)
+			return nil, fmt.Errorf("failed to send ChannelOpen %w", err)
 		}
 	}
 	return newDataChannel(stream, config)
@@ -132,7 +132,7 @@ func Server(stream *sctp.Stream, config *Config) (*DataChannel, error) {
 	}
 
 	if ppi != sctp.PayloadTypeWebRTCDCEP {
-		return nil, fmt.Errorf("unexpected packet type: %s", ppi)
+		return nil, fmt.Errorf("%w %s", ErrInvalidPayloadProtocolIdentifier, ppi)
 	}
 
 	openMsg, err := parseExpectDataChannelOpen(buffer[:n])
@@ -176,35 +176,27 @@ func (c *DataChannel) ReadDataChannel(p []byte) (int, bool, error) {
 		if err == io.EOF {
 			// When the peer sees that an incoming stream was
 			// reset, it also resets its corresponding outgoing stream.
-			closeErr := c.stream.Close()
-			if closeErr != nil {
+			if closeErr := c.stream.Close(); closeErr != nil {
 				return 0, false, closeErr
 			}
 		}
 		if err != nil {
 			return 0, false, err
 		}
 
-		var isString bool
-		switch ppi {
-		case sctp.PayloadTypeWebRTCDCEP:
-			err = c.handleDCEP(p[:n])
-			if err != nil {
+		if ppi == sctp.PayloadTypeWebRTCDCEP {
+			if err = c.handleDCEP(p[:n]); err != nil {
 				c.log.Errorf("Failed to handle DCEP: %s", err.Error())
-				continue
 			}
 			continue
-		case sctp.PayloadTypeWebRTCString, sctp.PayloadTypeWebRTCStringEmpty:
-			isString = true
-		}
-		switch ppi {
-		case sctp.PayloadTypeWebRTCBinaryEmpty, sctp.PayloadTypeWebRTCStringEmpty:
+		} else if ppi == sctp.PayloadTypeWebRTCBinaryEmpty || ppi == sctp.PayloadTypeWebRTCStringEmpty {
 			n = 0
 		}
 
 		atomic.AddUint32(&c.messagesReceived, 1)
 		atomic.AddUint64(&c.bytesReceived, uint64(n))
 
+		isString := ppi == sctp.PayloadTypeWebRTCString || ppi == sctp.PayloadTypeWebRTCStringEmpty
 		return n, isString, err
 	}
 }
@@ -237,29 +229,17 @@ func (c *DataChannel) StreamIdentifier() uint16 {
 func (c *DataChannel) handleDCEP(data []byte) error {
 	msg, err := parse(data)
 	if err != nil {
-		return fmt.Errorf("Failed to parse DataChannel packet %w", err)
+		return fmt.Errorf("failed to parse DataChannel packet %w", err)
 	}
 
 	switch msg := msg.(type) {
-	case *channelOpen:
-		c.log.Debug("Received DATA_CHANNEL_OPEN")
-		err = c.writeDataChannelAck()
-		if err != nil {
-			return fmt.Errorf("failed to ACK channel open: %v", err)
-		}
-		// Note: DATA_CHANNEL_OPEN message is handled inside Server() method.
-		// Therefore, the message will not reach here.
-
 	case *channelAck:
 		c.log.Debug("Received DATA_CHANNEL_ACK")
-		err = c.commitReliabilityParams()
-		if err != nil {
+		if err = c.commitReliabilityParams(); err != nil {
 			return err
 		}
-		// TODO: handle ChannelAck (https://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09#section-5.2)
-
 	default:
-		return fmt.Errorf("unhandled DataChannel message %v", msg)
+		return fmt.Errorf("%w %s", ErrInvalidMessageType, msg)
 	}
 
 	return nil
@@ -305,12 +285,11 @@ func (c *DataChannel) writeDataChannelAck() error {
 	ack := channelAck{}
 	ackMsg, err := ack.Marshal()
 	if err != nil {
-		return fmt.Errorf("failed to marshal ChannelOpen ACK: %v", err)
+		return fmt.Errorf("failed to marshal ChannelOpen ACK: %w", err)
 	}
 
-	_, err = c.stream.WriteSCTP(ackMsg, sctp.PayloadTypeWebRTCDCEP)
-	if err != nil {
-		return fmt.Errorf("failed to send ChannelOpen ACK: %v", err)
+	if _, err = c.stream.WriteSCTP(ackMsg, sctp.PayloadTypeWebRTCDCEP); err != nil {
+		return fmt.Errorf("failed to send ChannelOpen ACK: %w", err)
 	}
 
 	return err
@@ -371,7 +350,7 @@ func (c *DataChannel) commitReliabilityParams() error {
 	case ChannelTypePartialReliableTimedUnordered:
 		c.stream.SetReliabilityParams(true, sctp.ReliabilityTypeTimed, c.Config.ReliabilityParameter)
 	default:
-		return fmt.Errorf("invalid ChannelType: %v ", c.Config.ChannelType)
+		return fmt.Errorf("%w %v", ErrInvalidChannelType, c.Config.ChannelType)
 	}
 	return nil
 }

datachannel_test.go

@@ -161,12 +161,12 @@ func prOrderedTest(t *testing.T, channelType ChannelType) {
 
 	binary.BigEndian.PutUint32(sbuf, 1)
 	n, err = dc0.WriteDataChannel(sbuf, true)
-	assert.Nil(t, err, "Read() should succeed")
+	assert.Nil(t, err, "WriteDataChannel() should succeed")
 	assert.Equal(t, len(sbuf), n, "data length should match")
 
 	binary.BigEndian.PutUint32(sbuf, 2)
 	n, err = dc0.WriteDataChannel(sbuf, true)
-	assert.Nil(t, err, "Read() should succeed")
+	assert.Nil(t, err, "WriteDataChannel() should succeed")
 	assert.Equal(t, len(sbuf), n, "data length should match")
 
 	time.Sleep(100 * time.Millisecond)
@@ -636,3 +636,51 @@ func TestStats(t *testing.T) {
 
 	closeAssociationPair(br, a0, a1)
 }
+
+func TestDataChannelAcceptWrite(t *testing.T) {
+	loggerFactory := logging.NewDefaultLoggerFactory()
+
+	br := test.NewBridge()
+
+	in := []byte("HELLO WORLD")
+	out := make([]byte, 100)
+
+	a0, a1, err := createNewAssociationPair(br)
+	if !assert.Nil(t, err, "failed to create associations") {
+		assert.FailNow(t, "failed due to earlier error")
+	}
+
+	cfg := &Config{
+		ChannelType:          ChannelTypeReliable,
+		ReliabilityParameter: 123,
+		Label:                "data",
+		LoggerFactory:        loggerFactory,
+	}
+
+	dc0, err := Dial(a0, 100, cfg)
+	assert.NoError(t, err, "Dial() should succeed")
+	bridgeProcessAtLeastOne(br)
+
+	dc1, err := Accept(a1, &Config{
+		LoggerFactory: loggerFactory,
+	})
+	assert.NoError(t, err, "Accept() should succeed")
+	bridgeProcessAtLeastOne(br)
+
+	n, err := dc1.WriteDataChannel(in, true)
+	assert.Nil(t, err)
+	assert.Equal(t, len(in), n)
+	bridgeProcessAtLeastOne(br)
+
+	n, isString, err := dc0.ReadDataChannel(out)
+	assert.NoError(t, err)
+	assert.True(t, isString)
+	assert.Equal(t, len(in), n)
+	assert.Equal(t, in, out[:n])
+
+	assert.NoError(t, dc0.Close())
+	assert.NoError(t, dc1.Close())
+	bridgeProcessAtLeastOne(br)
+
+	closeAssociationPair(br, a0, a1)
+}

errors.go

@@ -0,0 +1,24 @@
+package datachannel
+
+import "errors"
+
+var (
+	// ErrDataChannelMessageTooShort means that the data isn't long enough to be a valid DataChannel message
+	ErrDataChannelMessageTooShort = errors.New("DataChannel message is not long enough to determine type")
+
+	// ErrInvalidPayloadProtocolIdentifier means that we got a DataChannel messages with a Payload Protocol Identifier
+	// we don't know how to handle
+	ErrInvalidPayloadProtocolIdentifier = errors.New("DataChannel message Payload Protocol Identifier is value we can't handle")
+
+	// ErrInvalidChannelType means that the remote requested a channel type that we don't support
+	ErrInvalidChannelType = errors.New("invalid Channel Type")
+
+	// ErrInvalidMessageType is returned when a DataChannel Message has a type we don't support
+	ErrInvalidMessageType = errors.New("invalid Message Type")
+
+	// ErrExpectedAndActualLengthMismatch is when the declared length and actual length don't match
+	ErrExpectedAndActualLengthMismatch = errors.New("expected and actual length do not match")
+
+	// ErrUnexpectedDataChannelType is when a message type does not match the expected type
+	ErrUnexpectedDataChannelType = errors.New("expected and actual message type does not match")
+)

message.go

@@ -33,7 +33,7 @@ func (t messageType) String() string {
 // parse accepts raw input and returns a DataChannel message
 func parse(raw []byte) (message, error) {
 	if len(raw) == 0 {
-		return nil, fmt.Errorf("DataChannel message is not long enough to determine type ")
+		return nil, ErrDataChannelMessageTooShort
 	}
 
 	var msg message
@@ -43,7 +43,7 @@ func parse(raw []byte) (message, error) {
 	case dataChannelAck:
 		msg = &channelAck{}
 	default:
-		return nil, fmt.Errorf("Unknown MessageType %v", messageType(raw[0]))
+		return nil, fmt.Errorf("%w %v", ErrInvalidMessageType, messageType(raw[0]))
 	}
 
 	if err := msg.Unmarshal(raw); err != nil {
@@ -57,11 +57,11 @@ func parse(raw []byte) (message, error) {
 // or throws an error
 func parseExpectDataChannelOpen(raw []byte) (*channelOpen, error) {
 	if len(raw) == 0 {
-		return nil, fmt.Errorf("the DataChannel message is not long enough to determine type")
+		return nil, ErrDataChannelMessageTooShort
 	}
 
 	if actualTyp := messageType(raw[0]); actualTyp != dataChannelOpen {
-		return nil, fmt.Errorf("expected DataChannelOpen but got %s", actualTyp)
+		return nil, fmt.Errorf("%w expected(%s) actual(%s)", ErrUnexpectedDataChannelType, actualTyp, dataChannelOpen)
 	}
 
 	msg := &channelOpen{}

message_channel_open.go

@@ -103,7 +103,7 @@ func (c *channelOpen) Marshal() ([]byte, error) {
 // Unmarshal populates the struct with the given raw data
 func (c *channelOpen) Unmarshal(raw []byte) error {
 	if len(raw) < channelOpenHeaderLength {
-		return fmt.Errorf("Length of input is not long enough to satisfy header %d", len(raw))
+		return fmt.Errorf("%w expected(%d) actual(%d)", ErrExpectedAndActualLengthMismatch, channelOpenHeaderLength, len(raw))
 	}
 	c.ChannelType = ChannelType(raw[1])
 	c.Priority = binary.BigEndian.Uint16(raw[2:])
@@ -112,8 +112,8 @@ func (c *channelOpen) Unmarshal(raw []byte) error {
 	labelLength := binary.BigEndian.Uint16(raw[8:])
 	protocolLength := binary.BigEndian.Uint16(raw[10:])
 
-	if len(raw) != int(channelOpenHeaderLength+labelLength+protocolLength) {
-		return fmt.Errorf("Label + Protocol length don't match full packet length")
+	if expectedLen := int(channelOpenHeaderLength + labelLength + protocolLength); len(raw) != expectedLen {
+		return fmt.Errorf("%w expected(%d) actual(%d)", ErrExpectedAndActualLengthMismatch, expectedLen, len(raw))
 	}
 
 	c.Label = raw[channelOpenHeaderLength : channelOpenHeaderLength+labelLength]