Refactor unpacker and add some more snap tests

chunk7/chunk.go

@@ -1,5 +1,11 @@
 package chunk7
 
+import (
+	"fmt"
+
+	"github.com/teeworlds-go/go-teeworlds-protocol/packer"
+)
+
 const (
 	chunkFlagVital  = 1
 	chunkFlagResend = 2
@@ -49,13 +55,49 @@ func (header *ChunkHeader) Pack() []byte {
 	return data
 }
 
-func (header *ChunkHeader) Unpack(data []byte) {
+func (header *ChunkHeader) Unpack(u *packer.Unpacker) error {
+	data, err := u.GetRaw(2)
+	if err != nil {
+		return err
+	}
+	flagBits := (data[0] >> 6) & 0x03
+	header.Flags.Vital = (flagBits & chunkFlagVital) != 0
+	header.Flags.Resend = (flagBits & chunkFlagResend) != 0
+	header.Size = (int(data[0]&0x3F) << 6) | (int(data[1]) & 0x3F)
+
+	if header.Flags.Vital {
+		thirdByte, err := u.GetByte()
+		if err != nil {
+			return err
+		}
+		header.Seq = int((data[1]&0xC0)<<2) | int(thirdByte)
+	}
+	return nil
+}
+
+// TODO: give this a better name
+//
+//	but it would be nice to have two unpack methods for all structs
+//
+//	Unpack(u *packer.Unpacker)
+//	UnpackTodoFindAGoodName(data []byte)
+//
+//	See https://github.com/teeworlds-go/go-teeworlds-protocol/issues/5
+func (header *ChunkHeader) UnpackRaw(data []byte) error {
+	if len(data) < 2 {
+		return fmt.Errorf("size=%d not enough data to read chunk header", len(data))
+	}
+
 	flagBits := (data[0] >> 6) & 0x03
 	header.Flags.Vital = (flagBits & chunkFlagVital) != 0
 	header.Flags.Resend = (flagBits & chunkFlagResend) != 0
 	header.Size = (int(data[0]&0x3F) << 6) | (int(data[1]) & 0x3F)
 
 	if header.Flags.Vital {
+		if len(data) < 3 {
+			return fmt.Errorf("size=%d not enough data to read vital chunk header", len(data))
+		}
 		header.Seq = int((data[1]&0xC0)<<2) | int(data[2])
 	}
+	return nil
 }

chunk7/chunk_test.go

@@ -12,7 +12,7 @@ func TestBrokenNonVitalHeader(t *testing.T) {
 
 	header := ChunkHeader{}
 	//            {0x40, 0x3a, 0x01}
-	header.Unpack([]byte{0x3a, 0x01})
+	header.UnpackRaw([]byte{0x3a, 0x01})
 
 	want := ChunkHeader{
 		Flags: ChunkFlags{
@@ -33,7 +33,7 @@ func TestVitalHeaderMapChange(t *testing.T) {
 	// verified with libtw2 wireshark dissector
 
 	header := ChunkHeader{}
-	header.Unpack([]byte{0x40, 0x3a, 0x01})
+	header.UnpackRaw([]byte{0x40, 0x3a, 0x01})
 
 	want := ChunkHeader{
 		Flags: ChunkFlags{
@@ -51,7 +51,7 @@ func TestVitalHeaderMapChange(t *testing.T) {
 
 func TestVitalHeader(t *testing.T) {
 	header := ChunkHeader{}
-	header.Unpack([]byte{0x40, 0x10, 0x0a})
+	header.UnpackRaw([]byte{0x40, 0x10, 0x0a})
 	want := ChunkHeader{
 		Flags: ChunkFlags{
 			Vital:  true,

chunk7/splitter.go

@@ -11,7 +11,8 @@ func UnpackChunks(data []byte) []Chunk {
 
 	for i < payloadSize {
 		chunk := Chunk{}
-		chunk.Header.Unpack(data[i:])
+		// TODO: can we use ChunkHeader.Unpack(u) here to simplify the code?
+		chunk.Header.UnpackRaw(data[i:])
 		i += 2 // header
 		if chunk.Header.Flags.Vital {
 			i++

messages7/cl_say.go

@@ -43,7 +43,7 @@ func (msg *ClSay) Pack() []byte {
 func (msg *ClSay) Unpack(u *packer.Unpacker) error {
 	msg.Mode = network7.ChatMode(u.GetInt())
 	msg.TargetId = u.GetInt()
-	msg.Message = u.GetString()
+	msg.Message, _ = u.GetString()
 
 	return nil
 }

messages7/cl_start_info.go

@@ -77,15 +77,15 @@ func (msg *ClStartInfo) Pack() []byte {
 }
 
 func (msg *ClStartInfo) Unpack(u *packer.Unpacker) error {
-	msg.Name = u.GetString()
-	msg.Clan = u.GetString()
+	msg.Name, _ = u.GetString()
+	msg.Clan, _ = u.GetString()
 	msg.Country = u.GetInt()
-	msg.Body = u.GetString()
-	msg.Marking = u.GetString()
-	msg.Decoration = u.GetString()
-	msg.Hands = u.GetString()
-	msg.Feet = u.GetString()
-	msg.Eyes = u.GetString()
+	msg.Body, _ = u.GetString()
+	msg.Marking, _ = u.GetString()
+	msg.Decoration, _ = u.GetString()
+	msg.Hands, _ = u.GetString()
+	msg.Feet, _ = u.GetString()
+	msg.Eyes, _ = u.GetString()
 	msg.CustomColorBody = u.GetInt() != 0
 	msg.CustomColorMarking = u.GetInt() != 0
 	msg.CustomColorDecoration = u.GetInt() != 0

messages7/ctrl_close.go

@@ -37,7 +37,7 @@ func (msg *CtrlClose) Pack() []byte {
 
 func (msg *CtrlClose) Unpack(u *packer.Unpacker) error {
 	// TODO: sanitize
-	msg.Reason = u.GetString()
+	msg.Reason, _ = u.GetString()
 	return nil
 }
 

messages7/info.go

@@ -59,8 +59,8 @@ func (msg *Info) Pack() []byte {
 }
 
 func (msg *Info) Unpack(u *packer.Unpacker) error {
-	msg.Version = u.GetString()
-	msg.Password = u.GetString()
+	msg.Version, _ = u.GetString()
+	msg.Password, _ = u.GetString()
 	msg.ClientVersion = u.GetInt()
 	return nil
 }

messages7/map_change.go

@@ -47,7 +47,7 @@ func (msg *MapChange) Pack() []byte {
 }
 
 func (msg *MapChange) Unpack(u *packer.Unpacker) error {
-	msg.Name = u.GetString()
+	msg.Name, _ = u.GetString()
 	msg.Crc = u.GetInt()
 	msg.Size = u.GetInt()
 	msg.NumResponseChunksPerRequest = u.GetInt()

messages7/maplist_entry_add.go

@@ -37,7 +37,7 @@ func (msg *MaplistEntryAdd) Pack() []byte {
 }
 
 func (msg *MaplistEntryAdd) Unpack(u *packer.Unpacker) error {
-	msg.MapName = u.GetString()
+	msg.MapName, _ = u.GetString()
 	return nil
 }
 

messages7/maplist_entry_rem.go

@@ -37,7 +37,7 @@ func (msg *MaplistEntryRem) Pack() []byte {
 }
 
 func (msg *MaplistEntryRem) Unpack(u *packer.Unpacker) error {
-	msg.MapName = u.GetString()
+	msg.MapName, _ = u.GetString()
 	return nil
 }
 

messages7/rcon_auth.go

@@ -37,7 +37,7 @@ func (msg *RconAuth) Pack() []byte {
 }
 
 func (msg *RconAuth) Unpack(u *packer.Unpacker) error {
-	msg.Password = u.GetString()
+	msg.Password, _ = u.GetString()
 	return nil
 }
 

messages7/rcon_cmd.go

@@ -37,7 +37,7 @@ func (msg *RconCmd) Pack() []byte {
 }
 
 func (msg *RconCmd) Unpack(u *packer.Unpacker) error {
-	msg.Command = u.GetString()
+	msg.Command, _ = u.GetString()
 	return nil
 }
 

messages7/rcon_cmd_add.go

@@ -41,9 +41,9 @@ func (msg *RconCmdAdd) Pack() []byte {
 }
 
 func (msg *RconCmdAdd) Unpack(u *packer.Unpacker) error {
-	msg.Name = u.GetString()
-	msg.Help = u.GetString()
-	msg.Params = u.GetString()
+	msg.Name, _ = u.GetString()
+	msg.Help, _ = u.GetString()
+	msg.Params, _ = u.GetString()
 	return nil
 }
 

messages7/rcon_cmd_rem.go

@@ -37,7 +37,7 @@ func (msg *RconCmdRem) Pack() []byte {
 }
 
 func (msg *RconCmdRem) Unpack(u *packer.Unpacker) error {
-	msg.Name = u.GetString()
+	msg.Name, _ = u.GetString()
 	return nil
 }
 

messages7/rcon_line.go

@@ -37,7 +37,7 @@ func (msg *RconLine) Pack() []byte {
 }
 
 func (msg *RconLine) Unpack(u *packer.Unpacker) error {
-	msg.Line = u.GetString()
+	msg.Line, _ = u.GetString()
 	return nil
 }
 

messages7/server_info.go

@@ -57,11 +57,11 @@ func (msg *ServerInfo) Pack() []byte {
 }
 
 func (msg *ServerInfo) Unpack(u *packer.Unpacker) error {
-	msg.Version = u.GetString()
-	msg.Name = u.GetString()
-	msg.Hostname = u.GetString()
-	msg.MapName = u.GetString()
-	msg.GameType = u.GetString()
+	msg.Version, _ = u.GetString()
+	msg.Name, _ = u.GetString()
+	msg.Hostname, _ = u.GetString()
+	msg.MapName, _ = u.GetString()
+	msg.GameType, _ = u.GetString()
 	msg.Flags = u.GetInt()
 	msg.SkillLevel = u.GetInt()
 	msg.PlayerCount = u.GetInt()

messages7/sv_broadcast.go

@@ -33,7 +33,7 @@ func (msg *SvBroadcast) Pack() []byte {
 }
 
 func (msg *SvBroadcast) Unpack(u *packer.Unpacker) error {
-	msg.Message = u.GetString()
+	msg.Message, _ = u.GetString()
 	return nil
 }
 

messages7/sv_chat.go

@@ -46,7 +46,7 @@ func (msg *SvChat) Unpack(u *packer.Unpacker) error {
 	msg.Mode = network7.ChatMode(u.GetInt())
 	msg.ClientId = u.GetInt()
 	msg.TargetId = u.GetInt()
-	msg.Message = u.GetString()
+	msg.Message, _ = u.GetString()
 
 	return nil
 }

messages7/sv_client_info.go

@@ -86,15 +86,15 @@ func (info *SvClientInfo) Unpack(u *packer.Unpacker) error {
 	info.ClientId = u.GetInt()
 	info.Local = u.GetInt() != 0
 	info.Team = u.GetInt()
-	info.Name = u.GetString()
-	info.Clan = u.GetString()
+	info.Name, _ = u.GetString()
+	info.Clan, _ = u.GetString()
 	info.Country = u.GetInt()
-	info.Body = u.GetString()
-	info.Marking = u.GetString()
-	info.Decoration = u.GetString()
-	info.Hands = u.GetString()
-	info.Feet = u.GetString()
-	info.Eyes = u.GetString()
+	info.Body, _ = u.GetString()
+	info.Marking, _ = u.GetString()
+	info.Decoration, _ = u.GetString()
+	info.Hands, _ = u.GetString()
+	info.Feet, _ = u.GetString()
+	info.Eyes, _ = u.GetString()
 	info.CustomColorBody = u.GetInt() != 0
 	info.CustomColorMarking = u.GetInt() != 0
 	info.CustomColorDecoration = u.GetInt() != 0

messages7/sv_emoticon.go

@@ -11,6 +11,7 @@ import (
 type SvEmoticon struct {
 	ChunkHeader *chunk7.ChunkHeader
 
+	ClientId int
 	Emoticon network7.Emote
 }
 
@@ -32,11 +33,13 @@ func (msg *SvEmoticon) Vital() bool {
 
 func (msg *SvEmoticon) Pack() []byte {
 	return slices.Concat(
+		packer.PackInt(msg.ClientId),
 		packer.PackInt(int(msg.Emoticon)),
 	)
 }
 
 func (msg *SvEmoticon) Unpack(u *packer.Unpacker) error {
+	msg.ClientId = u.GetInt()
 	msg.Emoticon = network7.Emote(u.GetInt())
 
 	return nil

messages7/sv_emoticon_test.go

@@ -0,0 +1,88 @@
+package messages7_test
+
+import (
+	"testing"
+
+	"github.com/teeworlds-go/go-teeworlds-protocol/internal/testutils/require"
+	"github.com/teeworlds-go/go-teeworlds-protocol/messages7"
+	"github.com/teeworlds-go/go-teeworlds-protocol/network7"
+	"github.com/teeworlds-go/go-teeworlds-protocol/packer"
+	"github.com/teeworlds-go/go-teeworlds-protocol/protocol7"
+)
+
+func TestFullPacket(t *testing.T) {
+	packet := protocol7.Packet{}
+	packet.Messages = append(
+		packet.Messages,
+		&messages7.SvEmoticon{
+			ClientId: 0,
+			Emoticon: network7.EmoteGhost,
+		},
+	)
+
+	{
+		// if this test breaks because the session tokens are actually used
+		// this is not necessarily a bad thing
+		session := &protocol7.Session{
+			ServerToken: [4]byte{0x55, 0x55, 0x55, 0x55},
+			ClientToken: [4]byte{0xfa, 0xfa, 0xfa, 0xfa},
+		}
+
+		want := []byte{0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x40, 0x03, 0x01, 0x14, 0x00, 0x07}
+		got := packet.Pack(session)
+
+		require.Equal(t, want, got)
+	}
+}
+
+func TestSvEmoticonStandalone(t *testing.T) {
+	// simple pack
+	emoticon := &messages7.SvEmoticon{
+		ClientId: 0,
+		Emoticon: network7.EmoteExclamation,
+	}
+
+	{
+		want := []byte{0x00, 0x01}
+		got := emoticon.Pack()
+
+		require.Equal(t, want, got)
+	}
+
+	// repack
+	u := &packer.Unpacker{}
+	u.Reset(emoticon.Pack())
+	emoticon.Unpack(u)
+
+	{
+		want := network7.EmoteExclamation
+		got := emoticon.Emoticon
+		require.Equal(t, want, got)
+	}
+}
+
+func TestSvEmoticonStandaloneCrazyGirlEdition(t *testing.T) {
+	// simple pack
+	emoticon := &messages7.SvEmoticon{
+		ClientId: -99999,
+		Emoticon: 999,
+	}
+
+	{
+		want := []byte{222, 154, 12, 167, 15}
+		got := emoticon.Pack()
+
+		require.Equal(t, want, got)
+	}
+
+	// repack
+	u := &packer.Unpacker{}
+	u.Reset(emoticon.Pack())
+	emoticon.Unpack(u)
+
+	{
+		want := network7.Emote(999)
+		got := emoticon.Emoticon
+		require.Equal(t, want, got)
+	}
+}