Add support for relative symlinks; better testing for interior links

Author: dsharp-pivotal

memfs/memfs.go

@@ -179,26 +179,17 @@ func (fs *MemFS) ReadDir(path string) ([]os.FileInfo, error) {
 }
 
 func (fs *MemFS) fileInfo(path string) (parent *fileInfo, node *fileInfo, err error) {
-	path = filepath.Clean(path)
-	segments := vfs.SplitPath(path, PathSeparator)
+	return fs.relativeFileInfo(fs.wd, path)
+}
 
-	// Shortcut for working directory and root
-	if len(segments) == 1 {
-		if segments[0] == "" {
-			return nil, fs.root, nil
-		} else if segments[0] == "." {
-			return fs.wd.parent, fs.wd, nil
-		}
-	}
+func (fs *MemFS) relativeFileInfo(wd *fileInfo, path string) (parent *fileInfo, node *fileInfo, err error) {
+	parent, segments := fs.dirSegments(wd, path)
 
-	// Determine root to traverse
-	parent = fs.root
-	if segments[0] == "." {
-		parent = fs.wd
+	// Shortcut for working directory and root
+	if len(segments) == 0 {
+		return parent.parent, parent, nil
 	}
-	segments = segments[1:]
 
-	// Further directories
 	for _, seg := range segments[:len(segments)-1] {
 
 		if parent.childs == nil {
@@ -211,10 +202,16 @@ func (fs *MemFS) fileInfo(path string) (parent *fileInfo, node *fileInfo, err er
 		if entry.dir {
 			parent = entry
 		} else if entry.mode&os.ModeSymlink != 0 {
-			_, parent, err = fs.fileInfo(string(*entry.buf))
+			// Look up interior symlink
+			_, parent, err = fs.relativeFileInfo(parent, string(*entry.buf))
 			if err != nil {
 				return nil, nil, err
 			}
+			// Symlink was not to a directory
+			if parent == nil {
+				return nil, nil, vfs.ErrNotDirectory
+			}
+
 		} else {
 			return nil, nil, os.ErrNotExist
 		}
@@ -223,6 +220,9 @@ func (fs *MemFS) fileInfo(path string) (parent *fileInfo, node *fileInfo, err er
 	lastSeg := segments[len(segments)-1]
 	if parent.childs != nil {
 		if node, ok := parent.childs[lastSeg]; ok {
+			if node.mode&os.ModeSymlink != 0 {
+				return fs.relativeFileInfo(parent, string(*node.buf))
+			}
 			return parent, node, nil
 		}
 	} else {
@@ -232,6 +232,19 @@ func (fs *MemFS) fileInfo(path string) (parent *fileInfo, node *fileInfo, err er
 	return parent, nil, nil
 }
 
+func (fs *MemFS) dirSegments(wd *fileInfo, path string) (parent *fileInfo, segments []string) {
+	path = filepath.Clean(path)
+	segments = vfs.SplitPath(path, PathSeparator)
+
+	// Determine root to traverse
+	parent = fs.root
+	if segments[0] == "." {
+		parent = wd
+	}
+	segments = segments[1:]
+	return parent, segments
+}
+
 func hasFlag(flag int, flags int) bool {
 	return flags&flag == flag
 }
@@ -243,11 +256,6 @@ func (fs *MemFS) OpenFile(name string, flag int, perm os.FileMode) (vfs.File, er
 	fs.lock.Lock()
 	defer fs.lock.Unlock()
 
-	return fs.openFile(name, flag, perm)
-}
-
-func (fs *MemFS) openFile(name string, flag int, perm os.FileMode) (vfs.File, error) {
-
 	name = filepath.Clean(name)
 	base := filepath.Base(name)
 	fiParent, fiNode, err := fs.fileInfo(name)
@@ -275,9 +283,6 @@ func (fs *MemFS) openFile(name string, flag int, perm os.FileMode) (vfs.File, er
 		if fiNode.dir {
 			return nil, &os.PathError{"open", name, ErrIsDirectory}
 		}
-		if fiNode.mode&os.ModeSymlink != 0 {
-			return fs.openFile(string(*fiNode.buf), flag, perm)
-		}
 	}
 
 	if !hasFlag(os.O_RDONLY, flag) {

memfs/memfs_test.go

@@ -3,6 +3,7 @@ package memfs
 import (
 	"io/ioutil"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -139,10 +140,6 @@ func TestSymlink(t *testing.T) {
 	if err != nil {
 		t.Fatal("Symlink failed:", err)
 	}
-	_, node, err := fs.fileInfo("/tmp/cup")
-	if string(*node.buf) != "/tmp/teacup" {
-		t.Fatal("Wrong symlink contents in buf:", string(*node.buf))
-	}
 	fluid, err := vfs.ReadFile(fs, "/tmp/cup")
 	if err != nil {
 		t.Fatal("Failed to read from /tmp/cup:", err)
@@ -158,9 +155,11 @@ func TestDirectorySymlink(t *testing.T) {
 	if err := vfs.MkdirAll(fs, "/foo/a/b", 0755); err != nil {
 		t.Fatal("Unable mkdir /foo/a/b:", err)
 	}
+
 	if err := vfs.WriteFile(fs, "/foo/a/b/c", []byte("I can \"c\" clearly now"), 0644); err != nil {
 		t.Fatal("Unable to write /foo/a/b/c:", err)
 	}
+
 	if err := fs.Symlink("/foo/a/b", "/foo/also_b"); err != nil {
 		t.Fatal("Unable to symlink /foo/also_b -> /foo/a/b:", err)
 	}
@@ -174,8 +173,60 @@ func TestDirectorySymlink(t *testing.T) {
 	}
 }
 
-// TODO: relative symlinks
-// TODO: overwrite symlinks
+func TestMultipleAndRelativeSymlinks(t *testing.T) {
+	fs := Create()
+	if err := vfs.MkdirAll(fs, "a/real_b/real_c", 0755); err != nil {
+		t.Fatal("Unable mkdir a/real_b/real_c:", err)
+	}
+
+	for _, fsEntry := range []struct {
+		name, link, content string
+	}{
+		{name: "a/b", link: "real_b"},
+		{name: "a/b/c", link: "real_c"},
+		{name: "a/b/c/real_d", content: "Lah dee dah"},
+		{name: "a/b/c/d", link: "real_d"},
+		{name: "a/d", link: "b/c/d"},
+	} {
+		if fsEntry.link != "" {
+			if err := fs.Symlink(fsEntry.link, fsEntry.name); err != nil {
+				t.Fatalf("Unable to symlink %s -> %s: %v", fsEntry.name, fsEntry.link, err)
+			}
+		} else if fsEntry.content != "" {
+			if err := vfs.WriteFile(fs, fsEntry.name, []byte(fsEntry.content), 0644); err != nil {
+				t.Fatalf("Unable to write %s: %v", fsEntry.name, err)
+			}
+		}
+	}
+
+	for _, fn := range []string{
+		"a/b/c/d",
+		"a/d",
+	} {
+		contents, err := vfs.ReadFile(fs, fn)
+		if err != nil {
+			t.Fatalf("Unable to read %s: %v", fn, err)
+		}
+		if string(contents) != "Lah dee dah" {
+			t.Fatalf("Unexpected contents read from %s: %v", fn, err)
+		}
+	}
+}
+
+func TestSymlinkIsNotADirectory(t *testing.T) {
+	fs := Create()
+	if err := vfs.MkdirAll(fs, "a/real_b/real_c", 0755); err != nil {
+		t.Fatal("Unable mkdir a/real_b/real_c:", err)
+	}
+	if err := fs.Symlink("broken", "a/b"); err != nil {
+		t.Fatal("Unable to symlink a/b -> broken:", err)
+	}
+	if err := vfs.WriteFile(fs, "a/b/c", []byte("Whatever"), 0644); !strings.Contains(err.Error(), vfs.ErrNotDirectory.Error()) {
+		t.Fatal("Expected an error when writing a/b/c:", err)
+	}
+}
+
+// TODO: overwrite/remove symlinks
 
 func TestReadDir(t *testing.T) {
 	fs := Create()