fix(simple-game-server-go): Guard against panic in ReadFromUDP()

.github/workflows/simple-game-server-go.yml

@@ -18,11 +18,11 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: 1.17
+          go-version: 1.19
       - name: Lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.39.0
+          version: v1.48.0
           working-directory: './simple-game-server-go'
       - name: Build
         uses: goreleaser/goreleaser-action@v2

simple-game-server-go/.golangci.yml

@@ -12,18 +12,26 @@ linters:
   enable-all: true
   disable:
     - exhaustive
+    - exhaustruct
     - exhaustivestruct
+    - ifshort
     - funlen
     - gochecknoglobals
+    - goerr113
+    - golint
     - gomoddirectives
     - interfacer
     - maligned
     - gomnd
     - nestif
+    - nonamedreturns
+    - nosnakecase
     - paralleltest
     - scopelint
+    - tagliatelle
     - testpackage
     - tparallel
+    - varnamelen
     - wrapcheck
     - wsl
 

simple-game-server-go/go.mod

@@ -3,14 +3,14 @@ module github.com/Unity-Technologies/multiplay-examples/simple-game-server-go
 go 1.17
 
 require (
-	github.com/fsnotify/fsnotify v1.4.9
-	github.com/sirupsen/logrus v1.8.1
-	github.com/stretchr/testify v1.4.0
+	github.com/fsnotify/fsnotify v1.5.4
+	github.com/sirupsen/logrus v1.9.0
+	github.com/stretchr/testify v1.7.0
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	golang.org/x/sys v0.0.0-20210616094352-59db8d763f22 // indirect
-	gopkg.in/yaml.v2 v2.2.2 // indirect
-)
+	golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 // indirect
+	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
+)

simple-game-server-go/go.sum

@@ -1,21 +1,20 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
-github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
+github.com/fsnotify/fsnotify v1.5.4 h1:jRbGcIw6P2Meqdwuo0H1p6JVLbL5DHKAKlYndzMwVZI=
+github.com/fsnotify/fsnotify v1.5.4/go.mod h1:OVB6XrOHzAwXMpEM7uPOzcehqUV2UqJxmVXmkdnm1bU=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
-github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
+github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0=
+github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
-github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
-github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
-golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210616094352-59db8d763f22 h1:RqytpXGR1iVNX7psjB3ff8y7sNFinVFvkx1c8SjBkio=
-golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
+github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 h1:v1W7bwXHsnLLloWYTVEdvGvA7BHMeBYsPcF0GLDxIRs=
+golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
-gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

simple-game-server-go/internal/game/bind.go

@@ -1,6 +1,11 @@
 package game
 
-import "net"
+import (
+	"errors"
+	"fmt"
+	"net"
+	"time"
+)
 
 type (
 	// udpBinding is a managed wrapper for a generic UDP listener.
@@ -28,13 +33,37 @@ func newUDPBinding(bindAddress string) (*udpBinding, error) {
 	}, nil
 }
 
