Skip to content

Commit 14f4efa

Browse files
nweedon-unweedon-u
authored
fix(simple-game-server-go): Guard against panic in ReadFromUDP() (#23)
* fix(simple-game-server-go): Guard against panic in ReadFromUDP() A race condition can occur rarely where the UDP connection used for handling query responses is used when it is nil. Refactor reading and writing into `Read()` and `Write()` methods which also guard against use when the connection has been closed. * fix: Update Go to 1.19, golangci-lint to 1.48.0 * fix: Update linters
1 parent 71721ab commit 14f4efa

14 files changed

Lines changed: 89 additions & 49 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ jobs:
1818
- name: Set up Go
1919
uses: actions/setup-go@v2
2020
with:
21-
go-version: 1.17
21+
go-version: 1.19
2222
- name: Lint
2323
uses: golangci/golangci-lint-action@v2
2424
with:
25-
version: v1.39.0
25+
version: v1.48.0
2626
working-directory: './simple-game-server-go'
2727
- name: Build
2828
uses: goreleaser/goreleaser-action@v2

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,26 @@ linters:
1212
enable-all: true
1313
disable:
1414
- exhaustive
15+
- exhaustruct
1516
- exhaustivestruct
17+
- ifshort
1618
- funlen
1719
- gochecknoglobals
20+
- goerr113
21+
- golint
1822
- gomoddirectives
1923
- interfacer
2024
- maligned
2125
- gomnd
2226
- nestif
27+
- nonamedreturns
28+
- nosnakecase
2329
- paralleltest
2430
- scopelint
31+
- tagliatelle
2532
- testpackage
2633
- tparallel
34+
- varnamelen
2735
- wrapcheck
2836
- wsl
2937

‎simple-game-server-go/go.mod‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@ module github.com/Unity-Technologies/multiplay-examples/simple-game-server-go
33
go 1.17
44

55
require (
6-
github.com/fsnotify/fsnotify v1.4.9
7-
github.com/sirupsen/logrus v1.8.1
8-
github.com/stretchr/testify v1.4.0
6+
github.com/fsnotify/fsnotify v1.5.4
7+
github.com/sirupsen/logrus v1.9.0
8+
github.com/stretchr/testify v1.7.0
99
)
1010

1111
require (
1212
github.com/davecgh/go-spew v1.1.1 // indirect
1313
github.com/pmezard/go-difflib v1.0.0 // indirect
14-
golang.org/x/sys v0.0.0-20210616094352-59db8d763f22 // indirect
15-
gopkg.in/yaml.v2 v2.2.2 // indirect
16-
)
14+
golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 // indirect
15+
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
16+
)

‎simple-game-server-go/go.sum‎

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

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package game
22

3-
import "net"
3+
import (
4+
"errors"
5+
"fmt"
6+
"net"
7+
"time"
8+
)
49

510
type (
611
// udpBinding is a managed wrapper for a generic UDP listener.
@@ -28,13 +33,37 @@ func newUDPBinding(bindAddress string) (*udpBinding, error) {
2833
}, nil
2934
}
3035

