Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flatten multipart transfers #2046

Merged
merged 6 commits into from
Dec 28, 2015
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions commands/cli/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,17 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c
}
}

stringArgs, fileArgs, err := parseArgs(stringVals, stdin, cmd.Arguments, recursive, root)
// if '--hidden' is provided, enumerate hidden paths
hiddenOpt := req.Option("hidden")
hidden := false
if hiddenOpt != nil {
hidden, _, err = hiddenOpt.Bool()
if err != nil {
return req, nil, nil, u.ErrCast()
}
}

stringArgs, fileArgs, err := parseArgs(stringVals, stdin, cmd.Arguments, recursive, hidden, root)
if err != nil {
return req, cmd, path, err
}
Expand Down Expand Up @@ -221,7 +231,7 @@ func parseOpts(args []string, root *cmds.Command) (
return
}

func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursive bool, root *cmds.Command) ([]string, []files.File, error) {
func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursive, hidden bool, root *cmds.Command) ([]string, []files.File, error) {
// ignore stdin on Windows
if runtime.GOOS == "windows" {
stdin = nil
Expand Down Expand Up @@ -306,7 +316,7 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
// treat stringArg values as file paths
fpath := inputs[0]
inputs = inputs[1:]
file, err := appendFile(fpath, argDef, recursive)
file, err := appendFile(fpath, argDef, recursive, hidden)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -387,7 +397,7 @@ func appendStdinAsString(args []string, stdin *os.File) ([]string, *os.File, err
const notRecursiveFmtStr = "'%s' is a directory, use the '-%s' flag to specify directories"
const dirNotSupportedFmtStr = "Invalid path '%s', argument '%s' does not support directories"

func appendFile(fpath string, argDef *cmds.Argument, recursive bool) (files.File, error) {
func appendFile(fpath string, argDef *cmds.Argument, recursive, hidden bool) (files.File, error) {
fpath = filepath.ToSlash(filepath.Clean(fpath))

if fpath == "." {
Expand All @@ -412,7 +422,7 @@ func appendFile(fpath string, argDef *cmds.Argument, recursive bool) (files.File
}
}

return files.NewSerialFile(path.Base(fpath), fpath, stat)
return files.NewSerialFile(path.Base(fpath), fpath, hidden, stat)
}

// isTerminal returns true if stdin is a Stdin pipe (e.g. `cat file | ipfs`),
Expand Down
105 changes: 30 additions & 75 deletions commands/files/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,38 @@ func TestSliceFiles(t *testing.T) {
sf := NewSliceFile(name, name, files)

if !sf.IsDirectory() {
t.Error("SliceFile should always be a directory")
t.Fatal("SliceFile should always be a directory")
}
if n, err := sf.Read(buf); n > 0 || err != ErrNotReader {
t.Error("Shouldn't be able to call `Read` on a SliceFile")

if n, err := sf.Read(buf); n > 0 || err != io.EOF {
t.Fatal("Shouldn't be able to read data from a SliceFile")
}

if err := sf.Close(); err != ErrNotReader {
t.Error("Shouldn't be able to call `Close` on a SliceFile")
t.Fatal("Shouldn't be able to call `Close` on a SliceFile")
}

file, err := sf.NextFile()
if file == nil || err != nil {
t.Error("Expected a file and nil error")
t.Fatal("Expected a file and nil error")
}
read, err := file.Read(buf)
if read != 11 || err != nil {
t.Error("NextFile got a file in the wrong order")
t.Fatal("NextFile got a file in the wrong order")
}

file, err = sf.NextFile()
if file == nil || err != nil {
t.Error("Expected a file and nil error")
t.Fatal("Expected a file and nil error")
}
file, err = sf.NextFile()
if file == nil || err != nil {
t.Error("Expected a file and nil error")
t.Fatal("Expected a file and nil error")
}

file, err = sf.NextFile()
if file != nil || err != io.EOF {
t.Error("Expected a nil file and io.EOF")
t.Fatal("Expected a nil file and io.EOF")
}
}

Expand All @@ -59,21 +61,21 @@ func TestReaderFiles(t *testing.T) {
buf := make([]byte, len(message))

if rf.IsDirectory() {
t.Error("ReaderFile should never be a directory")
t.Fatal("ReaderFile should never be a directory")
}
file, err := rf.NextFile()
if file != nil || err != ErrNotDirectory {
t.Error("Expected a nil file and ErrNotDirectory")
t.Fatal("Expected a nil file and ErrNotDirectory")
}

if n, err := rf.Read(buf); n == 0 || err != nil {
t.Error("Expected to be able to read")
t.Fatal("Expected to be able to read")
}
if err := rf.Close(); err != nil {
t.Error("Should be able to close")
t.Fatal("Should be able to close")
}
if n, err := rf.Read(buf); n != 0 || err != io.EOF {
t.Error("Expected EOF when reading after close")
t.Fatal("Expected EOF when reading after close")
}
}

Expand All @@ -86,23 +88,9 @@ Some-Header: beep

beep
--Boundary!
Content-Type: multipart/mixed; boundary=OtherBoundary
Content-Type: application/x-directory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should test file inside the directory as well

Content-Disposition: file; filename="dir"

--OtherBoundary
Content-Type: text/plain
Content-Disposition: file; filename="some/file/path"

test
--OtherBoundary
Content-Type: text/plain

boop
--OtherBoundary
Content-Type: text/plain

bloop
--OtherBoundary--
--Boundary!--

`
Expand All @@ -114,81 +102,48 @@ bloop
// test properties of a file created from the first part
part, err := mpReader.NextPart()
if part == nil || err != nil {
t.Error("Expected non-nil part, nil error")
t.Fatal("Expected non-nil part, nil error")
}
mpf, err := NewFileFromPart(part)
if mpf == nil || err != nil {
t.Error("Expected non-nil MultipartFile, nil error")
t.Fatal("Expected non-nil MultipartFile, nil error")
}
if mpf.IsDirectory() {
t.Error("Expected file to not be a directory")
t.Fatal("Expected file to not be a directory")
}
if mpf.FileName() != "name" {
t.Error("Expected filename to be \"name\"")
t.Fatal("Expected filename to be \"name\"")
}
if file, err := mpf.NextFile(); file != nil || err != ErrNotDirectory {
t.Error("Expected a nil file and ErrNotDirectory")
t.Fatal("Expected a nil file and ErrNotDirectory")
}
if n, err := mpf.Read(buf); n != 4 || err != nil {
t.Error("Expected to be able to read 4 bytes")
t.Fatal("Expected to be able to read 4 bytes")
}
if err := mpf.Close(); err != nil {
t.Error("Expected to be able to close file")
t.Fatal("Expected to be able to close file")
}

// test properties of file created from second part (directory)
part, err = mpReader.NextPart()
if part == nil || err != nil {
t.Error("Expected non-nil part, nil error")
t.Fatal("Expected non-nil part, nil error")
}
mpf, err = NewFileFromPart(part)
if mpf == nil || err != nil {
t.Error("Expected non-nil MultipartFile, nil error")
t.Fatal("Expected non-nil MultipartFile, nil error")
}
if !mpf.IsDirectory() {
t.Error("Expected file to be a directory")
t.Fatal("Expected file to be a directory")
}
if mpf.FileName() != "dir" {
t.Error("Expected filename to be \"dir\"")
t.Fatal("Expected filename to be \"dir\"")
}
if n, err := mpf.Read(buf); n > 0 || err != ErrNotReader {
t.Error("Shouldn't be able to call `Read` on a directory")
t.Fatal("Shouldn't be able to call `Read` on a directory")
}
if err := mpf.Close(); err != ErrNotReader {
t.Error("Shouldn't be able to call `Close` on a directory")
}

// test properties of first child file
child, err := mpf.NextFile()
if child == nil || err != nil {
t.Error("Expected to be able to read a child file")
}
if child.IsDirectory() {
t.Error("Expected file to not be a directory")
}
if child.FileName() != "some/file/path" {
t.Error("Expected filename to be \"some/file/path\"")
t.Fatal("Shouldn't be able to call `Close` on a directory")
}

// test processing files out of order
child, err = mpf.NextFile()
if child == nil || err != nil {
t.Error("Expected to be able to read a child file")
}
child2, err := mpf.NextFile()
if child == nil || err != nil {
t.Error("Expected to be able to read a child file")
}
if n, err := child2.Read(buf); n != 5 || err != nil {
t.Error("Expected to be able to read")
}
if n, err := child.Read(buf); n != 0 || err == nil {
t.Error("Expected to not be able to read after advancing NextFile() past this file")
}

// make sure the end is handled properly
child, err = mpf.NextFile()
if child != nil || err == nil {
t.Error("Expected NextFile to return (nil, EOF)")
}
}
30 changes: 12 additions & 18 deletions commands/files/multipartfile.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
package files

import (
"io"
"io/ioutil"
"mime"
"mime/multipart"
"net/http"
"net/url"
)

const (
multipartFormdataType = "multipart/form-data"
multipartMixedType = "multipart/mixed"

applicationSymlink = "application/symlink"
applicationDirectory = "application/x-directory"
applicationSymlink = "application/symlink"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this tested and documented anywhere (symlink)?


contentTypeHeader = "Content-Type"
)
Expand Down Expand Up @@ -45,40 +46,33 @@ func NewFileFromPart(part *multipart.Part) (File, error) {
}, nil
}

var params map[string]string
var err error
f.Mediatype, params, err = mime.ParseMediaType(contentType)
f.Mediatype, _, err = mime.ParseMediaType(contentType)
if err != nil {
return nil, err
}

if f.IsDirectory() {
boundary, found := params["boundary"]
if !found {
return nil, http.ErrMissingBoundary
}

f.Reader = multipart.NewReader(part, boundary)
}

return f, nil
}

func (f *MultipartFile) IsDirectory() bool {
return f.Mediatype == multipartFormdataType || f.Mediatype == multipartMixedType
return f.Mediatype == multipartFormdataType || f.Mediatype == applicationDirectory
}

func (f *MultipartFile) NextFile() (File, error) {
if !f.IsDirectory() {
return nil, ErrNotDirectory
}
if f.Reader != nil {
part, err := f.Reader.NextPart()
if err != nil {
return nil, err
}

part, err := f.Reader.NextPart()
if err != nil {
return nil, err
return NewFileFromPart(part)
}

return NewFileFromPart(part)
return nil, io.EOF
}

func (f *MultipartFile) FileName() string {
Expand Down
19 changes: 15 additions & 4 deletions commands/files/serialfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"
"syscall"
)

Expand All @@ -18,9 +19,10 @@ type serialFile struct {
files []os.FileInfo
stat os.FileInfo
current *File
hidden bool
}

func NewSerialFile(name, path string, stat os.FileInfo) (File, error) {
func NewSerialFile(name, path string, hidden bool, stat os.FileInfo) (File, error) {
switch mode := stat.Mode(); {
case mode.IsRegular():
file, err := os.Open(path)
Expand All @@ -35,7 +37,7 @@ func NewSerialFile(name, path string, stat os.FileInfo) (File, error) {
if err != nil {
return nil, err
}
return &serialFile{name, path, contents, stat, nil}, nil
return &serialFile{name, path, contents, stat, nil, hidden}, nil
case mode&os.ModeSymlink != 0:
target, err := os.Readlink(path)
if err != nil {
Expand Down Expand Up @@ -68,14 +70,23 @@ func (f *serialFile) NextFile() (File, error) {
stat := f.files[0]
f.files = f.files[1:]

for !f.hidden && strings.HasPrefix(stat.Name(), ".") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to rename .hidden to something like .addHidden or .ignoreHidden

if len(f.files) == 0 {
return nil, io.EOF
}

stat = f.files[0]
f.files = f.files[1:]
}

// open the next file
fileName := filepath.ToSlash(filepath.Join(f.name, stat.Name()))
filePath := filepath.ToSlash(filepath.Join(f.path, stat.Name()))

// recursively call the constructor on the next file
// if it's a regular file, we will open it as a ReaderFile
// if it's a directory, files in it will be opened serially
sf, err := NewSerialFile(fileName, filePath, stat)
sf, err := NewSerialFile(fileName, filePath, f.hidden, stat)
if err != nil {
return nil, err
}
Expand All @@ -94,7 +105,7 @@ func (f *serialFile) FullPath() string {
}

func (f *serialFile) Read(p []byte) (int, error) {
return 0, ErrNotReader
return 0, io.EOF
}

func (f *serialFile) Close() error {
Expand Down
2 changes: 1 addition & 1 deletion commands/files/slicefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (f *SliceFile) FullPath() string {
}

func (f *SliceFile) Read(p []byte) (int, error) {
return 0, ErrNotReader
return 0, io.EOF
}

func (f *SliceFile) Close() error {
Expand Down
Loading