increase test coverage on oscar package

handler/icbm_test.go

@@ -280,7 +280,7 @@ func TestICBMService_ClientEventHandler(t *testing.T) {
 					RequestID: 1234,
 				},
 				Body: oscar.SNAC_0x04_0x14_ICBMClientEvent{
-					Cookie:     [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
+					Cookie:     12345678,
 					ChannelID:  42,
 					ScreenName: "recipient-screen-name",
 					Event:      12,
@@ -293,7 +293,7 @@ func TestICBMService_ClientEventHandler(t *testing.T) {
 					RequestID: 1234,
 				},
 				Body: oscar.SNAC_0x04_0x14_ICBMClientEvent{
-					Cookie:     [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
+					Cookie:     12345678,
 					ChannelID:  42,
 					ScreenName: "sender-screen-name",
 					Event:      12,

oscar/decode.go

@@ -4,10 +4,13 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 	"reflect"
 )
 
+var ErrUnmarshalFailure = errors.New("failed to unmarshal")
+
 func Unmarshal(v any, r io.Reader) error {
 	return unmarshal(reflect.TypeOf(v).Elem(), reflect.ValueOf(v).Elem(), "", r)
 }
@@ -20,6 +23,7 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 				return err
 			}
 		}
+		return nil
 	case reflect.String:
 		var bufLen int
 		if lenTag, ok := tag.Lookup("len_prefix"); ok {
@@ -37,10 +41,10 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 				}
 				bufLen = int(l)
 			default:
-				panic("invalid len_prefix")
+				return fmt.Errorf("%w: unsupported len_prefix type %s. allowed types: uint8, uint16", ErrUnmarshalFailure, lenTag)
 			}
 		} else {
-			panic("string length not set")
+			return fmt.Errorf("%w: missing len_prefix tag", ErrUnmarshalFailure)
 		}
 		buf := make([]byte, bufLen)
 		if _, err := r.Read(buf); err != nil {
@@ -48,30 +52,35 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 		}
 		// todo is there a more efficient way?
 		v.SetString(string(buf))
+		return nil
 	case reflect.Uint8:
 		var l uint8
 		if err := binary.Read(r, binary.BigEndian, &l); err != nil {
 			return err
 		}
 		v.Set(reflect.ValueOf(l))
+		return nil
 	case reflect.Uint16:
 		var l uint16
 		if err := binary.Read(r, binary.BigEndian, &l); err != nil {
 			return err
 		}
 		v.Set(reflect.ValueOf(l))
+		return nil
 	case reflect.Uint32:
 		var l uint32
 		if err := binary.Read(r, binary.BigEndian, &l); err != nil {
 			return err
 		}
 		v.Set(reflect.ValueOf(l))
+		return nil
 	case reflect.Uint64:
 		var l uint64
 		if err := binary.Read(r, binary.BigEndian, &l); err != nil {
 			return err
 		}
 		v.Set(reflect.ValueOf(l))
+		return nil
 	case reflect.Slice:
 		if lenTag, ok := tag.Lookup("len_prefix"); ok {
 			var bufLen int
@@ -89,7 +98,7 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 				}
 				bufLen = int(l)
 			default:
-				panic("length not set")
+				return fmt.Errorf("%w: unsupported len_prefix type %s. allowed types: uint8, uint16", ErrUnmarshalFailure, lenTag)
 			}
 
 			buf := make([]byte, bufLen)
@@ -98,6 +107,9 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 			}
 			b := bytes.NewBuffer(buf)
 			slice := reflect.New(v.Type()).Elem()
+			// todo: if this is a slice of scalars, there should be no need to
+			//  call Unmarshal on each element. it should be possible to just
+			//  call binary.Read(r, binary.BigEndian, []byte)
 			for b.Len() > 0 {
 				v1 := reflect.New(v.Type().Elem()).Interface()
 				if err := Unmarshal(v1, b); err != nil {
@@ -122,7 +134,7 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 				}
 				count = int(l)
 			default:
-				panic("count not set")
+				return fmt.Errorf("%w: unsupported count_prefix type %s. allowed types: uint8, uint16", ErrUnmarshalFailure, lenTag)
 			}
 
 			slice := reflect.New(v.Type()).Elem()
