Handle zero length comment and file paths for Nostalgia compatibility

diff --git a/hotline/file_path.go b/hotline/file_path.go
index f7da29818e8bfc2489442609aeaf99f3316fac60..3ff243557f9b96a69a2d5be6a952fc48f1f03614 100644 (file)
--- a/hotline/file_path.go
+++ b/hotline/file_path.go
@@ -4,6 +4,7 @@ import (
        "bytes"
        "encoding/binary"
        "errors"
+       "io"
        "path"
        "strings"
 )
@@ -19,37 +20,39 @@ type FilePathItem struct {
        Name []byte
 }
 
-func NewFilePathItem(b []byte) FilePathItem {
-       return FilePathItem{
-               Len:  b[2],
-               Name: b[3:],
-       }
-}
-
 type FilePath struct {
        ItemCount [2]byte
        Items     []FilePathItem
 }
 
-const minFilePathLen = 2
-
 func (fp *FilePath) UnmarshalBinary(b []byte) error {
-       if b == nil {
-               return nil
-       }
-       if len(b) < minFilePathLen {
-               return errors.New("insufficient bytes")
-       }
-       err := binary.Read(bytes.NewReader(b[0:2]), binary.BigEndian, &fp.ItemCount)
-       if err != nil {
+       reader := bytes.NewReader(b)
+       err := binary.Read(reader, binary.BigEndian, &fp.ItemCount)
+       if err != nil && !errors.Is(err, io.EOF) {
                return err
        }
+       if errors.Is(err, io.EOF) {
+               return nil
+       }
 
-       pathData := b[2:]
        for i := uint16(0); i < fp.Len(); i++ {
-               segLen := pathData[2]
-               fp.Items = append(fp.Items, NewFilePathItem(pathData[:segLen+3]))
-               pathData = pathData[3+segLen:]
+               // skip two bytes for the file path delimiter
+               _, _ = reader.Seek(2, io.SeekCurrent)
+
+               // read the length of the next pathItem
+               segLen, err := reader.ReadByte()
+               if err != nil {
+                       return err
+               }
+
+               pBytes := make([]byte, segLen)
+
+               _, err = reader.Read(pBytes)
+               if err != nil && !errors.Is(err, io.EOF) {
+                       return err
+               }
+
+               fp.Items = append(fp.Items, FilePathItem{Len: segLen, Name: pBytes})
        }
 
        return nil

diff --git a/hotline/file_path_test.go b/hotline/file_path_test.go
index 4c2f6db80c855e1741595ab5e1210b358afda4b4..f25941ebc55bc27ccd7a0e4eb01185a8b0a5deff 100644 (file)
--- a/hotline/file_path_test.go
+++ b/hotline/file_path_test.go
@@ -41,6 +41,17 @@ func TestFilePath_UnmarshalBinary(t *testing.T) {
                        },
                        wantErr: false,
                },
+               {
+                       name: "handles empty data payload",
+                       args: args{b: []byte{
+                               0x00, 0x00,
+                       }},
+                       want: FilePath{
+                               ItemCount: [2]byte{0x00, 0x00},
+                               Items:     []FilePathItem(nil),
+                       },
+                       wantErr: false,
+               },
        }
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
@@ -78,7 +89,7 @@ func Test_readPath(t *testing.T) {
                                        0x61, 0x61, 0x61,
                                },
                        },
-                       want: "",
+                       want:    "",
                        wantErr: true,
                },
                {
@@ -87,7 +98,6 @@ func Test_readPath(t *testing.T) {
                                fileRoot: "/usr/local/var/mobius/Files",
                                filePath: nil,
                                fileName: []byte("foo"),
-
                        },
                        want: "/usr/local/var/mobius/Files/foo",
                },
@@ -153,4 +163,4 @@ func Test_readPath(t *testing.T) {
                        }
                })
        }
-}
\ No newline at end of file
+}

