Skip to content

Commit 7f683a6

Browse files
robinbraemersvetch
andauthored
fix(network): fix AvailableCommands packet for mods (minekube#534)
* fix(network): fix AvailableCommands packet for mods Fixes: minekube#322 Closes: minekube#527 Co-authored-by: @uberswe * test(util): enhance ReadVarInt tests with additional cases and fuzzing - Expanded unit tests for ReadVarInt to cover various scenarios including edge cases like maximum varint, empty buffer, and incomplete varint. - Added fuzz testing for ReadVarInt to ensure robustness against malformed input. - Introduced a benchmark for ReadVarInt to measure performance. * revert to original ReadVarInt * update go.sum * fix: lint --------- Co-authored-by: Román Benjámin <svetch.game@gmail.com>
1 parent cc649ab commit 7f683a6

3 files changed

Lines changed: 132 additions & 15 deletions

File tree

go.sum

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ github.com/gammazero/deque v1.0.0/go.mod h1:iflpYvtGfM3U8S8j+sZEKIak3SAKYpA5/SQe
5151
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
5252
github.com/gliderlabs/ssh v0.1.1/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0=
5353
github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q=
54-
github.com/go-faker/faker/v4 v4.5.0 h1:ARzAY2XoOL9tOUK+KSecUQzyXQsUaZHefjyF8x6YFHc=
55-
github.com/go-faker/faker/v4 v4.5.0/go.mod h1:p3oq1GRjG2PZ7yqeFFfQI20Xm61DoBDlCA8RiSyZ48M=
5654
github.com/go-faker/faker/v4 v4.6.0 h1:6aOPzNptRiDwD14HuAnEtlTa+D1IfFuEHO8+vEFwjTs=
5755
github.com/go-faker/faker/v4 v4.6.0/go.mod h1:ZmrHuVtTTm2Em9e0Du6CJ9CADaLEzGXW62z1YqFH0m0=
5856
github.com/go-faker/faker/v4 v4.6.1 h1:xUyVpAjEtB04l6XFY0V/29oR332rOSPWV4lU8RwDt4k=
@@ -246,8 +244,6 @@ golang.org/x/net v0.0.0-20181106065722-10aee1819953/go.mod h1:mL1N/T3taQHkDXs73r
246244
golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
247245
golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
248246
golang.org/x/net v0.0.0-20190313220215-9f648a60d977/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
249-
golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8=
250-
golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8=
251247
golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY=
252248
golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E=
253249
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
@@ -259,8 +255,6 @@ golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJ
259255
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
260256
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
261257
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
262-
golang.org/x/sync v0.12.0 h1:MHc5BpPuC30uJk597Ri8TV3CNZcTLu6B6z4lJy+g6Jw=
263-
golang.org/x/sync v0.12.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
264258
golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610=
265259
golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
266260
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
@@ -272,8 +266,6 @@ golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20=
272266
golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
273267
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
274268
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
275-
golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY=
276-
golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4=
277269
golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0=
278270
golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU=
279271
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=

pkg/edition/java/proto/packet/brigadier/registry.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (r *argPropReg) Encode(wr io.Writer, argType brigodier.ArgumentType, protoc
6161
if err != nil {
6262
return err
6363
}
64-
return util.WriteBytes(wr, property.Data)
64+
return util.WriteRawBytes(wr, property.Data)
6565
default:
6666
codec, ok := r.byType[argType.String()]
6767
id, ok2 := r.typeToID[argType.String()]

pkg/edition/java/proto/util/util_test.go

Lines changed: 131 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,73 @@ package util
22

33
import (
44
"bytes"
5+
"errors"
6+
"io"
7+
"testing"
8+
59
"github.com/stretchr/testify/require"
610
"go.minekube.com/gate/pkg/util/uuid"
7-
"testing"
811
)
912

1013
func TestVarInt(t *testing.T) {
11-
buf := new(bytes.Buffer)
12-
require.NoError(t, WriteVarInt(buf, 1))
13-
i, err := ReadVarInt(buf)
14-
require.NoError(t, err)
15-
require.Equal(t, 1, i)
14+
t.Parallel()
15+
testCases := []struct {
16+
name string
17+
data []byte
18+
wantVal int
19+
wantErr error
20+
}{
21+
{name: "single byte", data: []byte{0x01}, wantVal: 1},
22+
{name: "two bytes", data: []byte{0xAC, 0x02}, wantVal: 300},
23+
{name: "zero", data: []byte{0x00}, wantVal: 0},
24+
{
25+
name: "max varint",
26+
data: []byte{0xff, 0xff, 0xff, 0xff, 0x07},
27+
wantVal: 2147483647, // MaxInt32
28+
},
29+
{
30+
name: "varint too big",
31+
data: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0x01},
32+
wantErr: errors.New("decode: VarInt is too big"),
33+
},
34+
{
35+
name: "empty buffer",
36+
data: []byte{},
37+
wantErr: io.EOF,
38+
},
39+
{
40+
name: "incomplete varint",
41+
data: []byte{0xff}, // Missing subsequent bytes
42+
wantErr: io.EOF,
43+
},
44+
{
45+
name: "valid 5 byte varint",
46+
data: []byte{0x80, 0x80, 0x80, 0x80, 0x01},
47+
wantVal: 268435456,
48+
},
49+
}
50+
51+
for _, tc := range testCases {
52+
t.Run(tc.name, func(t *testing.T) {
53+
buf := bytes.NewBuffer(tc.data)
54+
gotVal, gotErr := ReadVarInt(buf)
55+
56+
if tc.wantErr != nil {
57+
require.Error(t, gotErr)
58+
// Check if the error message or type matches, depending on what's more appropriate
59+
// For "VarInt is too big", we check the exact error message.
60+
// For io.EOF, we check if it's an EOF error.
61+
if tc.name == "varint too big" || tc.name == "empty buffer" || tc.name == "incomplete varint" {
62+
require.EqualError(t, gotErr, tc.wantErr.Error())
63+
} else {
64+
require.ErrorIs(t, gotErr, tc.wantErr)
65+
}
66+
} else {
67+
require.NoError(t, gotErr)
68+
require.Equal(t, tc.wantVal, gotVal)
69+
}
70+
})
71+
}
1672
}
1773

1874
func TestUTF(t *testing.T) {
@@ -24,6 +80,7 @@ func TestUTF(t *testing.T) {
2480
}
2581

2682
func TestUUIDIntArray(t *testing.T) {
83+
t.Parallel()
2784
// Generate a random UUID
2885
id := uuid.New()
2986

@@ -39,3 +96,71 @@ func TestUUIDIntArray(t *testing.T) {
3996
// The read UUID should be the same as the original UUID
4097
require.Equal(t, id, readID)
4198
}
99+
100+
func FuzzReadVarInt(f *testing.F) {
101+
testCases := [][]byte{
102+
{0x01},
103+
{0xAC, 0x02},
104+
{0x00},
105+
{0xff, 0xff, 0xff, 0xff, 0x07},
106+
{0xff, 0xff, 0xff, 0xff, 0xff, 0x01}, // too big
107+
{}, // empty
108+
{0xff}, // incomplete
109+
{0x80, 0x80, 0x80, 0x80, 0x01},
110+
}
111+
for _, tc := range testCases {
112+
f.Add(tc)
113+
}
114+
115+
f.Fuzz(func(t *testing.T, data []byte) {
116+
buf := bytes.NewBuffer(data)
117+
val, err := ReadVarInt(buf)
118+
119+
// We expect an error if the VarInt is too big or incomplete
120+
// or if the input data is malformed in a way ReadVarInt can detect.
121+
// Otherwise, we expect the read to consume some bytes and potentially return a value.
122+
123+
if err != nil {
124+
if !errors.Is(err, io.EOF) && err.Error() != "decode: VarInt is too big" {
125+
// If it's another type of error, we might want to investigate.
126+
// For now, we just ensure it doesn't panic.
127+
t.Fatalf("ReadVarInt returned an unexpected error: %v for input %x", err, data)
128+
}
129+
return // Error is expected in some cases
130+
}
131+
132+
// If no error, ensure val is non-negative as VarInts typically represent sizes or counts
133+
if val < 0 {
134+
// This case should ideally be covered by the VarInt format itself,
135+
// as standard VarInts don't represent negative numbers directly
136+
// unless a zigzag encoding is used (which is not the case here).
137+
// The current ReadVarInt decodes into an int, so it *could* be negative
138+
// if a 5-byte sequence results in a value > MaxInt32 when interpreted as uint32.
139+
// For example: []byte{0xff, 0xff, 0xff, 0xff, 0x0f} would be -1 if read as int32.
140+
// Let's verify this behavior or decide if ReadVarInt should error.
141+
// The current implementation of ReadVarInt casts to int(result) where result is int32.
142+
// If the 5th byte makes the int32 value negative, it will be negative.
143+
t.Logf("ReadVarInt returned a negative value: %d for input %x. This may be expected due to int(int32) conversion for certain 5-byte sequences.", val, data)
144+
}
145+
146+
// Check remaining bytes if any
147+
// If the read was successful (no error), some bytes should have been consumed
148+
// unless the input was empty and ReadVarInt errored out before this check.
149+
if len(data) > 0 && buf.Len() == len(data) && err == nil {
150+
t.Errorf("ReadVarInt consumed no bytes from non-empty buffer without error, val: %d", val)
151+
}
152+
})
153+
}
154+
155+
func BenchmarkReadVarInt(b *testing.B) {
156+
data := []byte{0xAC, 0x02} // 300
157+
reader := bytes.NewReader(data)
158+
b.ResetTimer()
159+
for b.Loop() {
160+
_, _ = reader.Seek(0, io.SeekStart) // Reset reader for each iteration
161+
_, err := ReadVarInt(reader)
162+
if err != nil {
163+
b.Fatal(err)
164+
}
165+
}
166+
}

0 commit comments

Comments
 (0)