+// Read reads data from the open connection into the supplied buffer.
+func (b *udpBinding) Read(buf []byte) (int, *net.UDPAddr, error) {
+	if b.IsDone() {
+		return 0, nil, errors.New("binding is closed")
+	}
+
+	return b.conn.ReadFromUDP(buf)
+}
+
+// Write writes data to the specified UDP address.
+func (b *udpBinding) Write(buf []byte, to *net.UDPAddr) (int, error) {
+	if b.IsDone() {
+		return 0, errors.New("binding is closed")
+	}
+
+	if err := b.conn.SetWriteDeadline(time.Now().Add(1 * time.Second)); err != nil {
+		return 0, fmt.Errorf("error setting write deadline: %w", err)
+	}
+
+	return b.conn.WriteTo(buf, to)
+}
+
 // Close marks the binding as complete, closing any open connections.
 func (b *udpBinding) Close() {
-	if b.conn != nil {
-		close(b.done)
-		b.conn.Close()
-		b.conn = nil
+	if b.IsDone() {
+		return
 	}
+
+	close(b.done)
+	b.conn.Close()
+	b.conn = nil
 }
 
 // IsDone determines whether the binding is complete.

simple-game-server-go/internal/game/bind_test.go

@@ -24,7 +24,7 @@ func Test_BindLifecycle(t *testing.T) {
 	require.NoError(t, err)
 
 	actual := make([]byte, len(expected))
-	_, _, err = b.conn.ReadFromUDP(actual)
+	_, _, err = b.Read(actual)
 	require.NoError(t, err)
 	require.Equal(t, expected, actual)
 

simple-game-server-go/internal/game/event_loop_test.go

@@ -1,8 +1,8 @@
 package game
 
 import (
-	"io/ioutil"
 	"net/http"
+	"os"
 	"path"
 	"testing"
 	"time"
@@ -16,7 +16,7 @@ func Test_watchConfig(t *testing.T) {
 	l := logrus.NewEntry(logrus.New())
 	p := path.Join(t.TempDir(), "config.json")
 
-	require.NoError(t, ioutil.WriteFile(p, []byte(`{}`), 0o600))
+	require.NoError(t, os.WriteFile(p, []byte(`{}`), 0o600))
 
 	g, err := New(l, p, 9000, 9001, &http.Client{Timeout: 1 * time.Second})
 	require.NoError(t, err)
@@ -26,7 +26,7 @@ func Test_watchConfig(t *testing.T) {
 	<-g.internalEventProcessorReady
 
 	// Allocate
-	require.NoError(t, ioutil.WriteFile(p, []byte(`{
+	require.NoError(t, os.WriteFile(p, []byte(`{
 		"allocatedUUID": "alloc-uuid",
 		"maxPlayers": "12"
 	}`), 0o600))
@@ -37,7 +37,7 @@ func Test_watchConfig(t *testing.T) {
 	require.Equal(t, "sqp", ev.Config.QueryType)
 
 	// Deallocate
-	require.NoError(t, ioutil.WriteFile(p, []byte(`{
+	require.NoError(t, os.WriteFile(p, []byte(`{
 		"allocatedUUID": ""
 	}`), 0o600))
 	ev = <-g.gameEvents

simple-game-server-go/internal/game/game.go

@@ -4,7 +4,6 @@ import (
 	"net"
 	"net/http"
 	"sync"
-	"time"
 
 	"github.com/Unity-Technologies/multiplay-examples/simple-game-server-go/pkg/config"
 	"github.com/Unity-Technologies/multiplay-examples/simple-game-server-go/pkg/event"
@@ -148,7 +147,7 @@ func handleQuery(q proto.QueryResponder, logger *logrus.Entry, wg *sync.WaitGrou
 
 	for {
 		buf := make([]byte, size)
-		_, to, err := b.conn.ReadFromUDP(buf)
+		_, to, err := b.Read(buf)
 		if err != nil {
 			if b.IsDone() {
 				return
@@ -170,18 +169,16 @@ func handleQuery(q proto.QueryResponder, logger *logrus.Entry, wg *sync.WaitGrou
 			continue
 		}
 
-		if err = b.conn.SetWriteDeadline(time.Now().Add(1 * time.Second)); err != nil {
-			logger.
-				WithField("error", err.Error()).
-				Error("error setting write deadline")
-
-			continue
-		}
+		if _, err = b.Write(resp, to); err != nil {
+			if b.IsDone() {
+				return
+			}
 
-		if _, err = b.conn.WriteTo(resp, to); err != nil {
 			logger.
 				WithField("error", err.Error()).
 				Error("error writing response")
+
+			continue
 		}
 	}
 }

simple-game-server-go/internal/game/game_loop.go

@@ -221,6 +221,7 @@ func (g *Game) switchQueryProtocol(c config.Config) error {
 func (g *Game) restartQueryEndpoint(c config.Config) error {
 	if g.queryBind != nil {
 		g.queryBind.Close()
+		g.queryBind = nil
 	}
 
 	var err error

simple-game-server-go/internal/game/matchmaker.go

@@ -62,7 +62,7 @@ func (g *Game) approveBackfillTicket() (*http.Response, error) {
 func (g *Game) getJwtToken() (string, error) {
 	payloadProxyTokenURL := fmt.Sprintf("%s/token", g.backfillParams.PayloadProxyURL)
 
-	req, err := http.NewRequestWithContext(context.Background(), "GET", payloadProxyTokenURL, http.NoBody)
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, payloadProxyTokenURL, http.NoBody)
 	if err != nil {
 		return "", err
 	}
@@ -105,7 +105,7 @@ func (g *Game) updateBackfillAllocation(token string) (*http.Response, error) {
 		g.backfillParams.MatchmakerURL,
 		g.backfillParams.AllocatedUUID)
 
-	req, err := http.NewRequestWithContext(context.Background(), "POST", backfillApprovalURL, http.NoBody)
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, backfillApprovalURL, http.NoBody)
 	if err != nil {
 		return nil, err
 	}

simple-game-server-go/internal/game/matchmaker_test.go

@@ -4,9 +4,9 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"path"
 	"testing"
 	"time"
@@ -61,7 +61,7 @@ func Test_approveBackfillTicket(t *testing.T) {
 	}))
 	defer mmBackfillServer.Close()
 
-	require.NoError(t, ioutil.WriteFile(p, []byte(fmt.Sprintf(`{
+	require.NoError(t, os.WriteFile(p, []byte(fmt.Sprintf(`{
 			"allocatedUUID": "77c31f84-b890-48e8-be08-5db9a551bba3",
 			"matchmakerUrl": "%s",
 			"payloadProxyUrl": "%s"

simple-game-server-go/pkg/config/config_test.go

@@ -1,7 +1,7 @@
 package config
 
 import (
-	"io/ioutil"
+	"os"
 	"path"
 	"reflect"
 	"testing"
@@ -80,7 +80,7 @@ func Test_NewConfigFromFile(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := path.Join(t.TempDir(), "config.json")
-			require.NoError(t, ioutil.WriteFile(f, []byte(tt.fields.configContent), 0o600))
+			require.NoError(t, os.WriteFile(f, []byte(tt.fields.configContent), 0o600))
 
 			got, err := NewConfigFromFile(f)
 			if (err != nil) != tt.wantErr {

simple-game-server-go/pkg/proto/sqp/errors.go

@@ -13,6 +13,7 @@ type (
 )
 
 var (
+	ErrChallengeMalformed  = errors.New("challenge malformed")
 	ErrChallengeMismatch   = errors.New("challenge mismatch")
 	ErrInvalidPacketLength = errors.New("invalid packet length")
 	ErrNoChallenge         = errors.New("no challenge")

simple-game-server-go/pkg/proto/sqp/sqp.go

@@ -108,7 +108,12 @@ func (q *QueryResponder) handleQuery(clientAddress string, buf []byte) ([]byte,
 	}
 
 	// Challenge doesn't match, return with no response
-	if binary.BigEndian.Uint32(buf[1:5]) != expectedChallenge.(uint32) {
+	expectedChallengeInt, ok := expectedChallenge.(uint32)
+	if !ok {
+		return nil, ErrChallengeMalformed
+	}
+
+	if binary.BigEndian.Uint32(buf[1:5]) != expectedChallengeInt {
 		return nil, ErrChallengeMismatch
 	}
 
@@ -120,7 +125,7 @@ func (q *QueryResponder) handleQuery(clientAddress string, buf []byte) ([]byte,
 	wantsServerInfo := requestedChunks&0x1 == 1
 	f := queryWireFormat{
 		Header:     1,
-		Challenge:  expectedChallenge.(uint32),
+		Challenge:  expectedChallengeInt,
 		SQPVersion: 1,
 	}