@@ -148,19 +160,8 @@ func unmarshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, r io.Read
 			}
 			v.Set(slice)
 		}
-	case reflect.Array:
-		buf := make([]byte, v.Len())
-		if _, err := r.Read(buf); err != nil {
-			return err
-		}
-		array := reflect.New(v.Type()).Elem()
-		for j := 0; j < len(buf); j++ {
-			array.Index(j).SetUint(uint64(buf[j]))
-		}
-		v.Set(array)
+		return nil
 	default:
-		return errors.New("unsupported type for unmarshalling")
+		return fmt.Errorf("%w: unsupported type %v", ErrUnmarshalFailure, t.Kind())
 	}
-
-	return nil
 }

oscar/decode_test.go

@@ -0,0 +1,365 @@
+package oscar
+
+import (
+	"bytes"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUnmarshal(t *testing.T) {
+	tests := []struct {
+		name      string
+		prototype any
+		given     []byte
+		want      any
+		wantErr   error
+	}{
+		{
+			name: "uint8",
+			prototype: &struct {
+				Val uint8
+			}{},
+			want: &struct {
+				Val uint8
+			}{
+				Val: 100,
+			},
+			given: []byte{0x64},
+		},
+		{
+			name: "uint8 with read error",
+			prototype: &struct {
+				Val uint8
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "uint16",
+			prototype: &struct {
+				Val uint16
+			}{},
+			want: &struct {
+				Val uint16
+			}{
+				Val: 100,
+			},
+			given: []byte{0x0, 0x64},
+		},
+		{
+			name: "uint16 with read error",
+			prototype: &struct {
+				Val uint16
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "uint32",
+			prototype: &struct {
+				Val uint32
+			}{},
+			want: &struct {
+				Val uint32
+			}{
+				Val: 100,
+			},
+			given: []byte{0x0, 0x0, 0x0, 0x64},
+		},
+		{
+			name: "uint32 with read error",
+			prototype: &struct {
+				Val uint32
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "uint64",
+			prototype: &struct {
+				Val uint64
+			}{},
+			want: &struct {
+				Val uint64
+			}{
+				Val: 100,
+			},
+			given: []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x64},
+		},
+		{
+			name: "uint64 with read error",
+			prototype: &struct {
+				Val uint64
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "string8",
+			prototype: &struct {
+				Val string `len_prefix:"uint8"`
+			}{},
+			want: &struct {
+				Val string `len_prefix:"uint8"`
+			}{
+				Val: "test-value",
+			},
+			given: append(
+				[]byte{0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "string8 read error",
+			prototype: &struct {
+				Val string `len_prefix:"uint8"`
+			}{},
+			given:   []byte{},
+			wantErr: io.EOF,
+		},
+		{
+			name: "string16",
+			prototype: &struct {
+				Val string `len_prefix:"uint16"`
+			}{},
+			want: &struct {
+				Val string `len_prefix:"uint16"`
+			}{
+				Val: "test-value",
+			},
+			given: append(
+				[]byte{0x0, 0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "string16 read error",
+			prototype: &struct {
+				Val string `len_prefix:"uint16"`
+			}{},
+			given:   []byte{},
+			wantErr: io.EOF,
+		},
+		{
+			name: "unsupported string prefix type",
+			prototype: &struct {
+				Val string `len_prefix:"uint128"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x0, 0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "string with missing len_prefix",
+			prototype: &struct {
+				Val string
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x0, 0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "partial string8",
+			prototype: &struct {
+				Val string `len_prefix:"uint8"`
+			}{},
+			wantErr: io.EOF,
+			given: append(
+				[]byte{0xa},  /* len prefix */
+				[]byte{}...), /* truncated payload */
+		},
+		{
+			name: "byte slice with uint8 len_prefix",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint8"`
+			}{},
+			want: &struct {
+				Val []byte `len_prefix:"uint8"`
+			}{
+				Val: []byte(`hello`),
+			},
+			given: append(
+				[]byte{0x05},                             /* len prefix */
+				[]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f}...), /* slice val */
+		},
+		{
+			name: "slice of invalid type with uint8 len_prefix",
+			prototype: &struct {
+				Val []int `len_prefix:"uint8"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given:   []byte{0x68, 0x65, 0x6c, 0x6c, 0x6f},
+		},
+		{
+			name: "byte slice with uint8 len_prefix with read error",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint8"`
+			}{},
+			wantErr: io.EOF,
+			given: append(
+				[]byte{0x05}, /* len prefix */
+				[]byte{}...), /* slice val */
+		},
+		{
+			name: "byte slice with uint8 len_prefix read error",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint8"`
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "byte slice with uint16 len_prefix",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint16"`
+			}{},
+			want: &struct {
+				Val []byte `len_prefix:"uint16"`
+			}{
+				Val: []byte(`hello`),
+			},
+			given: append(
+				[]byte{0x00, 0x05},                       /* len prefix */
+				[]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f}...), /* slice val */
+		},
+		{
+			name: "byte slice with uint16 len_prefix read error",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint16"`
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "byte slice with invalid len_prefix",
+			prototype: &struct {
+				Val []byte `len_prefix:"uint128"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x00, 0x05},                       /* len prefix */
+				[]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f}...), /* slice val */
+		},
+		{
+			name: "struct slice without prefix",
+			prototype: &struct {
+				Val []TLV
+			}{},
+			want: &struct {
+				Val []TLV
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+					NewTLV(20, uint16(1234)),
+				},
+			},
+			given: []byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2},
+		},
+		{
+			name: "slice of unsupported type without prefix",
+			prototype: &struct {
+				Val []int
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given:   []byte{0x0, 0xa, 0x0, 0x2},
+		},
+		{
+			name: "struct slice with uint8 count_prefix",
+			prototype: &struct {
+				Val []TLV `count_prefix:"uint8"`
+			}{},
+			want: &struct {
+				Val []TLV `count_prefix:"uint8"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+					NewTLV(20, uint16(1234)),
+				},
+			},
+			given: append(
+				[]byte{0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint8 count_prefix and unsupported type",
+			prototype: &struct {
+				Val []struct {
+					Val int16
+				} `count_prefix:"uint8"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint8 count_prefix read error",
+			prototype: &struct {
+				Val []TLV `count_prefix:"uint8"`
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "struct slice with uint16 count_prefix",
+			prototype: &struct {
+				Val []TLV `count_prefix:"uint16"`
+			}{},
+			want: &struct {
+				Val []TLV `count_prefix:"uint16"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+					NewTLV(20, uint16(1234)),
+				},
+			},
+			given: append(
+				[]byte{0x0, 0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint16 count_prefix and unsupported type",
+			prototype: &struct {
+				Val []struct {
+					Val int16
+				} `count_prefix:"uint16"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x0, 0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint16 count_prefix read error",
+			prototype: &struct {
+				Val []TLV `count_prefix:"uint16"`
+			}{},
+			wantErr: io.EOF,
+			given:   []byte{},
+		},
+		{
+			name: "struct slice with invalid count_prefix",
+			prototype: &struct {
+				Val []TLV `count_prefix:"uint128"`
+			}{},
+			wantErr: ErrUnmarshalFailure,
+			given: append(
+				[]byte{0x0, 0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			r := bytes.NewBuffer(tt.given)
+
+			err := Unmarshal(tt.prototype, r)
+			assert.ErrorIs(t, err, tt.wantErr)
+			if tt.wantErr == nil {
+				assert.Equal(t, tt.want, tt.prototype)
+			}
+		})
+	}
+}

oscar/encode.go

@@ -4,10 +4,13 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 	"reflect"
 )
 
+var ErrMarshalFailure = errors.New("failed to marshal")
+
 func Marshal(v any, w io.Writer) error {
 	return marshal(reflect.TypeOf(v), reflect.ValueOf(v), "", w)
 }
@@ -20,9 +23,10 @@ func marshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, w io.Writer
 				return err
 			}
 		}
+		return nil
 	case reflect.String:
-		if l, ok := tag.Lookup("len_prefix"); ok {
-			switch l {
+		if lenTag, ok := tag.Lookup("len_prefix"); ok {
+			switch lenTag {
 			case "uint8":
 				if err := binary.Write(w, binary.BigEndian, uint8(len(v.String()))); err != nil {
 					return err
@@ -32,12 +36,10 @@ func marshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, w io.Writer
 					return err
 				}
 			default:
-				panic("length not set")
+				return fmt.Errorf("%w: unsupported len_prefix type %s. allowed types: uint8, uint16", ErrMarshalFailure, lenTag)
 			}
 		}
-		if err := binary.Write(w, binary.BigEndian, []byte(v.String())); err != nil {
-			return err
-		}
+		return binary.Write(w, binary.BigEndian, []byte(v.String()))
 	case reflect.Slice:
 		// todo: only write to temporary buffer if len_prefix is set
 		buf := &bytes.Buffer{}
@@ -50,11 +52,13 @@ func marshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, w io.Writer
 			}
 		} else {
 			if err := binary.Write(buf, binary.BigEndian, v.Interface()); err != nil {
-				return err
+				return fmt.Errorf("%w: error marshalling %s", ErrMarshalFailure, t.Elem().Kind())
 			}
 		}
-		//todo what if both len_prefix and count_prefix are set?
+
+		var hasLenPrefix bool
 		if l, ok := tag.Lookup("len_prefix"); ok {
+			hasLenPrefix = true
 			switch l {
 			case "uint8":
 				if err := binary.Write(w, binary.BigEndian, uint8(buf.Len())); err != nil {
@@ -64,9 +68,14 @@ func marshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, w io.Writer
 				if err := binary.Write(w, binary.BigEndian, uint16(buf.Len())); err != nil {
 					return err
 				}
+			default:
+				return fmt.Errorf("%w: unsupported len_prefix type %s. allowed types: uint8, uint16", ErrMarshalFailure, l)
 			}
 		}
 		if l, ok := tag.Lookup("count_prefix"); ok {
+			if hasLenPrefix {
+				return fmt.Errorf("%w: struct elem has both len_prefix and count_prefix: ", ErrMarshalFailure)
+			}
 			switch l {
 			case "uint8":
 				if err := binary.Write(w, binary.BigEndian, uint8(v.Len())); err != nil {
@@ -76,20 +85,18 @@ func marshal(t reflect.Type, v reflect.Value, tag reflect.StructTag, w io.Writer
 				if err := binary.Write(w, binary.BigEndian, uint16(v.Len())); err != nil {
 					return err
 				}
+			default:
+				return fmt.Errorf("%w: unsupported count_prefix type %s. allowed types: uint8, uint16", ErrMarshalFailure, l)
 			}
 		}
 		if buf.Len() > 0 {
-			if _, err := w.Write(buf.Bytes()); err != nil {
-				return err
-			}
-		}
-	case reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Array:
-		if err := binary.Write(w, binary.BigEndian, v.Interface()); err != nil {
+			_, err := w.Write(buf.Bytes())
 			return err
 		}
+		return nil
+	case reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
+		return binary.Write(w, binary.BigEndian, v.Interface())
 	default:
-		return errors.New("unsupported type for marshalling")
+		return fmt.Errorf("%w: unsupported type %v", ErrMarshalFailure, t.Kind())
 	}
-
-	return nil
 }

oscar/encode_test.go

@@ -0,0 +1,338 @@
+package oscar
+
+import (
+	"bytes"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// errWriter is a writer that always returns an error.
+type errWriter struct{}
+
+func (errWriter) Write(p []byte) (n int, err error) {
+	return 0, io.EOF
+}
+
+func TestMarshal(t *testing.T) {
+	tests := []struct {
+		name    string
+		w       io.Writer
+		given   any
+		want    []byte
+		wantErr error
+	}{
+		{
+			name: "marshal uint8",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val uint8
+			}{
+				Val: 100,
+			},
+			want: []byte{0x64},
+		},
+		{
+			name: "marshal uint16",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val uint16
+			}{
+				Val: 100,
+			},
+			want: []byte{0x0, 0x64},
+		},
+		{
+			name: "marshal uint32",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val uint32
+			}{
+				Val: 100,
+			},
+			want: []byte{0x0, 0x0, 0x0, 0x64},
+		},
+		{
+			name: "marshal uint64",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val uint64
+			}{
+				Val: 100,
+			},
+			want: []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x64},
+		},
+		{
+			name: "unsupported type",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val int16
+			}{
+				Val: 100,
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "string8",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val string `len_prefix:"uint8"`
+			}{
+				Val: "test-value",
+			},
+			want: append(
+				[]byte{0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "string8 write error",
+			w:    errWriter{},
+			given: struct {
+				Val string `len_prefix:"uint8"`
+			}{
+				Val: "test-value",
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "string16",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val string `len_prefix:"uint16"`
+			}{
+				Val: "test-value",
+			},
+			want: append(
+				[]byte{0x0, 0xa}, /* len prefix */
+				[]byte{0x74, 0x65, 0x73, 0x74, 0x2d, 0x76, 0x61, 0x6c, 0x75, 0x65}...), /* str val */
+		},
+		{
+			name: "string16 write error",
+			w:    errWriter{},
+			given: struct {
+				Val string `len_prefix:"uint16"`
+			}{
+				Val: "test-value",
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "string with unknown prefix type",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val string `len_prefix:"uint128"`
+			}{
+				Val: "test-value",
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "byte slice with no prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte
+			}{
+				Val: []byte(`hello`),
+			},
+			want: []byte{0x68, 0x65, 0x6c, 0x6c, 0x6f},
+		},
+		{
+			name: "byte slice with no prefix with write error",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []int
+			}{
+				Val: []int{1, 2, 3, 4},
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "byte slice with no prefix with write error",
+			w:    errWriter{},
+			given: struct {
+				Val []byte
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "empty byte slice",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte
+			}{
+				Val: []byte{},
+			},
+			want: []byte(nil),
+		},
+		{
+			name: "byte slice with uint8 len_prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte `len_prefix:"uint8"`
+			}{
+				Val: []byte(`hello`),
+			},
+			want: append(
+				[]byte{0x05},                             /* len prefix */
+				[]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f}...), /* slice val */
+		},
+		{
+			name: "byte slice with uint8 len_prefix with error",
+			w:    errWriter{},
+			given: struct {
+				Val []byte `len_prefix:"uint8"`
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "byte slice with uint16 len_prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte `len_prefix:"uint16"`
+			}{
+				Val: []byte(`hello`),
+			},
+			want: append(
+				[]byte{0x00, 0x05},                       /* len prefix */
+				[]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f}...), /* slice val */
+		},
+		{
+			name: "byte slice with uint16 len_prefix with error",
+			w:    errWriter{},
+			given: struct {
+				Val []byte `len_prefix:"uint16"`
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "empty struct slice",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []TLV
+			}{
+				Val: []TLV{},
+			},
+			want: []byte(nil),
+		},
+		{
+			name: "struct slice with invalid type in struct",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []struct {
+					Val int16
+				}
+			}{
+				Val: []struct {
+					Val int16
+				}{
+					{
+						Val: 1234,
+					},
+				},
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "struct slice with uint8 count_prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []TLV `count_prefix:"uint8"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+					NewTLV(20, uint16(1234)),
+				},
+			},
+			want: append(
+				[]byte{0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint8 count_prefix with error",
+			w:    errWriter{},
+			given: struct {
+				Val []TLV `count_prefix:"uint8"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+				},
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "struct slice with uint16 count_prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []TLV `count_prefix:"uint16"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+					NewTLV(20, uint16(1234)),
+				},
+			},
+			want: append(
+				[]byte{0x00, 0x02}, /* count prefix */
+				[]byte{0x0, 0xa, 0x0, 0x2, 0x4, 0xd2, 0x0, 0x14, 0x0, 0x2, 0x4, 0xd2}...), /* slice val */
+		},
+		{
+			name: "struct slice with uint16 count_prefix with error",
+			w:    errWriter{},
+			given: struct {
+				Val []TLV `count_prefix:"uint16"`
+			}{
+				Val: []TLV{
+					NewTLV(10, uint16(1234)),
+				},
+			},
+			wantErr: io.EOF,
+		},
+		{
+			name: "byte slice with uint16 len_prefix and uint16 count_prefix",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte `len_prefix:"uint16" count_prefix:"uint16"`
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "byte slice with unknown len_prefix type",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte `len_prefix:"uint128"`
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: ErrMarshalFailure,
+		},
+		{
+			name: "byte slice with unknown count_prefix type",
+			w:    &bytes.Buffer{},
+			given: struct {
+				Val []byte `count_prefix:"uint128"`
+			}{
+				Val: []byte(`hello`),
+			},
+			wantErr: ErrMarshalFailure,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := Marshal(tt.given, tt.w)
+			assert.ErrorIs(t, err, tt.wantErr)
+			if tt.wantErr == nil {
+				if w, ok := tt.w.(*bytes.Buffer); ok {
+					assert.Equal(t, tt.want, w.Bytes())
+				}
+			}
+		})
+	}
+}

oscar/frames.go

@@ -25,7 +25,7 @@ type FLAPFrame struct {
 	PayloadLength uint16
 }
 
-func (f FLAPFrame) SNACBuffer(r io.Reader) (*bytes.Buffer, error) {
+func (f FLAPFrame) ReadBody(r io.Reader) (*bytes.Buffer, error) {
 	b := make([]byte, f.PayloadLength)
 	if _, err := r.Read(b); err != nil {
 		return nil, err

oscar/frames_test.go

@@ -0,0 +1,28 @@
+package oscar
+
+import (
+	"bytes"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFLAPFrame_ReadBody(t *testing.T) {
+	flap := FLAPFrame{
+		PayloadLength: 4,
+	}
+	bufIn := bytes.NewBuffer([]byte{0, 1, 2, 3, 4, 5})
+	buf, err := flap.ReadBody(bufIn)
+	assert.NoError(t, err)
+	assert.Equal(t, []byte{0, 1, 2, 3}, buf.Bytes())
+}
+
+func TestFLAPFrame_ReadBodyError(t *testing.T) {
+	flap := FLAPFrame{
+		PayloadLength: 4,
+	}
+	bufIn := &bytes.Buffer{}
+	_, err := flap.ReadBody(bufIn)
+	assert.ErrorIs(t, err, io.EOF)
+}

oscar/snacs.go

@@ -430,14 +430,14 @@ type SNAC_0x04_0x05_ICBMParameterReply struct {
 }
 
 type SNAC_0x04_0x06_ICBMChannelMsgToHost struct {
-	Cookie     [8]byte
+	Cookie     uint64
 	ChannelID  uint16
 	ScreenName string `len_prefix:"uint8"`
 	TLVRestBlock
 }
 
 type SNAC_0x04_0x07_ICBMChannelMsgToClient struct {
-	Cookie    [8]byte
+	Cookie    uint64
 	ChannelID uint16
 	TLVUserInfo
 	TLVRestBlock
@@ -454,7 +454,7 @@ type SNAC_0x04_0x09_ICBMEvilReply struct {
 }
 
 type SNAC_0x04_0x0B_ICBMClientErr struct {
-	Cookie     [8]byte
+	Cookie     uint64
 	ChannelID  uint16
 	ScreenName string `len_prefix:"uint8"`
 	Code       uint16
@@ -462,13 +462,13 @@ type SNAC_0x04_0x0B_ICBMClientErr struct {
 }
 
 type SNAC_0x04_0x0C_ICBMHostAck struct {
-	Cookie     [8]byte
+	Cookie     uint64
 	ChannelID  uint16
 	ScreenName string `len_prefix:"uint8"`
 }
 
 type SNAC_0x04_0x14_ICBMClientEvent struct {
-	Cookie     [8]byte
+	Cookie     uint64
 	ChannelID  uint16
 	ScreenName string `len_prefix:"uint8"`
 	Event      uint16

oscar/snacs_test.go

@@ -1,146 +0,0 @@
-package oscar
-
-import (
-	"bytes"
-	"reflect"
-	"testing"
-)
-
-func TestMarshal(t *testing.T) {
-	snac := SNAC_0x0E_0x03_ChatUsersJoined{
-		Users: []TLVUserInfo{
-			{
-				ScreenName: "screenname1",
-			},
-			{
-				ScreenName: "screenname2",
-			},
-		},
-	}
-
-	buf1 := &bytes.Buffer{}
-	if err := Marshal(snac, buf1); err != nil {
-		t.Fatalf("error: %s", err.Error())
-	}
-	t.Log(buf1)
-}
-
-func TestUnmarshal(t *testing.T) {
-	snac1 := SNAC_0x04_0x07_ICBMChannelMsgToClient{
-		Cookie:    [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
-		ChannelID: 4,
-		TLVUserInfo: TLVUserInfo{
-			ScreenName:   "myscreenname",
-			WarningLevel: 100,
-		},
-		TLVRestBlock: TLVRestBlock{
-			TLVList: TLVList{
-				{
-					Tag:   0x0B,
-					Value: []byte{1, 2, 3, 4},
-				},
-			},
-		},
-	}
-
-	buf := &bytes.Buffer{}
-	if err := Marshal(snac1, buf); err != nil {
-		t.Fatalf("error: %s", err.Error())
-	}
-
-	snac2 := SNAC_0x04_0x07_ICBMChannelMsgToClient{}
-	if err := Unmarshal(&snac2, buf); err != nil {
-		t.Fatalf("error: %s", err.Error())
-	}
-
-	if !reflect.DeepEqual(snac1, snac2) {
-		t.Fatalf("structs are not the same: %v %v", snac1, snac2)
-	}
-}
-
-type DummyContainerLen struct {
-	Elems []struct {
-		Name string `len_prefix:"uint8"`
-	} `len_prefix:"uint8"`
-}
-
-func TestDummyContainerLen(t *testing.T) {
-
-	x := DummyContainerLen{
-		Elems: []struct {
-			Name string `len_prefix:"uint8"`
-		}{
-			{"Mike"},
-			{"John"},
-			{"Jay"},
-		},
-	}
-
-	buf := &bytes.Buffer{}
-	if err := Marshal(x, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	y := DummyContainerLen{}
-	if err := Unmarshal(&y, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	if !reflect.DeepEqual(x, y) {
-		t.Fatal("structs are not the same")
-	}
-}
-
-type DummyContainerRest struct {
-	Elems []struct {
-		Name string `len_prefix:"uint8"`
-	}
-}
-
-func TestDummyContainerRest(t *testing.T) {
-
-	x := DummyContainerRest{
-		Elems: []struct {
-			Name string `len_prefix:"uint8"`
-		}{
-			{"Mike"},
-			{"John"},
-			{"Jay"},
-		},
-	}
-
-	buf := &bytes.Buffer{}
-	if err := Marshal(x, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	y := DummyContainerRest{}
-	if err := Unmarshal(&y, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	if !reflect.DeepEqual(x, y) {
-		t.Fatal("structs are not the same")
-	}
-}
-
-func TestSNAC_0x01_0x17_OServiceClientVersions(t *testing.T) {
-
-	x := SNAC_0x01_0x17_OServiceClientVersions{
-		Versions: []uint16{1, 2, 3},
-	}
-
-	buf := &bytes.Buffer{}
-	if err := Marshal(x, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	y := SNAC_0x01_0x17_OServiceClientVersions{}
-	if err := Unmarshal(&y, buf); err != nil {
-		t.Fatal(err.Error())
-	}
-
-	if !reflect.DeepEqual(x, y) {
-		t.Fatal("structs are not the same")
-	}
-}

oscar/tlv_test.go

@@ -198,3 +198,11 @@ func TestTLVList_Getters(t *testing.T) {
 		})
 	}
 }
+
+func TestTLVList_NewTLVPanic(t *testing.T) {
+	// make sure NewTLV panics when it encounters an unsupported type, in this
+	// case it's int.
+	assert.Panics(t, func() {
+		NewTLV(1, 30)
+	})
+}

server/alert_test.go

@@ -87,7 +87,7 @@ func TestAlertRouter_RouteAlert(t *testing.T) {
 			// make sure the sequence number was incremented
 			assert.Equal(t, uint32(2), seq)
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame

server/bos_service_test.go

@@ -43,7 +43,7 @@ func TestBOSService_handleNewConnection(t *testing.T) {
 		// < receive FLAPSignonFrame
 		flap := oscar.FLAPFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flap, serverReader))
-		buf, err := flap.SNACBuffer(serverReader)
+		buf, err := flap.ReadBody(serverReader)
 		assert.NoError(t, err)
 		flapSignonFrame := oscar.FLAPSignonFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flapSignonFrame, buf))
@@ -67,7 +67,7 @@ func TestBOSService_handleNewConnection(t *testing.T) {
 		// < receive SNAC_0x01_0x03_OServiceHostOnline
 		flap = oscar.FLAPFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flap, serverReader))
-		buf, err = flap.SNACBuffer(serverReader)
+		buf, err = flap.ReadBody(serverReader)
 		assert.NoError(t, err)
 		frame := oscar.SNACFrame{}
 		assert.NoError(t, oscar.Unmarshal(&frame, buf))

server/bucp.go

@@ -113,7 +113,7 @@ func flapSignonHandshake(rw io.ReadWriter, sequence *uint32) (oscar.FLAPSignonFr
 	if err := oscar.Unmarshal(&flap, rw); err != nil {
 		return oscar.FLAPSignonFrame{}, err
 	}
-	buf, err := flap.SNACBuffer(rw)
+	buf, err := flap.ReadBody(rw)
 	if err != nil {
 		return oscar.FLAPSignonFrame{}, err
 	}

server/bucp_test.go

@@ -19,7 +19,7 @@ func TestBUCPAuthService_handleNewConnection(t *testing.T) {
 		// < receive FLAPSignonFrame
 		flap := oscar.FLAPFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flap, serverReader))
-		buf, err := flap.SNACBuffer(serverReader)
+		buf, err := flap.ReadBody(serverReader)
 		assert.NoError(t, err)
 		flapSignonFrame := oscar.FLAPSignonFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flapSignonFrame, buf))

server/buddy_test.go

@@ -95,7 +95,7 @@ func TestBuddyRouter_RouteBuddy(t *testing.T) {
 			assert.Equal(t, seq, uint32(1))
 			assert.Equal(t, flap.Sequence, uint16(0))
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame

server/chat_nav_test.go

@@ -151,7 +151,7 @@ func TestChatNavRouter_RouteChatNavRouter(t *testing.T) {
 			assert.Equal(t, seq, uint32(1))
 			assert.Equal(t, flap.Sequence, uint16(0))
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame

server/chat_service_test.go

@@ -29,7 +29,7 @@ func TestChatService_handleNewConnection(t *testing.T) {
 		// < receive FLAPSignonFrame
 		flap := oscar.FLAPFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flap, serverReader))
-		buf, err := flap.SNACBuffer(serverReader)
+		buf, err := flap.ReadBody(serverReader)
 		assert.NoError(t, err)
 		flapSignonFrame := oscar.FLAPSignonFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flapSignonFrame, buf))
@@ -57,7 +57,7 @@ func TestChatService_handleNewConnection(t *testing.T) {
 		// < receive SNAC_0x01_0x03_OServiceHostOnline
 		flap = oscar.FLAPFrame{}
 		assert.NoError(t, oscar.Unmarshal(&flap, serverReader))
-		buf, err = flap.SNACBuffer(serverReader)
+		buf, err = flap.ReadBody(serverReader)
 		assert.NoError(t, err)
 		frame := oscar.SNACFrame{}
 		assert.NoError(t, oscar.Unmarshal(&frame, buf))

server/chat_test.go

@@ -111,7 +111,7 @@ func TestChatRouter_RouteChat(t *testing.T) {
 			assert.Equal(t, seq, uint32(1))
 			assert.Equal(t, flap.Sequence, uint16(0))
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame

server/connection.go

@@ -62,7 +62,7 @@ func receiveSNAC(frame *oscar.SNACFrame, body any, r io.Reader) error {
 	if err := oscar.Unmarshal(&flap, r); err != nil {
 		return err
 	}
-	buf, err := flap.SNACBuffer(r)
+	buf, err := flap.ReadBody(r)
 	if err != nil {
 		return err
 	}

server/feedbag_test.go

@@ -294,7 +294,7 @@ func TestFeedbagRouter_RouteFeedbag(t *testing.T) {
 			assert.Equal(t, seq, uint32(1))
 			assert.Equal(t, flap.Sequence, uint16(0))
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame

server/oservice_test.go

@@ -279,7 +279,7 @@ func TestOServiceRouter_RouteOService_ForBOS(t *testing.T) {
 			// make sure the sequence number was incremented
 			assert.Equal(t, uint32(2), seq)
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame
@@ -557,7 +557,7 @@ func TestOServiceRouter_RouteOService_ForChat(t *testing.T) {
 			// make sure the sequence number was incremented
 			assert.Equal(t, uint32(2), seq)
 
-			flapBuf, err := flap.SNACBuffer(bufOut)
+			flapBuf, err := flap.ReadBody(bufOut)
 			assert.NoError(t, err)
 
 			// verify the SNAC frame