io/fs: add ReadLinkFS interface

Added implementations for *io/fs.subFS, os.DirFS, and testing/fstest.MapFS.
Amended testing/fstest.TestFS to check behavior.

Addressed TODOs in archive/tar and os.CopyFS around symbolic links.

I am deliberately not changing archive/zip in this CL,
since it currently does not resolve symlinks
as part of its filesystem implementation.
I am unsure of the compatibility restrictions on doing so,
so figured it would be better to address independently.

testing/fstest.MapFS now includes resolution of symlinks,
with MapFile.Data storing the symlink data.
The behavior change there seemed less intrusive,
especially given its intended usage in tests,
and it is especially helpful in testing the io/fs function implementations.

Fixes #49580

Change-Id: I58ec6915e8cc97341cdbfd9c24c67d1b60139447
Reviewed-on: https://go-review.googlesource.com/c/go/+/385534
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Quim Muntal <[email protected]>
Reviewed-by: Funda Secgin <[email protected]>

Author: zombiezen

api/next/49580.txt

@@ -0,0 +1,8 @@
+pkg io/fs, func Lstat(FS, string) (FileInfo, error) #49580
+pkg io/fs, func ReadLink(FS, string) (string, error) #49580
+pkg io/fs, type ReadLinkFS interface { Lstat, Open, ReadLink } #49580
+pkg io/fs, type ReadLinkFS interface, Lstat(string) (FileInfo, error) #49580
+pkg io/fs, type ReadLinkFS interface, Open(string) (File, error) #49580
+pkg io/fs, type ReadLinkFS interface, ReadLink(string) (string, error) #49580
+pkg testing/fstest, method (MapFS) Lstat(string) (fs.FileInfo, error) #49580
+pkg testing/fstest, method (MapFS) ReadLink(string) (string, error) #49580

doc/next/6-stdlib/99-minor/archive/tar/49580.md

@@ -0,0 +1,2 @@
+The [*Writer.AddFS] implementation now supports symbolic links
+for filesystems that implement [io/fs.ReadLinkFS].

doc/next/6-stdlib/99-minor/io/fs/49580.md

@@ -0,0 +1 @@
+A new [ReadLinkFS] interface provides the ability to read symbolic links in a filesystem.

doc/next/6-stdlib/99-minor/os/49580.md

@@ -0,0 +1,2 @@
+The filesystem returned by [DirFS] implements the new [io/fs.ReadLinkFS] interface.
+[CopyFS] supports symlinks when copying filesystems that implement [io/fs.ReadLinkFS].

doc/next/6-stdlib/99-minor/testing/fstest/49580.md

@@ -0,0 +1,3 @@
+[MapFS] implements the new [io/fs.ReadLinkFS] interface.
+[TestFS] will verify the functionality of the [io/fs.ReadLinkFS] interface if implemented.
+[TestFS] will no longer follow symlinks to avoid unbounded recursion.

src/archive/tar/writer.go

