test more combinations, including fail (old client with new server)

files/multifilereader_test.go

@@ -5,28 +5,36 @@ import (
 	"mime/multipart"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var text = "Some text! :)"
 
-func getTestMultiFileReader(t *testing.T, rawAbsPath bool) *MultiFileReader {
-	sf := NewMapDirectory(map[string]Node{
-		"file.txt": NewBytesFile([]byte(text)),
-		"boop": NewMapDirectory(map[string]Node{
-			"a.txt": NewBytesFile([]byte("bleep")),
-			"b.txt": NewBytesFile([]byte("bloop")),
-		}),
-		"beep.txt": NewBytesFile([]byte("beep")),
-	})
+func TestMultiFileReaderToMultiFile(t *testing.T) {
+	do := func(t *testing.T, binaryFileName, rawAbsPath, expectFailure bool) {
+		var (
+			filename string
+			file     File
+		)
+
+		if binaryFileName {
+			filename = "bad\x7fname.txt"
+			file = NewBytesFileWithPath("/my/path/boop/bad\x7fname.txt", []byte("bloop"))
+		} else {
+			filename = "résumé.txt"
+			file = NewBytesFileWithPath("/my/path/boop/résumé.txt", []byte("bloop"))
+		}
 
-	// testing output by reading it with the go stdlib "mime/multipart" Reader
-	return NewMultiFileReader(sf, true, rawAbsPath)
-}
+		sf := NewMapDirectory(map[string]Node{
+			"file.txt": NewBytesFileWithPath("/my/path/file.txt", []byte(text)),
+			"boop": NewMapDirectory(map[string]Node{
+				"a.txt":  NewBytesFileWithPath("/my/path/boop/a.txt", []byte("bleep")),
+				filename: file,
+			}),
+			"beep.txt": NewBytesFileWithPath("/my/path/beep.txt", []byte("beep")),
+		})
 
-func TestMultiFileReaderToMultiFile(t *testing.T) {
-	do := func(t *testing.T, rawAbsPath bool) {
-		mfr := getTestMultiFileReader(t, rawAbsPath)
+		mfr := NewMultiFileReader(sf, true, rawAbsPath)
 		mpReader := multipart.NewReader(mfr, mfr.Boundary())
 		mf, err := NewFileFromPartReader(mpReader, multipartFormdataType)
 		if err != nil {
@@ -35,48 +43,77 @@ func TestMultiFileReaderToMultiFile(t *testing.T) {
 
 		it := mf.Entries()
 
-		assert.True(t, it.Next())
-		assert.Equal(t, "beep.txt", it.Name())
-		assert.True(t, it.Next())
-		assert.Equal(t, "boop", it.Name())
-		assert.NotNil(t, DirFromEntry(it))
+		require.True(t, it.Next())
+		require.Equal(t, "beep.txt", it.Name())
+		require.True(t, it.Next())
+		require.Equal(t, "boop", it.Name())
+		require.NotNil(t, DirFromEntry(it))
 
 		subIt := DirFromEntry(it).Entries()
-		assert.True(t, subIt.Next())
-		assert.Equal(t, "a.txt", subIt.Name())
-		assert.Nil(t, DirFromEntry(subIt))
-
-		assert.True(t, subIt.Next())
-		assert.Equal(t, "b.txt", subIt.Name())
-		assert.Nil(t, DirFromEntry(subIt))
-
-		assert.False(t, subIt.Next())
-		assert.Nil(t, it.Err())
+		require.True(t, subIt.Next(), subIt.Err())
+		require.Equal(t, "a.txt", subIt.Name())
+		require.Nil(t, DirFromEntry(subIt))
+
+		if expectFailure {
+			require.False(t, subIt.Next())
+			require.Error(t, subIt.Err())
+		} else {
+			require.True(t, subIt.Next(), subIt.Err())
+			require.Equal(t, filename, subIt.Name())
+			require.Nil(t, DirFromEntry(subIt))
+
+			require.False(t, subIt.Next())
+			require.Nil(t, it.Err())
+
+			// try to break internal state
+			require.False(t, subIt.Next())
+			require.Nil(t, it.Err())
+
+			require.True(t, it.Next())
+			require.Equal(t, "file.txt", it.Name())
+			require.Nil(t, DirFromEntry(it))
+			require.Nil(t, it.Err())
+
+			require.False(t, it.Next())
+			require.Nil(t, it.Err())
+		}
+	}
 
-		// try to break internal state
-		assert.False(t, subIt.Next())
-		assert.Nil(t, it.Err())
+	t.Run("Header 'abspath' with UTF-8 filename succeeds", func(t *testing.T) {
+		do(t, false, true, false)
+	})
 
-		assert.True(t, it.Next())
-		assert.Equal(t, "file.txt", it.Name())
-		assert.Nil(t, DirFromEntry(it))
-		assert.Nil(t, it.Err())
+	t.Run("Header 'abspath-encoded' with UTF-8 filename succeeds", func(t *testing.T) {
+		do(t, false, false, false)
+	})
 
-		assert.False(t, it.Next())
-		assert.Nil(t, it.Err())
-	}
+	t.Run("Header 'abspath' with binary filename fails", func(t *testing.T) {
+		// Simulates old client talking to new server. Old client will send the
+		// binary filename in the regular headers and the new server will error.
+		do(t, true, true, true)
+	})
 
-	t.Run("With Header 'abspath'", func(t *testing.T) {
-		do(t, true)
+	t.Run("Header 'abspath-encoded' with binary filename succeeds", func(t *testing.T) {
+		do(t, true, false, false)
 	})
+}
 
-	t.Run("With Header 'abspath-encoded'", func(t *testing.T) {
-		do(t, false)
+func getTestMultiFileReader(t *testing.T) *MultiFileReader {
+	sf := NewMapDirectory(map[string]Node{
+		"file.txt": NewBytesFile([]byte(text)),
+		"boop": NewMapDirectory(map[string]Node{
+			"a.txt": NewBytesFile([]byte("bleep")),
+			"b.txt": NewBytesFile([]byte("bloop")),
+		}),
+		"beep.txt": NewBytesFile([]byte("beep")),
 	})
+
+	// testing output by reading it with the go stdlib "mime/multipart" Reader
+	return NewMultiFileReader(sf, true, false)
 }
 
 func TestMultiFileReaderToMultiFileSkip(t *testing.T) {
-	mfr := getTestMultiFileReader(t, false)
+	mfr := getTestMultiFileReader(t)
 	mpReader := multipart.NewReader(mfr, mfr.Boundary())
 	mf, err := NewFileFromPartReader(mpReader, multipartFormdataType)
 	if err != nil {
@@ -103,7 +140,7 @@ func TestMultiFileReaderToMultiFileSkip(t *testing.T) {
 }
 
 func TestOutput(t *testing.T) {
-	mfr := getTestMultiFileReader(t, false)
+	mfr := getTestMultiFileReader(t)
 	walker := &multipartWalker{reader: multipart.NewReader(mfr, mfr.Boundary())}
 	buf := make([]byte, 20)
 

files/readerfile.go

@@ -21,6 +21,10 @@ func NewBytesFile(b []byte) File {
 	return &ReaderFile{"", bytesReaderCloser{bytes.NewReader(b)}, nil, int64(len(b))}
 }
 
+func NewBytesFileWithPath(abspath string, b []byte) File {
+	return &ReaderFile{abspath, bytesReaderCloser{bytes.NewReader(b)}, nil, int64(len(b))}
+}
+
 // TODO: Is this the best way to fix this bug?
 // The bug is we want to be an io.ReadSeekCloser, but bytes.NewReader only gives a io.ReadSeeker and io.NopCloser
 // effectively removes the io.Seeker ability from the passed in io.Reader