Merge pull request #1641 from rht/namedpipe

Serialfile: Explicit err on unrecognized file type

commands/files/serialfile.go

 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).
@@ -22,50 +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) {
-	if stat.Mode()&os.ModeSymlink != 0 {
+	switch mode := stat.Mode(); {
+	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
-	}
-
-	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
+		return NewLinkFile(name, path, target, stat), nil
+	default:
+		return nil, fmt.Errorf("Unrecognized file type for %s: %s", name, mode.String())
 	}
-
-	// 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 {
@@ -92,31 +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
-	}
-
-	if st.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
-	}
 
-	file, err := os.Open(filePath)
+	// 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)
 	if err != nil {
 		return nil, err
 	}
 
-	f.current = file
+	f.current = &sf
 
-	// 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)
+	return sf, nil
 }
 
 func (f *serialFile) FileName() string {
@@ -134,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

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"
+}

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

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