@@ -415,11 +415,17 @@ func (tw *Writer) AddFS(fsys fs.FS) error {
 		if err != nil {
 			return err
 		}
-		// TODO(#49580): Handle symlinks when fs.ReadLinkFS is available.
-		if !d.IsDir() && !info.Mode().IsRegular() {
+		linkTarget := ""
+		if typ := d.Type(); typ == fs.ModeSymlink {
+			var err error
+			linkTarget, err = fs.ReadLink(fsys, name)
+			if err != nil {
+				return err
+			}
+		} else if !typ.IsRegular() && typ != fs.ModeDir {
 			return errors.New("tar: cannot add non-regular file")
 		}
-		h, err := FileInfoHeader(info, "")
+		h, err := FileInfoHeader(info, linkTarget)
 		if err != nil {
 			return err
 		}
@@ -430,7 +436,7 @@ func (tw *Writer) AddFS(fsys fs.FS) error {
 		if err := tw.WriteHeader(h); err != nil {
 			return err
 		}
-		if d.IsDir() {
+		if !d.Type().IsRegular() {
 			return nil
 		}
 		f, err := fsys.Open(name)

src/archive/tar/writer_test.go

@@ -1342,6 +1342,7 @@ func TestWriterAddFS(t *testing.T) {
 		"emptyfolder":          {Mode: 0o755 | os.ModeDir},
 		"file.go":              {Data: []byte("hello")},
 		"subfolder/another.go": {Data: []byte("world")},
+		"symlink.go":           {Mode: 0o777 | os.ModeSymlink, Data: []byte("file.go")},
 		// Notably missing here is the "subfolder" directory. This makes sure even
 		// if we don't have a subfolder directory listed.
 	}
@@ -1370,7 +1371,7 @@ func TestWriterAddFS(t *testing.T) {
 	for _, name := range names {
 		entriesLeft--
 
-		entryInfo, err := fsys.Stat(name)
+		entryInfo, err := fsys.Lstat(name)
 		if err != nil {
 			t.Fatalf("getting entry info error: %v", err)
 		}
@@ -1396,18 +1397,23 @@ func TestWriterAddFS(t *testing.T) {
 				name, entryInfo.Mode(), hdr.FileInfo().Mode())
 		}
 
-		if entryInfo.IsDir() {
-			continue
-		}
-
-		data, err := io.ReadAll(tr)
-		if err != nil {
-			t.Fatal(err)
-		}
-		origdata := fsys[name].Data
-		if string(data) != string(origdata) {
-			t.Fatalf("test fs has file content %v; archive header has %v",
-				data, origdata)
+		switch entryInfo.Mode().Type() {
+		case fs.ModeDir:
+			// No additional checks necessary.
+		case fs.ModeSymlink:
+			origtarget := string(fsys[name].Data)
+			if hdr.Linkname != origtarget {
+				t.Fatalf("test fs has link content %s; archive header %v", origtarget, hdr.Linkname)
+			}
+		default:
+			data, err := io.ReadAll(tr)
+			if err != nil {
+				t.Fatal(err)
+			}
+			origdata := fsys[name].Data
+			if string(data) != string(origdata) {
+				t.Fatalf("test fs has file content %v; archive header has %v", origdata, data)
+			}
 		}
 	}
 	if entriesLeft > 0 {

src/io/fs/readlink.go

@@ -0,0 +1,45 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package fs
+
+// ReadLinkFS is the interface implemented by a file system
+// that supports reading symbolic links.
+type ReadLinkFS interface {
+	FS
+
+	// ReadLink returns the destination of the named symbolic link.
+	// If there is an error, it should be of type [*PathError].
+	ReadLink(name string) (string, error)
+
+	// Lstat returns a [FileInfo] describing the named file.
+	// If the file is a symbolic link, the returned [FileInfo] describes the symbolic link.
+	// Lstat makes no attempt to follow the link.
+	// If there is an error, it should be of type [*PathError].
+	Lstat(name string) (FileInfo, error)
+}
+
+// ReadLink returns the destination of the named symbolic link.
+//
+// If fsys does not implement [ReadLinkFS], then ReadLink returns an error.
+func ReadLink(fsys FS, name string) (string, error) {
+	sym, ok := fsys.(ReadLinkFS)
+	if !ok {
+		return "", &PathError{Op: "readlink", Path: name, Err: ErrInvalid}
+	}
+	return sym.ReadLink(name)
+}
+
+// Lstat returns a [FileInfo] describing the named file.
+// If the file is a symbolic link, the returned [FileInfo] describes the symbolic link.
+// Lstat makes no attempt to follow the link.
+//
+// If fsys does not implement [ReadLinkFS], then Lstat is identical to [Stat].
+func Lstat(fsys FS, name string) (FileInfo, error) {
+	sym, ok := fsys.(ReadLinkFS)
+	if !ok {
+		return Stat(fsys, name)
+	}
+	return sym.Lstat(name)
+}

src/io/fs/readlink_test.go

@@ -0,0 +1,106 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package fs_test
+
+import (
+	. "io/fs"
+	"testing"
+	"testing/fstest"
+)
+
+func TestReadLink(t *testing.T) {
+	testFS := fstest.MapFS{
+		"foo": {
+			Data: []byte("bar"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"bar": {
+			Data: []byte("Hello, World!\n"),
+			Mode: 0o644,
+		},
+
+		"dir/parentlink": {
+			Data: []byte("../bar"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"dir/link": {
+			Data: []byte("file"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"dir/file": {
+			Data: []byte("Hello, World!\n"),
+			Mode: 0o644,
+		},
+	}
+
+	check := func(fsys FS, name string, want string) {
+		t.Helper()
+		got, err := ReadLink(fsys, name)
+		if got != want || err != nil {
+			t.Errorf("ReadLink(%q) = %q, %v; want %q, <nil>", name, got, err, want)
+		}
+	}
+
+	check(testFS, "foo", "bar")
+	check(testFS, "dir/parentlink", "../bar")
+	check(testFS, "dir/link", "file")
+
+	// Test that ReadLink on Sub works.
+	sub, err := Sub(testFS, "dir")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	check(sub, "link", "file")
+	check(sub, "parentlink", "../bar")
+}
+
+func TestLstat(t *testing.T) {
+	testFS := fstest.MapFS{
+		"foo": {
+			Data: []byte("bar"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"bar": {
+			Data: []byte("Hello, World!\n"),
+			Mode: 0o644,
+		},
+
+		"dir/parentlink": {
+			Data: []byte("../bar"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"dir/link": {
+			Data: []byte("file"),
+			Mode: ModeSymlink | 0o777,
+		},
+		"dir/file": {
+			Data: []byte("Hello, World!\n"),
+			Mode: 0o644,
+		},
+	}
+
+	check := func(fsys FS, name string, want FileMode) {
+		t.Helper()
+		info, err := Lstat(fsys, name)
+		var got FileMode
+		if err == nil {
+			got = info.Mode()
+		}
+		if got != want || err != nil {
+			t.Errorf("Lstat(%q) = %v, %v; want %v, <nil>", name, got, err, want)
+		}
+	}
+
+	check(testFS, "foo", ModeSymlink|0o777)
+	check(testFS, "bar", 0o644)
+
+	// Test that Lstat on Sub works.
+	sub, err := Sub(testFS, "dir")
+	if err != nil {
+		t.Fatal(err)
+	}
+	check(sub, "link", ModeSymlink|0o777)
+}

src/io/fs/sub.go

@@ -23,7 +23,8 @@ type SubFS interface {
 // Otherwise, if fs implements [SubFS], Sub returns fsys.Sub(dir).
 // Otherwise, Sub returns a new [FS] implementation sub that,
 // in effect, implements sub.Open(name) as fsys.Open(path.Join(dir, name)).
-// The implementation also translates calls to ReadDir, ReadFile, and Glob appropriately.
+// The implementation also translates calls to ReadDir, ReadFile,
+// ReadLink, Lstat, and Glob appropriately.
 //
 // Note that Sub(os.DirFS("/"), "prefix") is equivalent to os.DirFS("/prefix")
 // and that neither of them guarantees to avoid operating system
@@ -44,6 +45,12 @@ func Sub(fsys FS, dir string) (FS, error) {
 	return &subFS{fsys, dir}, nil
 }
 
+var _ FS = (*subFS)(nil)
+var _ ReadDirFS = (*subFS)(nil)
+var _ ReadFileFS = (*subFS)(nil)
+var _ ReadLinkFS = (*subFS)(nil)
+var _ GlobFS = (*subFS)(nil)
+
 type subFS struct {
 	fsys FS
 	dir  string
@@ -105,6 +112,30 @@ func (f *subFS) ReadFile(name string) ([]byte, error) {
 	return data, f.fixErr(err)
 }
 
+func (f *subFS) ReadLink(name string) (string, error) {
+	full, err := f.fullName("readlink", name)
+	if err != nil {
+		return "", err
+	}
+	target, err := ReadLink(f.fsys, full)
+	if err != nil {
+		return "", f.fixErr(err)
+	}
+	return target, nil
+}
+
+func (f *subFS) Lstat(name string) (FileInfo, error) {
+	full, err := f.fullName("lstat", name)
+	if err != nil {
+		return nil, err
+	}
+	info, err := Lstat(f.fsys, full)
+	if err != nil {
+		return nil, f.fixErr(err)
+	}
+	return info, nil
+}
+
 func (f *subFS) Glob(pattern string) ([]string, error) {
 	// Check pattern is well-formed.
 	if _, err := path.Match(pattern, ""); err != nil {

src/io/fs/walk_test.go

@@ -110,6 +110,47 @@ func TestWalkDir(t *testing.T) {
 	})
 }
 
+func TestWalkDirSymlink(t *testing.T) {
+	fsys := fstest.MapFS{
+		"link":    {Data: []byte("dir"), Mode: ModeSymlink},
+		"dir/a":   {},
+		"dir/b/c": {},
+		"dir/d":   {Data: []byte("b"), Mode: ModeSymlink},
+	}
+
+	wantTypes := map[string]FileMode{
+		"link":     ModeDir,
+		"link/a":   0,
+		"link/b":   ModeDir,
+		"link/b/c": 0,
+		"link/d":   ModeSymlink,
+	}
+	marks := make(map[string]int)
+	walkFn := func(path string, entry DirEntry, err error) error {
+		marks[path]++
+		if want, ok := wantTypes[path]; !ok {
+			t.Errorf("Unexpected path %q in walk", path)
+		} else if got := entry.Type(); got != want {
+			t.Errorf("%s entry type = %v; want %v", path, got, want)
+		}
+		if err != nil {
+			t.Errorf("Walking %s: %v", path, err)
+		}
+		return nil
+	}
+
+	// Expect no errors.
+	err := WalkDir(fsys, "link", walkFn)
+	if err != nil {
+		t.Fatalf("no error expected, found: %s", err)
+	}
+	for path := range wantTypes {
+		if got := marks[path]; got != 1 {
+			t.Errorf("%s visited %d times; expected 1", path, got)
+		}
+	}
+}
+
 func TestIssue51617(t *testing.T) {
 	dir := t.TempDir()
 	for _, sub := range []string{"a", filepath.Join("a", "bad"), filepath.Join("a", "next")} {