From ddc8dff7368b4e5ce31596513666ebc6c1373381 Mon Sep 17 00:00:00 2001 From: rht Date: Thu, 3 Sep 2015 17:18:28 +0700 Subject: [PATCH 1/3] serialfile: Explicit err on unrecognized file type So that ipfs add on daemon no longer blocks License: MIT Signed-off-by: rht --- commands/files/serialfile.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/commands/files/serialfile.go b/commands/files/serialfile.go index a62115ef4..822343e58 100644 --- a/commands/files/serialfile.go +++ b/commands/files/serialfile.go @@ -1,6 +1,7 @@ package files import ( + "fmt" "io" "os" fp "path/filepath" @@ -26,13 +27,18 @@ type serialFile struct { } func NewSerialFile(name, path string, stat os.FileInfo) (File, error) { - if stat.Mode()&os.ModeSymlink != 0 { + switch mode := stat.Mode(); { + case mode.IsDir() || mode.IsRegular(): + break + case mode&os.ModeSymlink != 0: target, err := os.Readlink(path) if err != nil { return nil, err } return NewLinkFile("", path, target, stat), nil + default: + return nil, fmt.Errorf("Unrecognized file type for %s: %s", stat.Name(), mode.String()) } file, err := os.Open(path) @@ -97,13 +103,18 @@ func (f *serialFile) NextFile() (File, error) { return nil, err } - if st.Mode()&os.ModeSymlink != 0 { + switch mode := st.Mode(); { + case mode.IsDir() || mode.IsRegular(): + break + case mode&os.ModeSymlink != 0: f.current = nil target, err := os.Readlink(filePath) if err != nil { return nil, err } return NewLinkFile(fileName, filePath, target, st), nil + default: + return nil, fmt.Errorf("Unrecognized file type for %s: %s", st.Name(), mode.String()) } file, err := os.Open(filePath) From ccc9cd6a6cc0747f10689e22bbd547cfb8ab6600 Mon Sep 17 00:00:00 2001 From: rht Date: Fri, 4 Sep 2015 14:03:34 +0700 Subject: [PATCH 2/3] Refactor serialfile License: MIT Signed-off-by: rht --- commands/files/serialfile.go | 99 ++++++++++-------------------------- 1 file changed, 27 insertions(+), 72 deletions(-) diff --git a/commands/files/serialfile.go b/commands/files/serialfile.go index 822343e58..b43869a4d 100644 --- a/commands/files/serialfile.go +++ b/commands/files/serialfile.go @@ -3,18 +3,12 @@ package files import ( "fmt" "io" + "io/ioutil" "os" fp "path/filepath" - "sort" "syscall" ) -type sortFIByName []os.FileInfo - -func (es sortFIByName) Len() int { return len(es) } -func (es sortFIByName) Swap(i, j int) { es[i], es[j] = es[j], es[i] } -func (es sortFIByName) Less(i, j int) bool { return es[i].Name() < es[j].Name() } - // serialFile implements File, and reads from a path on the OS filesystem. // No more than one file will be opened at a time (directories will advance // to the next file when NextFile() is called). @@ -23,55 +17,34 @@ type serialFile struct { path string files []os.FileInfo stat os.FileInfo - current *os.File + current *File } func NewSerialFile(name, path string, stat os.FileInfo) (File, error) { switch mode := stat.Mode(); { - case mode.IsDir() || mode.IsRegular(): - break + case mode.IsRegular(): + file, err := os.Open(path) + if err != nil { + return nil, err + } + return NewReaderFile(name, path, file, stat), nil + case mode.IsDir(): + // for directories, stat all of the contents first, so we know what files to + // open when NextFile() is called + contents, err := ioutil.ReadDir(path) + if err != nil { + return nil, err + } + return &serialFile{name, path, contents, stat, nil}, nil case mode&os.ModeSymlink != 0: target, err := os.Readlink(path) if err != nil { return nil, err } - - return NewLinkFile("", path, target, stat), nil + return NewLinkFile(name, path, target, stat), nil default: - return nil, fmt.Errorf("Unrecognized file type for %s: %s", stat.Name(), mode.String()) + return nil, fmt.Errorf("Unrecognized file type for %s: %s", name, mode.String()) } - - file, err := os.Open(path) - if err != nil { - return nil, err - } - - return newSerialFile(name, path, file, stat) -} - -func newSerialFile(name, path string, file *os.File, stat os.FileInfo) (File, error) { - // for non-directories, return a ReaderFile - if !stat.IsDir() { - return &ReaderFile{name, path, file, stat}, nil - } - - // for directories, stat all of the contents first, so we know what files to - // open when NextFile() is called - contents, err := file.Readdir(0) - if err != nil { - return nil, err - } - - // we no longer need our root directory file (we already statted the contents), - // so close it - if err := file.Close(); err != nil { - return nil, err - } - - // make sure contents are sorted so -- repeatably -- we get the same inputs. - sort.Sort(sortFIByName(contents)) - - return &serialFile{name, path, contents, stat, nil}, nil } func (f *serialFile) IsDirectory() bool { @@ -98,36 +71,18 @@ func (f *serialFile) NextFile() (File, error) { // open the next file fileName := fp.Join(f.name, stat.Name()) filePath := fp.Join(f.path, stat.Name()) - st, err := os.Lstat(filePath) - if err != nil { - return nil, err - } - - switch mode := st.Mode(); { - case mode.IsDir() || mode.IsRegular(): - break - case mode&os.ModeSymlink != 0: - f.current = nil - target, err := os.Readlink(filePath) - if err != nil { - return nil, err - } - return NewLinkFile(fileName, filePath, target, st), nil - default: - return nil, fmt.Errorf("Unrecognized file type for %s: %s", st.Name(), mode.String()) - } - - file, err := os.Open(filePath) - if err != nil { - return nil, err - } - - f.current = file // 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 - return newSerialFile(fileName, filePath, file, stat) + sf, err := NewSerialFile(fileName, filePath, stat) + if err != nil { + return nil, err + } + + f.current = &sf + + return sf, nil } func (f *serialFile) FileName() string { @@ -145,7 +100,7 @@ func (f *serialFile) Read(p []byte) (int, error) { func (f *serialFile) Close() error { // close the current file if there is one if f.current != nil { - err := f.current.Close() + err := (*f.current).Close() // ignore EINVAL error, the file might have already been closed if err != nil && err != syscall.EINVAL { return err From 1b26ca724f9dda116ab959db14e5bafc64bbf51b Mon Sep 17 00:00:00 2001 From: rht Date: Fri, 4 Sep 2015 16:55:34 +0700 Subject: [PATCH 3/3] Add test for ipfs add err for unsupported file type License: MIT Signed-off-by: rht --- test/sharness/lib/test-lib.sh | 14 ++++++++++++++ test/sharness/t0040-add-and-cat.sh | 19 +++++++++++++++++++ test/sharness/t0041-add-cat-offline.sh | 19 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/test/sharness/lib/test-lib.sh b/test/sharness/lib/test-lib.sh index 78c358b14..1d6866c48 100644 --- a/test/sharness/lib/test-lib.sh +++ b/test/sharness/lib/test-lib.sh @@ -330,3 +330,17 @@ disk_usage() { esac $DU "$1" | awk "{print \$1}" } + +# output a file's permission in human readable format +generic_stat() { + # normalize stat across systems + case $(uname -s) in + Linux) + _STAT="stat -c %A" + ;; + FreeBSD | Darwin | DragonFly) + _STAT="stat -f %Sp" + ;; + esac + $_STAT "$1" +} diff --git a/test/sharness/t0040-add-and-cat.sh b/test/sharness/t0040-add-and-cat.sh index 28da0225c..85739a7b5 100755 --- a/test/sharness/t0040-add-and-cat.sh +++ b/test/sharness/t0040-add-and-cat.sh @@ -8,6 +8,10 @@ test_description="Test add and cat commands" . lib/test-lib.sh +client_err() { + printf "$@\n\nUse 'ipfs add --help' for information about this command\n" +} + test_launch_ipfs_daemon_and_mount test_expect_success "'ipfs add --help' succeeds" ' @@ -262,6 +266,21 @@ test_expect_success FUSE,EXPENSIVE "cat ipfs/bigfile looks good" ' test_cmp sha1_expected sha1_actual ' +test_expect_success "useful error message when adding a named pipe" ' + mkfifo named-pipe && + test_expect_code 1 ipfs add named-pipe 2>actual && + client_err "Error: Unrecognized file type for named-pipe: $(generic_stat named-pipe)" >expected && + test_cmp expected actual +' + +test_expect_success "useful error message when recursively adding a named pipe" ' + mkdir named-pipe-dir && + mkfifo named-pipe-dir/named-pipe && + test_expect_code 1 ipfs add -r named-pipe-dir 2>actual && + printf "Error: Post http://127.0.0.1:$PORT_API/api/v0/add?encoding=json&progress=true&r=true&stream-channels=true: Unrecognized file type for named-pipe-dir/named-pipe: $(generic_stat named-pipe-dir/named-pipe)\n" >expected && + test_cmp expected actual +' + test_kill_ipfs_daemon test_done diff --git a/test/sharness/t0041-add-cat-offline.sh b/test/sharness/t0041-add-cat-offline.sh index 68f632267..96c61afe9 100755 --- a/test/sharness/t0041-add-cat-offline.sh +++ b/test/sharness/t0041-add-cat-offline.sh @@ -8,6 +8,10 @@ test_description="Test add and cat commands" . lib/test-lib.sh +client_err() { + printf "$@\n\nUse 'ipfs add --help' for information about this command\n" +} + test_init_ipfs test_expect_success "ipfs add file succeeds" ' @@ -53,4 +57,19 @@ test_expect_success "ipfs cat file fails" ' test_must_fail ipfs cat $(cat oh_hash) ' +test_expect_success "useful error message when adding a named pipe" ' + mkfifo named-pipe && + test_expect_code 1 ipfs add named-pipe 2>actual && + client_err "Error: Unrecognized file type for named-pipe: $(generic_stat named-pipe)" >expected && + test_cmp expected actual +' + +test_expect_success "useful error message when recursively adding a named pipe" ' + mkdir named-pipe-dir && + mkfifo named-pipe-dir/named-pipe && + test_expect_code 1 ipfs add -r named-pipe-dir 2>actual && + printf "Error: Unrecognized file type for named-pipe-dir/named-pipe: $(generic_stat named-pipe-dir/named-pipe)\n" >expected && + test_cmp expected actual +' + test_done