fix!: only send one header

CHANGELOG.md

@@ -22,10 +22,15 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
-* 🛠 `MultiFileReader` has been updated to escape the absolute paths due to the regression
-  found in [`net/textproto`](https://github.com/golang/go/issues/60674). This is no longer
-  compatible with previous versions when using files that have binary characters in their
-  name.
+* 🛠 `MultiFileReader` has been updated with a new header with the encoded file
+  name instead of the plain filename, due to a regression found in 
+  [`net/textproto`](https://github.com/golang/go/issues/60674). This only affects
+  files with binary characters in their name. By keeping the old header, we maximize
+  backwards compatibility.
+  |            | New Client | Old Client  |
+  |------------|------------|-------------|
+  | New Server | ✅         | ❌          |
+  | Old Server | ✅         | ✅          |
 
 ### Security
 

files/multifilereader.go

@@ -29,19 +29,25 @@ type MultiFileReader struct {
 	// if true, the content disposition will be "form-data"
 	// if false, the content disposition will be "attachment"
 	form bool
+
+	// if true, 'abspath' header will be sent with raw file name
+	// if false, 'abspath-encoded' header will be sent with %-encoded filename
+	rawAbspath bool
 }
 
 // NewMultiFileReader constructs a MultiFileReader. `file` can be any `commands.Directory`.
 // If `form` is set to true, the Content-Disposition will be "form-data".
-// Otherwise, it will be "attachment".
-func NewMultiFileReader(file Directory, form bool) *MultiFileReader {
+// Otherwise, it will be "attachment". If `rawAbspath` is set to true, the
+// "abspath" header will be sent. Otherwise, the "abspath-encoded" header will be sent.
+func NewMultiFileReader(file Directory, form, rawAbspath bool) *MultiFileReader {
 	it := file.Entries()
 
 	mfr := &MultiFileReader{
-		files: []DirIterator{it},
-		path:  []string{""},
-		form:  form,
-		mutex: &sync.Mutex{},
+		files:      []DirIterator{it},
+		path:       []string{""},
+		form:       form,
+		rawAbspath: rawAbspath,
+		mutex:      &sync.Mutex{},
 	}
 	mfr.mpWriter = multipart.NewWriter(&mfr.buf)
 
@@ -114,7 +120,11 @@ func (mfr *MultiFileReader) Read(buf []byte) (written int, err error) {
 
 			header.Set("Content-Type", contentType)
 			if rf, ok := entry.Node().(FileInfo); ok {
-				header.Set("abspath", url.QueryEscape(rf.AbsPath()))
+				if mfr.rawAbspath {
+					header.Set("abspath", rf.AbsPath())
+				} else {
+					header.Set("abspath-encoded", url.QueryEscape(rf.AbsPath()))
+				}
 			}
 
 			_, err := mfr.mpWriter.CreatePart(header)

files/multifilereader_test.go

@@ -4,11 +4,13 @@ import (
 	"io"
 	"mime/multipart"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var text = "Some text! :)"
 
-func getTestMultiFileReader(t *testing.T) *MultiFileReader {
+func getTestMultiFileReader(t *testing.T, rawAbsPath bool) *MultiFileReader {
 	sf := NewMapDirectory(map[string]Node{
 		"file.txt": NewBytesFile([]byte(text)),
 		"boop": NewMapDirectory(map[string]Node{
@@ -19,57 +21,62 @@ func getTestMultiFileReader(t *testing.T) *MultiFileReader {
 	})
 
 	// testing output by reading it with the go stdlib "mime/multipart" Reader
-	return NewMultiFileReader(sf, true)
+	return NewMultiFileReader(sf, true, rawAbsPath)
 }
 
 func TestMultiFileReaderToMultiFile(t *testing.T) {
-	mfr := getTestMultiFileReader(t)
-	mpReader := multipart.NewReader(mfr, mfr.Boundary())
-	mf, err := NewFileFromPartReader(mpReader, multipartFormdataType)
-	if err != nil {
-		t.Fatal(err)
-	}
+	do := func(t *testing.T, rawAbsPath bool) {
+		mfr := getTestMultiFileReader(t, rawAbsPath)
+		mpReader := multipart.NewReader(mfr, mfr.Boundary())
+		mf, err := NewFileFromPartReader(mpReader, multipartFormdataType)
+		if err != nil {
+			t.Fatal(err)
+		}
 
-	it := mf.Entries()
+		it := mf.Entries()
 
-	if !it.Next() || it.Name() != "beep.txt" {
-		t.Fatal("iterator didn't work as expected")
-	}
+		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))
 
-	if !it.Next() || it.Name() != "boop" || DirFromEntry(it) == nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+		subIt := DirFromEntry(it).Entries()
+		assert.True(t, subIt.Next())
+		assert.Equal(t, "a.txt", subIt.Name())
+		assert.Nil(t, DirFromEntry(subIt))
 
-	subIt := DirFromEntry(it).Entries()
+		assert.True(t, subIt.Next())
+		assert.Equal(t, "b.txt", subIt.Name())
+		assert.Nil(t, DirFromEntry(subIt))
 
-	if !subIt.Next() || subIt.Name() != "a.txt" || DirFromEntry(subIt) != nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+		assert.False(t, subIt.Next())
+		assert.Nil(t, it.Err())
 
-	if !subIt.Next() || subIt.Name() != "b.txt" || DirFromEntry(subIt) != nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+		// try to break internal state
+		assert.False(t, subIt.Next())
+		assert.Nil(t, it.Err())
 
-	if subIt.Next() || it.Err() != nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+		assert.True(t, it.Next())
+		assert.Equal(t, "file.txt", it.Name())
+		assert.Nil(t, DirFromEntry(it))
+		assert.Nil(t, it.Err())
 
-	// try to break internal state
-	if subIt.Next() || it.Err() != nil {
-		t.Fatal("iterator didn't work as expected")
+		assert.False(t, it.Next())
+		assert.Nil(t, it.Err())
 	}
 
-	if !it.Next() || it.Name() != "file.txt" || DirFromEntry(it) != nil || it.Err() != nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+	t.Run("With Header 'abspath'", func(t *testing.T) {
+		do(t, true)
+	})
 
-	if it.Next() || it.Err() != nil {
-		t.Fatal("iterator didn't work as expected")
-	}
+	t.Run("With Header 'abspath-encoded'", func(t *testing.T) {
+		do(t, false)
+	})
 }
 
 func TestMultiFileReaderToMultiFileSkip(t *testing.T) {
-	mfr := getTestMultiFileReader(t)
+	mfr := getTestMultiFileReader(t, false)
 	mpReader := multipart.NewReader(mfr, mfr.Boundary())
 	mf, err := NewFileFromPartReader(mpReader, multipartFormdataType)
 	if err != nil {
@@ -96,7 +103,7 @@ func TestMultiFileReaderToMultiFileSkip(t *testing.T) {
 }
 
 func TestOutput(t *testing.T) {
-	mfr := getTestMultiFileReader(t)
+	mfr := getTestMultiFileReader(t, false)
 	walker := &multipartWalker{reader: multipart.NewReader(mfr, mfr.Boundary())}
 	buf := make([]byte, 20)
 
@@ -164,7 +171,7 @@ func TestCommonPrefix(t *testing.T) {
 			"aaa": NewBytesFile([]byte("bleep")),
 		}),
 	})
-	mfr := NewMultiFileReader(sf, true)
+	mfr := NewMultiFileReader(sf, true, false)
 	reader, err := NewFileFromPartReader(multipart.NewReader(mfr, mfr.Boundary()), multipartFormdataType)
 	if err != nil {
 		t.Fatal(err)

files/multipartfile.go

@@ -100,14 +100,19 @@ func (w *multipartWalker) nextFile() (Node, error) {
 
 		return NewLinkFile(string(out), nil), nil
 	default:
-		abspath, err := url.QueryUnescape(part.Header.Get("abspath"))
-		if err != nil {
-			return nil, err
+		var absPath string
+		if absPathEncoded := part.Header.Get("abspath-encoded"); absPathEncoded != "" {
+			absPath, err = url.QueryUnescape(absPathEncoded)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			absPath = part.Header.Get("abspath")
 		}
 
 		return &ReaderFile{
 			reader:  part,
-			abspath: abspath,
+			abspath: absPath,
 		}, nil
 	}
 }