36+
// Read reads data from the open connection into the supplied buffer.
37+
func (b *udpBinding) Read(buf []byte) (int, *net.UDPAddr, error) {
38+
if b.IsDone() {
39+
return 0, nil, errors.New("binding is closed")
40+
}
41+
42+
return b.conn.ReadFromUDP(buf)
43+
}
44+
45+
// Write writes data to the specified UDP address.
46+
func (b *udpBinding) Write(buf []byte, to *net.UDPAddr) (int, error) {
47+
if b.IsDone() {
48+
return 0, errors.New("binding is closed")
49+
}
50+
51+
if err := b.conn.SetWriteDeadline(time.Now().Add(1 * time.Second)); err != nil {
52+
return 0, fmt.Errorf("error setting write deadline: %w", err)
53+
}
54+
55+
return b.conn.WriteTo(buf, to)
56+
}
57+
3158
// Close marks the binding as complete, closing any open connections.
3259
func (b *udpBinding) Close() {
33-
if b.conn != nil {
34-
close(b.done)
35-
b.conn.Close()
36-
b.conn = nil
60+
if b.IsDone() {
61+
return
3762
}
63+
64+
close(b.done)
65+
b.conn.Close()
66+
b.conn = nil
3867
}
3968

4069
// IsDone determines whether the binding is complete.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func Test_BindLifecycle(t *testing.T) {
2424
require.NoError(t, err)
2525

2626
actual := make([]byte, len(expected))
27-
_, _, err = b.conn.ReadFromUDP(actual)
27+
_, _, err = b.Read(actual)
2828
require.NoError(t, err)
2929
require.Equal(t, expected, actual)
3030

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package game
22

33
import (
4-
"io/ioutil"
54
"net/http"
5+
"os"
66
"path"
77
"testing"
88
"time"
@@ -16,7 +16,7 @@ func Test_watchConfig(t *testing.T) {
1616
l := logrus.NewEntry(logrus.New())
1717
p := path.Join(t.TempDir(), "config.json")
1818

19-
require.NoError(t, ioutil.WriteFile(p, []byte(`{}`), 0o600))
19+
require.NoError(t, os.WriteFile(p, []byte(`{}`), 0o600))
2020

2121
g, err := New(l, p, 9000, 9001, &http.Client{Timeout: 1 * time.Second})
2222
require.NoError(t, err)
@@ -26,7 +26,7 @@ func Test_watchConfig(t *testing.T) {
2626
<-g.internalEventProcessorReady
2727

2828
// Allocate
29-
require.NoError(t, ioutil.WriteFile(p, []byte(`{
29+
require.NoError(t, os.WriteFile(p, []byte(`{
3030
"allocatedUUID": "alloc-uuid",
3131
"maxPlayers": "12"
3232
}`), 0o600))
@@ -37,7 +37,7 @@ func Test_watchConfig(t *testing.T) {
3737
require.Equal(t, "sqp", ev.Config.QueryType)
3838

3939
// Deallocate
40-
require.NoError(t, ioutil.WriteFile(p, []byte(`{
40+
require.NoError(t, os.WriteFile(p, []byte(`{
4141
"allocatedUUID": ""
4242
}`), 0o600))
4343
ev = <-g.gameEvents

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"net"
55
"net/http"
66
"sync"
7-
"time"
87

98
"github.com/Unity-Technologies/multiplay-examples/simple-game-server-go/pkg/config"
109
"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
148147

149148
for {
150149
buf := make([]byte, size)
151-
_, to, err := b.conn.ReadFromUDP(buf)
150+
_, to, err := b.Read(buf)
152151
if err != nil {
153152
if b.IsDone() {
154153
return
@@ -170,18 +169,16 @@ func handleQuery(q proto.QueryResponder, logger *logrus.Entry, wg *sync.WaitGrou
170169
continue
171170
}
172171

173-
if err = b.conn.SetWriteDeadline(time.Now().Add(1 * time.Second)); err != nil {
174-
logger.
175-
WithField("error", err.Error()).
176-
Error("error setting write deadline")
177-
178-
continue
179-
}
172+
if _, err = b.Write(resp, to); err != nil {
173+
if b.IsDone() {
174+
return
175+
}
180176

181-
if _, err = b.conn.WriteTo(resp, to); err != nil {
182177
logger.
183178
WithField("error", err.Error()).
184179
Error("error writing response")
180+
181+
continue
185182
}
186183
}
187184
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ func (g *Game) switchQueryProtocol(c config.Config) error {
221221
func (g *Game) restartQueryEndpoint(c config.Config) error {
222222
if g.queryBind != nil {
223223
g.queryBind.Close()
224+
g.queryBind = nil
224225
}
225226

226227
var err error

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (g *Game) approveBackfillTicket() (*http.Response, error) {
6262
func (g *Game) getJwtToken() (string, error) {
6363
payloadProxyTokenURL := fmt.Sprintf("%s/token", g.backfillParams.PayloadProxyURL)
6464

65-
req, err := http.NewRequestWithContext(context.Background(), "GET", payloadProxyTokenURL, http.NoBody)
65+
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, payloadProxyTokenURL, http.NoBody)
6666
if err != nil {
6767
return "", err
6868
}
@@ -105,7 +105,7 @@ func (g *Game) updateBackfillAllocation(token string) (*http.Response, error) {
105105
g.backfillParams.MatchmakerURL,
106106
g.backfillParams.AllocatedUUID)
107107

108-
req, err := http.NewRequestWithContext(context.Background(), "POST", backfillApprovalURL, http.NoBody)
108+
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, backfillApprovalURL, http.NoBody)
109109
if err != nil {
110110
return nil, err
111111
}

0 commit comments

Comments
 (0)