diff --git a/hotline/flattened_file_object.go b/hotline/flattened_file_object.go
index 6f38e465367d72c10ca37302d333d37d2ec868c9..a43677b2da4a8e59302a5466aafa44e63079190e 100644 (file)
--- a/hotline/flattened_file_object.go
+++ b/hotline/flattened_file_object.go
@@ -110,20 +110,10 @@ type FlatFileDataForkHeader struct {
 }
 
 func (ffif *FlatFileInformationFork) UnmarshalBinary(b []byte) error {
-
        nameSize := b[70:72]
        bs := binary.BigEndian.Uint16(nameSize)
-
        nameEnd := 72 + bs
 
-       commentSize := b[nameEnd : nameEnd+2]
-       commentLen := binary.BigEndian.Uint16(commentSize)
-
-       commentStartPos := int(nameEnd) + 2
-       commentEndPos := int(nameEnd) + 2 + int(commentLen)
-
-       comment := b[commentStartPos:commentEndPos]
-
        ffif.Platform = b[0:4]
        ffif.TypeSignature = b[4:8]
        ffif.CreatorSignature = b[8:12]
@@ -135,8 +125,16 @@ func (ffif *FlatFileInformationFork) UnmarshalBinary(b []byte) error {
        ffif.NameScript = b[68:70]
        ffif.NameSize = b[70:72]
        ffif.Name = b[72:nameEnd]
-       ffif.CommentSize = b[nameEnd : nameEnd+2]
-       ffif.Comment = comment
+
+       if len(b) > int(nameEnd) {
+               ffif.CommentSize = b[nameEnd : nameEnd+2]
+               commentLen := binary.BigEndian.Uint16(ffif.CommentSize)
+
+               commentStartPos := int(nameEnd) + 2
+               commentEndPos := int(nameEnd) + 2 + int(commentLen)
+
+               ffif.Comment = b[commentStartPos:commentEndPos]
+       }
 
        return nil
 }

diff --git a/hotline/flattened_file_object_test.go b/hotline/flattened_file_object_test.go
index 9a57e7ed49211a8403432168bedb85e25d7f591d..1d438188bb1380f1b4ec7a0e4e6f58b3b8728bc2 100644 (file)
--- a/hotline/flattened_file_object_test.go
+++ b/hotline/flattened_file_object_test.go
@@ -2,6 +2,7 @@ package hotline
 
 import (
        "fmt"
+       "github.com/davecgh/go-spew/spew"
        "github.com/stretchr/testify/assert"
        "os"
        "testing"
@@ -65,3 +66,41 @@ func TestNewFlattenedFileObject(t *testing.T) {
                })
        }
 }
+
+func TestFlatFileInformationFork_UnmarshalBinary(t *testing.T) {
+       type args struct {
+               b []byte
+       }
+       tests := []struct {
+               name    string
+               args    args
+               wantErr assert.ErrorAssertionFunc
+       }{
+               {
+                       name: "when zero length comment size is omitted (Nostalgia client behavior)",
+                       args: args{
+                               b: []byte{
+                                       0x41, 0x4d, 0x41, 0x43, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x09, 0x62, 0x65, 0x61, 0x72, 0x2e, 0x74, 0x69, 0x66, 0x66,
+                               },
+                       },
+                       wantErr: assert.NoError,
+               },
+               {
+                       name: "when zero length comment size is included",
+                       args: args{
+                               b: []byte{
+                                       0x41, 0x4d, 0x41, 0x43, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x09, 0x62, 0x65, 0x61, 0x72, 0x2e, 0x74, 0x69, 0x66, 0x66, 0x00, 0x00,
+                               },
+                       },
+                       wantErr: assert.NoError,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       ffif := &FlatFileInformationFork{}
+                       tt.wantErr(t, ffif.UnmarshalBinary(tt.args.b), fmt.Sprintf("UnmarshalBinary(%v)", tt.args.b))
+
+                       spew.Dump(ffif)
+               })
+       }
+}