Skip to content

Commit

Permalink
fix!: add escaped abspath header
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias committed Aug 16, 2023
1 parent b96767c commit 8d86569
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 45 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ The following emojis are used to highlight certain changes:

### Fixed

* 🛠 `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

## [v0.11.0]
Expand Down
24 changes: 17 additions & 7 deletions files/multifilereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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", rf.AbsPath())
if mfr.rawAbspath {
header.Set("abspath", rf.AbsPath())
} else {
header.Set("abspath-encoded", url.QueryEscape(rf.AbsPath()))
}
}

_, err := mfr.mpWriter.CreatePart(header)
Expand Down
81 changes: 44 additions & 37 deletions files/multifilereader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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 {
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 11 additions & 1 deletion files/multipartfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,19 @@ func (w *multipartWalker) nextFile() (Node, error) {

return NewLinkFile(string(out), nil), nil
default:
var absPath string
if absPathEncoded := part.Header.Get("abspath-encoded"); absPathEncoded != "" {
absPath, err = url.QueryUnescape(absPathEncoded)
if err != nil {
return nil, err
}

Check warning on line 108 in files/multipartfile.go

View check run for this annotation

Codecov / codecov/patch

files/multipartfile.go#L105-L108

Added lines #L105 - L108 were not covered by tests
} else {
absPath = part.Header.Get("abspath")
}

return &ReaderFile{
reader: part,
abspath: part.Header.Get("abspath"),
abspath: absPath,
}, nil
}
}
Expand Down

0 comments on commit 8d86569

Please sign in to comment.