From a03da936f282465650b41e5e066c1f589000b3df Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Fri, 12 Mar 2021 22:03:34 -0500 Subject: [PATCH 01/17] txtar: add fs.FS support Fixes https://github.com/golang/go/issues/44158 From implementation copied with permission from https://github.com/josharian/txtarfs/blob/main/txtarfs_test.go https://github.com/golang/go/issues/44158#issuecomment-797672335 --- txtar/fs.go | 221 +++++++++++++++++++++++++++++++++++++++++++++++ txtar/fs_test.go | 93 ++++++++++++++++++++ 2 files changed, 314 insertions(+) create mode 100644 txtar/fs.go create mode 100644 txtar/fs_test.go diff --git a/txtar/fs.go b/txtar/fs.go new file mode 100644 index 00000000000..caa1ea954c3 --- /dev/null +++ b/txtar/fs.go @@ -0,0 +1,221 @@ +// Copyright 2021 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. + +// +build go1.16 + +package txtar + +import ( + "errors" + "io" + "io/fs" + "path" + "sort" + "strings" + "time" +) + +var _ fs.FS = (*Archive)(nil) + +// Open implements fs.FS. +func (a *Archive) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + + for _, f := range a.Files { + if f.Name == name { + return &openFile{f, 0}, nil + } + } + var list []fileInfo + var dirs = make(map[string]bool) + if name == "." { + for _, f := range a.Files { + i := strings.Index(f.Name, "/") + if i < 0 { + list = append(list, fileInfo{f, 0666}) + } else { + dirs[f.Name[:i]] = true + } + } + } else { + prefix := name + "/" + for _, f := range a.Files { + if strings.HasPrefix(f.Name, prefix) { + felem := f.Name[len(prefix):] + i := strings.Index(felem, "/") + if i < 0 { + list = append(list, fileInfo{f, 0666}) + } else { + dirs[f.Name[len(prefix):len(prefix)+i]] = true + } + } + } + // If there are no children of the name, + // then the directory is treated as not existing. + if list == nil && len(dirs) == 0 { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + } + for name := range dirs { + list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0666}) + } + sort.Slice(list, func(i, j int) bool { + return list[i].File.Name < list[j].File.Name + }) + + return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0666}, list, 0}, nil +} + +var _ fs.ReadFileFS = (*Archive)(nil) + +// ReadFile implements fs.ReadFileFS. +func (a *Archive) ReadFile(name string) ([]byte, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + if name == "." { + return nil, &fs.PathError{Op: "read", Path: name, Err: errors.New("is a directory")} + } + prefix := name + "/" + for _, f := range a.Files { + if f.Name == name { + return f.Data, nil + } + // It's a directory + if strings.HasPrefix(f.Name, prefix) { + return nil, &fs.PathError{Op: "read", Path: name, Err: errors.New("is a directory")} + } + } + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} +} + +var _ fs.File = (*openFile)(nil) + +type openFile struct { + File + offset int64 +} + +func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0666}, nil } + +func (o *openFile) Close() error { return nil } + +func (f *openFile) Read(b []byte) (int, error) { + if f.offset >= int64(len(f.File.Data)) { + return 0, io.EOF + } + if f.offset < 0 { + return 0, &fs.PathError{Op: "read", Path: f.File.Name, Err: fs.ErrInvalid} + } + n := copy(b, f.File.Data[f.offset:]) + f.offset += int64(n) + return n, nil +} + +func (f *openFile) Seek(offset int64, whence int) (int64, error) { + switch whence { + case 0: + // offset += 0 + case 1: + offset += f.offset + case 2: + offset += int64(len(f.File.Data)) + } + if offset < 0 || offset > int64(len(f.File.Data)) { + return 0, &fs.PathError{Op: "seek", Path: f.File.Name, Err: fs.ErrInvalid} + } + f.offset = offset + return offset, nil +} + +func (f *openFile) ReadAt(b []byte, offset int64) (int, error) { + if offset < 0 || offset > int64(len(f.File.Data)) { + return 0, &fs.PathError{Op: "read", Path: f.File.Name, Err: fs.ErrInvalid} + } + n := copy(b, f.File.Data[offset:]) + if n < len(b) { + return n, io.EOF + } + return n, nil +} + +var _ fs.FileInfo = fileInfo{} + +type fileInfo struct { + File + m fs.FileMode +} + +func (f fileInfo) Name() string { return path.Base(f.File.Name) } +func (f fileInfo) Size() int64 { return int64(len(f.File.Data)) } +func (f fileInfo) Mode() fs.FileMode { return f.m } +func (f fileInfo) Type() fs.FileMode { return f.m.Type() } +func (f fileInfo) ModTime() time.Time { return time.Time{} } +func (f fileInfo) IsDir() bool { return f.m.IsDir() } +func (f fileInfo) Sys() interface{} { return f.File } +func (f fileInfo) Info() (fs.FileInfo, error) { return f, nil } + +type openDir struct { + path string + fileInfo + entry []fileInfo + offset int +} + +func (d *openDir) Stat() (fs.FileInfo, error) { return &d.fileInfo, nil } +func (d *openDir) Close() error { return nil } +func (d *openDir) Read(b []byte) (int, error) { + return 0, &fs.PathError{Op: "read", Path: d.path, Err: errors.New("is a directory")} +} + +func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { + n := len(d.entry) - d.offset + if count > 0 && n > count { + n = count + } + if n == 0 && count > 0 { + return nil, io.EOF + } + list := make([]fs.DirEntry, n) + for i := range list { + list[i] = &d.entry[d.offset+i] + } + d.offset += n + return list, nil +} + +// From constructs an Archive with the contents of fsys and an empty Comment. +// Subsequent changes to fsys are not reflected in the returned archive. +// +// The transformation is lossy. +// For example, because directories are implicit in txtar archives, +// empty directories in fsys will be lost, and txtar does not represent file mode, mtime, or other file metadata. +// From does not guarantee that a.File[i].Data contain no file marker lines. +// See also warnings on Format. +// In short, it is unwise to use txtar as a generic filesystem serialization mechanism. +func From(fsys fs.FS) (*Archive, error) { + ar := new(Archive) + walkfn := func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + // Directories in txtar are implicit. + return nil + } + data, err := fs.ReadFile(fsys, path) + if err != nil { + return err + } + ar.Files = append(ar.Files, File{Name: path, Data: data}) + return nil + } + + if err := fs.WalkDir(fsys, ".", walkfn); err != nil { + return nil, err + } + return ar, nil +} diff --git a/txtar/fs_test.go b/txtar/fs_test.go new file mode 100644 index 00000000000..a8fde9a151a --- /dev/null +++ b/txtar/fs_test.go @@ -0,0 +1,93 @@ +// Copyright 2021 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 txtar + +import ( + "io/fs" + "sort" + "strings" + "testing" + "testing/fstest" +) + +func TestFS(t *testing.T) { + for _, tc := range []struct{ name, input, files string }{ + { + name: "empty", + input: ``, + files: "", + }, + { + name: "one", + input: ` +-- one.txt -- +one +`, + files: "one.txt", + }, + { + name: "two", + input: ` +-- one.txt -- +one +-- two.txt -- +two +`, + files: "one.txt two.txt", + }, + { + name: "subdirectories", + input: ` +-- one.txt -- +one +-- 2/two.txt -- +two +-- 2/3/three.txt -- +three +-- 4/four.txt -- +three +`, + files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", + }, + } { + t.Run(tc.name, func(t *testing.T) { + a := Parse([]byte(tc.input)) + files := strings.Fields(tc.files) + if err := fstest.TestFS(a, files...); err != nil { + t.Fatal(err) + } + for _, name := range files { + for _, f := range a.Files { + if f.Name == name { + b, err := fs.ReadFile(a, name) + if err != nil { + t.Fatal(err) + } + if string(b) != string(f.Data) { + t.Fatalf("mismatched contents for %q", name) + } + } + } + } + a2, err := From(a) + if err != nil { + t.Fatalf("failed to write fsys for %v: %v", tc.name, err) + } + + if in, out := normalized(a), normalized(a2); in != out { + t.Errorf("From round trip failed: %q != %q", in, out) + } + + }) + } +} + +func normalized(a *Archive) string { + a.Comment = nil + sort.Slice(a.Files, func(i, j int) bool { + return a.Files[i].Name < a.Files[j].Name + }) + return string(Format(a)) +} From cccd7f89fee7e36019b656de2e16251a298d17cb Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Mon, 15 Mar 2021 21:15:10 -0400 Subject: [PATCH 02/17] ILT suggestions --- txtar/fs.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index caa1ea954c3..602f6ebba93 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -11,7 +11,6 @@ import ( "io" "io/fs" "path" - "sort" "strings" "time" ) @@ -35,7 +34,7 @@ func (a *Archive) Open(name string) (fs.File, error) { for _, f := range a.Files { i := strings.Index(f.Name, "/") if i < 0 { - list = append(list, fileInfo{f, 0666}) + list = append(list, fileInfo{f, 0444}) } else { dirs[f.Name[:i]] = true } @@ -47,7 +46,7 @@ func (a *Archive) Open(name string) (fs.File, error) { felem := f.Name[len(prefix):] i := strings.Index(felem, "/") if i < 0 { - list = append(list, fileInfo{f, 0666}) + list = append(list, fileInfo{f, 0444}) } else { dirs[f.Name[len(prefix):len(prefix)+i]] = true } @@ -60,13 +59,10 @@ func (a *Archive) Open(name string) (fs.File, error) { } } for name := range dirs { - list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0666}) + list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0444}) } - sort.Slice(list, func(i, j int) bool { - return list[i].File.Name < list[j].File.Name - }) - return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0666}, list, 0}, nil + return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0444}, list, 0}, nil } var _ fs.ReadFileFS = (*Archive)(nil) @@ -82,7 +78,7 @@ func (a *Archive) ReadFile(name string) ([]byte, error) { prefix := name + "/" for _, f := range a.Files { if f.Name == name { - return f.Data, nil + return append(([]byte)(nil), f.Data...), nil } // It's a directory if strings.HasPrefix(f.Name, prefix) { @@ -99,7 +95,7 @@ type openFile struct { offset int64 } -func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0666}, nil } +func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0444}, nil } func (o *openFile) Close() error { return nil } From 54914df36578c6f009c8101cc72eebae1e7d23c0 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Mon, 15 Mar 2021 21:30:19 -0400 Subject: [PATCH 03/17] More ILT suggestions, simplify --- txtar/fs.go | 78 +++++++++++++++++------------------------------------ 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 602f6ebba93..7aa2ad1712d 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -7,6 +7,7 @@ package txtar import ( + "bytes" "errors" "io" "io/fs" @@ -25,39 +26,33 @@ func (a *Archive) Open(name string) (fs.File, error) { for _, f := range a.Files { if f.Name == name { - return &openFile{f, 0}, nil + return newOpenFile(f), nil } } var list []fileInfo var dirs = make(map[string]bool) + prefix := name + "/" if name == "." { - for _, f := range a.Files { - i := strings.Index(f.Name, "/") + prefix = "" + } + + for _, f := range a.Files { + if strings.HasPrefix(f.Name, prefix) { + felem := f.Name[len(prefix):] + i := strings.Index(felem, "/") if i < 0 { list = append(list, fileInfo{f, 0444}) } else { - dirs[f.Name[:i]] = true + dirs[felem[:i]] = true } } - } else { - prefix := name + "/" - for _, f := range a.Files { - if strings.HasPrefix(f.Name, prefix) { - felem := f.Name[len(prefix):] - i := strings.Index(felem, "/") - if i < 0 { - list = append(list, fileInfo{f, 0444}) - } else { - dirs[f.Name[len(prefix):len(prefix)+i]] = true - } - } - } - // If there are no children of the name, - // then the directory is treated as not existing. - if list == nil && len(dirs) == 0 { - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} - } } + // If there are no children of the name, + // then the directory is treated as not existing. + if len(list) == 0 && len(dirs) == 0 && name != "." { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + for name := range dirs { list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0444}) } @@ -92,7 +87,11 @@ var _ fs.File = (*openFile)(nil) type openFile struct { File - offset int64 + *bytes.Reader +} + +func newOpenFile(f File) *openFile { + return &openFile{f, bytes.NewReader(f.Data)} } func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0444}, nil } @@ -100,42 +99,15 @@ func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0444}, func (o *openFile) Close() error { return nil } func (f *openFile) Read(b []byte) (int, error) { - if f.offset >= int64(len(f.File.Data)) { - return 0, io.EOF - } - if f.offset < 0 { - return 0, &fs.PathError{Op: "read", Path: f.File.Name, Err: fs.ErrInvalid} - } - n := copy(b, f.File.Data[f.offset:]) - f.offset += int64(n) - return n, nil + return f.Reader.Read(b) } func (f *openFile) Seek(offset int64, whence int) (int64, error) { - switch whence { - case 0: - // offset += 0 - case 1: - offset += f.offset - case 2: - offset += int64(len(f.File.Data)) - } - if offset < 0 || offset > int64(len(f.File.Data)) { - return 0, &fs.PathError{Op: "seek", Path: f.File.Name, Err: fs.ErrInvalid} - } - f.offset = offset - return offset, nil + return f.Reader.Seek(offset, whence) } func (f *openFile) ReadAt(b []byte, offset int64) (int, error) { - if offset < 0 || offset > int64(len(f.File.Data)) { - return 0, &fs.PathError{Op: "read", Path: f.File.Name, Err: fs.ErrInvalid} - } - n := copy(b, f.File.Data[offset:]) - if n < len(b) { - return n, io.EOF - } - return n, nil + return f.Reader.ReadAt(b, offset) } var _ fs.FileInfo = fileInfo{} From 45be54e4832a43abd5f73e6fc3e115c0d13b7572 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Mon, 15 Mar 2021 21:47:56 -0400 Subject: [PATCH 04/17] More tests, more bytes.Reader stuff --- txtar/fs.go | 24 +++++++++++++++++++++-- txtar/fs_test.go | 50 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 7aa2ad1712d..69f3dac4af2 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -102,12 +102,32 @@ func (f *openFile) Read(b []byte) (int, error) { return f.Reader.Read(b) } +func (f *openFile) ReadAt(b []byte, offset int64) (int, error) { + return f.Reader.ReadAt(b, offset) +} + +func (f *openFile) ReadByte() (byte, error) { + return f.Reader.ReadByte() +} + +func (f *openFile) ReadRune() (ch rune, size int, err error) { + return f.Reader.ReadRune() +} + func (f *openFile) Seek(offset int64, whence int) (int64, error) { return f.Reader.Seek(offset, whence) } -func (f *openFile) ReadAt(b []byte, offset int64) (int, error) { - return f.Reader.ReadAt(b, offset) +func (f *openFile) UnreadByte() error { + return f.Reader.UnreadByte() +} + +func (f *openFile) UnreadRune() error { + return f.Reader.UnreadRune() +} + +func (f *openFile) WriteTo(w io.Writer) (n int64, err error) { + return f.Reader.WriteTo(w) } var _ fs.FileInfo = fileInfo{} diff --git a/txtar/fs_test.go b/txtar/fs_test.go index a8fde9a151a..4f7897044b8 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -5,11 +5,13 @@ package txtar import ( + "io" "io/fs" "sort" "strings" "testing" "testing/fstest" + "testing/iotest" ) func TestFS(t *testing.T) { @@ -60,14 +62,46 @@ three } for _, name := range files { for _, f := range a.Files { - if f.Name == name { - b, err := fs.ReadFile(a, name) - if err != nil { - t.Fatal(err) - } - if string(b) != string(f.Data) { - t.Fatalf("mismatched contents for %q", name) - } + if f.Name != name { + continue + } + b, err := fs.ReadFile(a, name) + if err != nil { + t.Fatal(err) + } + if string(b) != string(f.Data) { + t.Fatalf("mismatched contents for %q", name) + } + // Be careful with n cases, this open is O(n^3) deep + // Do iotest + fsfile, err := a.Open(name) + if err != nil { + t.Fatal(err) + } + if err = iotest.TestReader(fsfile, f.Data); err != nil { + t.Fatal(err) + } + if err = fsfile.Close(); err != nil { + t.Fatal(err) + } + // test io.Copy + fsfile, err = a.Open(name) + if err != nil { + t.Fatal(err) + } + var buf strings.Builder + n, err := io.Copy(&buf, fsfile) + if err != nil { + t.Fatal(err) + } + if n != int64(len(f.Data)) { + t.Fatalf("bad copy size: %d", n) + } + if buf.String() != string(f.Data) { + t.Fatalf("mismatched contents for io.Copy of %q", name) + } + if err = fsfile.Close(); err != nil { + t.Fatal(err) } } } From 5d4035bfa3b09a84720e8bfbe6b40b7ec006da65 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Mon, 15 Mar 2021 21:54:04 -0400 Subject: [PATCH 05/17] Dir mode is 555 --- txtar/fs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 69f3dac4af2..e131bbf7cc3 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -54,10 +54,10 @@ func (a *Archive) Open(name string) (fs.File, error) { } for name := range dirs { - list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0444}) + list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0555}) } - return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0444}, list, 0}, nil + return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0555}, list, 0}, nil } var _ fs.ReadFileFS = (*Archive)(nil) From ae71cca78082446afe3eb4d3cd600ef4b8b722e2 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 16 Mar 2021 09:01:12 -0400 Subject: [PATCH 06/17] Clean filenames in archive --- txtar/fs.go | 24 ++++++++++++++---------- txtar/fs_test.go | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index e131bbf7cc3..7e1c24bc025 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -25,7 +25,9 @@ func (a *Archive) Open(name string) (fs.File, error) { } for _, f := range a.Files { - if f.Name == name { + // In case the txtar has weird filenames + cleanName := path.Clean(f.Name) + if name == cleanName { return newOpenFile(f), nil } } @@ -37,14 +39,16 @@ func (a *Archive) Open(name string) (fs.File, error) { } for _, f := range a.Files { - if strings.HasPrefix(f.Name, prefix) { - felem := f.Name[len(prefix):] - i := strings.Index(felem, "/") - if i < 0 { - list = append(list, fileInfo{f, 0444}) - } else { - dirs[felem[:i]] = true - } + cleanName := path.Clean(f.Name) + if !strings.HasPrefix(cleanName, prefix) { + continue + } + felem := cleanName[len(prefix):] + i := strings.Index(felem, "/") + if i < 0 { + list = append(list, fileInfo{f, 0444}) + } else { + dirs[felem[:i]] = true } } // If there are no children of the name, @@ -72,7 +76,7 @@ func (a *Archive) ReadFile(name string) ([]byte, error) { } prefix := name + "/" for _, f := range a.Files { - if f.Name == name { + if cleanName := path.Clean(f.Name); name == cleanName { return append(([]byte)(nil), f.Data...), nil } // It's a directory diff --git a/txtar/fs_test.go b/txtar/fs_test.go index 4f7897044b8..97925fd0f2a 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -7,6 +7,7 @@ package txtar import ( "io" "io/fs" + "path" "sort" "strings" "testing" @@ -53,6 +54,16 @@ three `, files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", }, + { + name: "unclean file names", + input: ` +-- 1/../one.txt -- +one +-- 2/sub/../two.txt -- +two +`, + files: "one.txt 2/two.txt", + }, } { t.Run(tc.name, func(t *testing.T) { a := Parse([]byte(tc.input)) @@ -113,13 +124,16 @@ three if in, out := normalized(a), normalized(a2); in != out { t.Errorf("From round trip failed: %q != %q", in, out) } - }) } } func normalized(a *Archive) string { a.Comment = nil + for i := range a.Files { + f := &a.Files[i] + f.Name = path.Clean(f.Name) + } sort.Slice(a.Files, func(i, j int) bool { return a.Files[i].Name < a.Files[j].Name }) From 7dc7a1a392f9a424359701f7d9c814079f0dc851 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 16 Mar 2021 09:07:30 -0400 Subject: [PATCH 07/17] simplify openFile --- txtar/fs.go | 45 +++++++++------------------------------------ 1 file changed, 9 insertions(+), 36 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 7e1c24bc025..554f8b4e552 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -87,53 +87,26 @@ func (a *Archive) ReadFile(name string) ([]byte, error) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } -var _ fs.File = (*openFile)(nil) +var ( + _ fs.File = (*openFile)(nil) + _ io.ReadSeekCloser = (*openFile)(nil) + _ io.ReaderAt = (*openFile)(nil) + _ io.WriterTo = (*openFile)(nil) +) type openFile struct { - File *bytes.Reader + fileInfo } func newOpenFile(f File) *openFile { - return &openFile{f, bytes.NewReader(f.Data)} + return &openFile{bytes.NewReader(f.Data), fileInfo{f, 0444}} } -func (o *openFile) Stat() (fs.FileInfo, error) { return fileInfo{o.File, 0444}, nil } +func (o *openFile) Stat() (fs.FileInfo, error) { return o.fileInfo, nil } func (o *openFile) Close() error { return nil } -func (f *openFile) Read(b []byte) (int, error) { - return f.Reader.Read(b) -} - -func (f *openFile) ReadAt(b []byte, offset int64) (int, error) { - return f.Reader.ReadAt(b, offset) -} - -func (f *openFile) ReadByte() (byte, error) { - return f.Reader.ReadByte() -} - -func (f *openFile) ReadRune() (ch rune, size int, err error) { - return f.Reader.ReadRune() -} - -func (f *openFile) Seek(offset int64, whence int) (int64, error) { - return f.Reader.Seek(offset, whence) -} - -func (f *openFile) UnreadByte() error { - return f.Reader.UnreadByte() -} - -func (f *openFile) UnreadRune() error { - return f.Reader.UnreadRune() -} - -func (f *openFile) WriteTo(w io.Writer) (n int64, err error) { - return f.Reader.WriteTo(w) -} - var _ fs.FileInfo = fileInfo{} type fileInfo struct { From 474e4e60a28e25d423d00648301c6a9d6eff541b Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 16 Mar 2021 09:41:00 -0400 Subject: [PATCH 08/17] better variable names --- txtar/fs.go | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 554f8b4e552..cad89c57add 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -31,8 +31,8 @@ func (a *Archive) Open(name string) (fs.File, error) { return newOpenFile(f), nil } } - var list []fileInfo - var dirs = make(map[string]bool) + var entries []fileInfo + dirs := make(map[string]bool) prefix := name + "/" if name == "." { prefix = "" @@ -46,22 +46,23 @@ func (a *Archive) Open(name string) (fs.File, error) { felem := cleanName[len(prefix):] i := strings.Index(felem, "/") if i < 0 { - list = append(list, fileInfo{f, 0444}) + entries = append(entries, newFileInfo(f)) } else { dirs[felem[:i]] = true } } // If there are no children of the name, - // then the directory is treated as not existing. - if len(list) == 0 && len(dirs) == 0 && name != "." { + // then the directory is treated as not existing + // unless the directory is "." + if len(entries) == 0 && len(dirs) == 0 && name != "." { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } for name := range dirs { - list = append(list, fileInfo{File{Name: name}, fs.ModeDir | 0555}) + entries = append(entries, newDirInfo(name)) } - return &openDir{name, fileInfo{File{Name: name}, fs.ModeDir | 0555}, list, 0}, nil + return &openDir{newDirInfo(name), entries, 0}, nil } var _ fs.ReadFileFS = (*Archive)(nil) @@ -114,6 +115,14 @@ type fileInfo struct { m fs.FileMode } +func newFileInfo(f File) fileInfo { + return fileInfo{f, 0444} +} + +func newDirInfo(name string) fileInfo { + return fileInfo{File{Name: name}, fs.ModeDir | 0555} +} + func (f fileInfo) Name() string { return path.Base(f.File.Name) } func (f fileInfo) Size() int64 { return int64(len(f.File.Data)) } func (f fileInfo) Mode() fs.FileMode { return f.m } @@ -124,32 +133,31 @@ func (f fileInfo) Sys() interface{} { return f.File } func (f fileInfo) Info() (fs.FileInfo, error) { return f, nil } type openDir struct { - path string - fileInfo - entry []fileInfo - offset int + dirInfo fileInfo + entries []fileInfo + offset int } -func (d *openDir) Stat() (fs.FileInfo, error) { return &d.fileInfo, nil } +func (d *openDir) Stat() (fs.FileInfo, error) { return &d.dirInfo, nil } func (d *openDir) Close() error { return nil } func (d *openDir) Read(b []byte) (int, error) { - return 0, &fs.PathError{Op: "read", Path: d.path, Err: errors.New("is a directory")} + return 0, &fs.PathError{Op: "read", Path: d.dirInfo.File.Name, Err: errors.New("is a directory")} } func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { - n := len(d.entry) - d.offset + n := len(d.entries) - d.offset if count > 0 && n > count { n = count } if n == 0 && count > 0 { return nil, io.EOF } - list := make([]fs.DirEntry, n) - for i := range list { - list[i] = &d.entry[d.offset+i] + entries := make([]fs.DirEntry, n) + for i := range entries { + entries[i] = &d.entries[d.offset+i] } d.offset += n - return list, nil + return entries, nil } // From constructs an Archive with the contents of fsys and an empty Comment. From 224f1d080ef98cd94fb160cd986d5c43e37da305 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Thu, 17 Nov 2022 16:33:10 -0500 Subject: [PATCH 09/17] Respond to feedback; make fsys separate from Archive --- txtar/archive.go | 6 ++++++ txtar/fs.go | 19 ++++++++++--------- txtar/fs_test.go | 11 ++++++----- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/txtar/archive.go b/txtar/archive.go index fd95f1e64a1..14dc68b85e1 100644 --- a/txtar/archive.go +++ b/txtar/archive.go @@ -34,6 +34,7 @@ package txtar import ( "bytes" "fmt" + "io/fs" "os" "strings" ) @@ -138,3 +139,8 @@ func fixNL(data []byte) []byte { d[len(data)] = '\n' return d } + +// FS returns an fs.FS that reads from the Archive. +func (a *Archive) FS() fs.FS { + return fsys{a} +} diff --git a/txtar/fs.go b/txtar/fs.go index cad89c57add..ad996b1841d 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -1,9 +1,7 @@ -// Copyright 2021 The Go Authors. All rights reserved. +// Copyright 2022 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. -// +build go1.16 - package txtar import ( @@ -16,10 +14,12 @@ import ( "time" ) -var _ fs.FS = (*Archive)(nil) +type fsys struct { + *Archive +} // Open implements fs.FS. -func (a *Archive) Open(name string) (fs.File, error) { +func (a fsys) Open(name string) (fs.File, error) { if !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } @@ -65,10 +65,10 @@ func (a *Archive) Open(name string) (fs.File, error) { return &openDir{newDirInfo(name), entries, 0}, nil } -var _ fs.ReadFileFS = (*Archive)(nil) +var _ fs.ReadFileFS = fsys{} // ReadFile implements fs.ReadFileFS. -func (a *Archive) ReadFile(name string) ([]byte, error) { +func (a fsys) ReadFile(name string) ([]byte, error) { if !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } @@ -165,8 +165,9 @@ func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { // // The transformation is lossy. // For example, because directories are implicit in txtar archives, -// empty directories in fsys will be lost, and txtar does not represent file mode, mtime, or other file metadata. -// From does not guarantee that a.File[i].Data contain no file marker lines. +// empty directories in fsys will be lost, +// and txtar does not represent file mode, mtime, or other file metadata. +// From does not guarantee that a.File[i].Data contains no file marker lines. // See also warnings on Format. // In short, it is unwise to use txtar as a generic filesystem serialization mechanism. func From(fsys fs.FS) (*Archive, error) { diff --git a/txtar/fs_test.go b/txtar/fs_test.go index 97925fd0f2a..3c853c7ed96 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -68,7 +68,7 @@ two t.Run(tc.name, func(t *testing.T) { a := Parse([]byte(tc.input)) files := strings.Fields(tc.files) - if err := fstest.TestFS(a, files...); err != nil { + if err := fstest.TestFS(a.FS(), files...); err != nil { t.Fatal(err) } for _, name := range files { @@ -76,7 +76,8 @@ two if f.Name != name { continue } - b, err := fs.ReadFile(a, name) + fsys := a.FS() + b, err := fs.ReadFile(fsys, name) if err != nil { t.Fatal(err) } @@ -85,7 +86,7 @@ two } // Be careful with n cases, this open is O(n^3) deep // Do iotest - fsfile, err := a.Open(name) + fsfile, err := fsys.Open(name) if err != nil { t.Fatal(err) } @@ -96,7 +97,7 @@ two t.Fatal(err) } // test io.Copy - fsfile, err = a.Open(name) + fsfile, err = fsys.Open(name) if err != nil { t.Fatal(err) } @@ -116,7 +117,7 @@ two } } } - a2, err := From(a) + a2, err := From(a.FS()) if err != nil { t.Fatalf("failed to write fsys for %v: %v", tc.name, err) } From 241c69779ae126e91ec7b22a2b52fb8fd3533c02 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Thu, 17 Nov 2022 16:39:35 -0500 Subject: [PATCH 10/17] copyright --- txtar/fs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txtar/fs_test.go b/txtar/fs_test.go index 3c853c7ed96..44f480898c2 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 The Go Authors. All rights reserved. +// Copyright 2022 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. From 6a3f80366d86da9eb37657f4a698061257e0b2e4 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Fri, 18 Nov 2022 09:38:11 -0500 Subject: [PATCH 11/17] Unembed structs --- txtar/archive.go | 2 +- txtar/fs.go | 37 ++++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/txtar/archive.go b/txtar/archive.go index 14dc68b85e1..8f10f16ad62 100644 --- a/txtar/archive.go +++ b/txtar/archive.go @@ -142,5 +142,5 @@ func fixNL(data []byte) []byte { // FS returns an fs.FS that reads from the Archive. func (a *Archive) FS() fs.FS { - return fsys{a} + return archiveFS{a} } diff --git a/txtar/fs.go b/txtar/fs.go index ad996b1841d..2ede4108642 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -14,17 +14,17 @@ import ( "time" ) -type fsys struct { - *Archive +type archiveFS struct { + a *Archive } // Open implements fs.FS. -func (a fsys) Open(name string) (fs.File, error) { +func (fsys archiveFS) Open(name string) (fs.File, error) { if !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } - for _, f := range a.Files { + for _, f := range fsys.a.Files { // In case the txtar has weird filenames cleanName := path.Clean(f.Name) if name == cleanName { @@ -38,7 +38,7 @@ func (a fsys) Open(name string) (fs.File, error) { prefix = "" } - for _, f := range a.Files { + for _, f := range fsys.a.Files { cleanName := path.Clean(f.Name) if !strings.HasPrefix(cleanName, prefix) { continue @@ -65,10 +65,10 @@ func (a fsys) Open(name string) (fs.File, error) { return &openDir{newDirInfo(name), entries, 0}, nil } -var _ fs.ReadFileFS = fsys{} +var _ fs.ReadFileFS = archiveFS{} // ReadFile implements fs.ReadFileFS. -func (a fsys) ReadFile(name string) ([]byte, error) { +func (fsys archiveFS) ReadFile(name string) ([]byte, error) { if !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} } @@ -76,7 +76,7 @@ func (a fsys) ReadFile(name string) ([]byte, error) { return nil, &fs.PathError{Op: "read", Path: name, Err: errors.New("is a directory")} } prefix := name + "/" - for _, f := range a.Files { + for _, f := range fsys.a.Files { if cleanName := path.Clean(f.Name); name == cleanName { return append(([]byte)(nil), f.Data...), nil } @@ -96,22 +96,25 @@ var ( ) type openFile struct { - *bytes.Reader - fileInfo + bytes.Reader + fi fileInfo } func newOpenFile(f File) *openFile { - return &openFile{bytes.NewReader(f.Data), fileInfo{f, 0444}} + var o openFile + o.Reader.Reset(f.Data) + o.fi = fileInfo{f, 0444} + return &o } -func (o *openFile) Stat() (fs.FileInfo, error) { return o.fileInfo, nil } +func (o *openFile) Stat() (fs.FileInfo, error) { return o.fi, nil } func (o *openFile) Close() error { return nil } var _ fs.FileInfo = fileInfo{} type fileInfo struct { - File + f File m fs.FileMode } @@ -123,13 +126,13 @@ func newDirInfo(name string) fileInfo { return fileInfo{File{Name: name}, fs.ModeDir | 0555} } -func (f fileInfo) Name() string { return path.Base(f.File.Name) } -func (f fileInfo) Size() int64 { return int64(len(f.File.Data)) } +func (f fileInfo) Name() string { return path.Base(f.f.Name) } +func (f fileInfo) Size() int64 { return int64(len(f.f.Data)) } func (f fileInfo) Mode() fs.FileMode { return f.m } func (f fileInfo) Type() fs.FileMode { return f.m.Type() } func (f fileInfo) ModTime() time.Time { return time.Time{} } func (f fileInfo) IsDir() bool { return f.m.IsDir() } -func (f fileInfo) Sys() interface{} { return f.File } +func (f fileInfo) Sys() interface{} { return f.f } func (f fileInfo) Info() (fs.FileInfo, error) { return f, nil } type openDir struct { @@ -141,7 +144,7 @@ type openDir struct { func (d *openDir) Stat() (fs.FileInfo, error) { return &d.dirInfo, nil } func (d *openDir) Close() error { return nil } func (d *openDir) Read(b []byte) (int, error) { - return 0, &fs.PathError{Op: "read", Path: d.dirInfo.File.Name, Err: errors.New("is a directory")} + return 0, &fs.PathError{Op: "read", Path: d.dirInfo.f.Name, Err: errors.New("is a directory")} } func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { From 3e6aa587f2dde101868728cc0e1ab03e5c11c3a5 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 24 Oct 2023 15:37:55 -0400 Subject: [PATCH 12/17] Test --- txtar/fs_test.go | 177 +++++++++++++++++++++++++---------------------- 1 file changed, 95 insertions(+), 82 deletions(-) diff --git a/txtar/fs_test.go b/txtar/fs_test.go index 44f480898c2..dc0a170a234 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -15,34 +15,33 @@ import ( "testing/iotest" ) -func TestFS(t *testing.T) { - for _, tc := range []struct{ name, input, files string }{ - { - name: "empty", - input: ``, - files: "", - }, - { - name: "one", - input: ` +var fstestcases = []struct{ name, input, files string }{ + { + name: "empty", + input: ``, + files: "", + }, + { + name: "one", + input: ` -- one.txt -- one `, - files: "one.txt", - }, - { - name: "two", - input: ` + files: "one.txt", + }, + { + name: "two", + input: ` -- one.txt -- one -- two.txt -- two `, - files: "one.txt two.txt", - }, - { - name: "subdirectories", - input: ` + files: "one.txt two.txt", + }, + { + name: "subdirectories", + input: ` -- one.txt -- one -- 2/two.txt -- @@ -52,80 +51,94 @@ three -- 4/four.txt -- three `, - files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", - }, - { - name: "unclean file names", - input: ` + files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", + }, + { + name: "unclean file names", + input: ` -- 1/../one.txt -- one -- 2/sub/../two.txt -- two `, - files: "one.txt 2/two.txt", - }, - } { + files: "one.txt 2/two.txt", + }, +} + +func TestFS(t *testing.T) { + for _, tc := range fstestcases { t.Run(tc.name, func(t *testing.T) { - a := Parse([]byte(tc.input)) - files := strings.Fields(tc.files) - if err := fstest.TestFS(a.FS(), files...); err != nil { + testFS(t, tc.name, tc.input, tc.files) + }) + } +} + +func FuzzFS(f *testing.F) { + for _, tc := range fstestcases { + f.Add(tc.name, tc.input, tc.files) + } + f.Fuzz(testFS) +} + +func testFS(t *testing.T, name, input, fileNames string) { + a := Parse([]byte(input)) + files := strings.Fields(fileNames) + if err := fstest.TestFS(a.FS(), files...); err != nil { + t.Fatal(err) + } + for _, name := range files { + for _, f := range a.Files { + if f.Name != name { + continue + } + fsys := a.FS() + b, err := fs.ReadFile(fsys, name) + if err != nil { t.Fatal(err) } - for _, name := range files { - for _, f := range a.Files { - if f.Name != name { - continue - } - fsys := a.FS() - b, err := fs.ReadFile(fsys, name) - if err != nil { - t.Fatal(err) - } - if string(b) != string(f.Data) { - t.Fatalf("mismatched contents for %q", name) - } - // Be careful with n cases, this open is O(n^3) deep - // Do iotest - fsfile, err := fsys.Open(name) - if err != nil { - t.Fatal(err) - } - if err = iotest.TestReader(fsfile, f.Data); err != nil { - t.Fatal(err) - } - if err = fsfile.Close(); err != nil { - t.Fatal(err) - } - // test io.Copy - fsfile, err = fsys.Open(name) - if err != nil { - t.Fatal(err) - } - var buf strings.Builder - n, err := io.Copy(&buf, fsfile) - if err != nil { - t.Fatal(err) - } - if n != int64(len(f.Data)) { - t.Fatalf("bad copy size: %d", n) - } - if buf.String() != string(f.Data) { - t.Fatalf("mismatched contents for io.Copy of %q", name) - } - if err = fsfile.Close(); err != nil { - t.Fatal(err) - } - } + if string(b) != string(f.Data) { + t.Fatalf("mismatched contents for %q", name) } - a2, err := From(a.FS()) + // Be careful with n cases, this open is O(n^3) deep + // Do iotest + fsfile, err := fsys.Open(name) if err != nil { - t.Fatalf("failed to write fsys for %v: %v", tc.name, err) + t.Fatal(err) } - - if in, out := normalized(a), normalized(a2); in != out { - t.Errorf("From round trip failed: %q != %q", in, out) + if err = iotest.TestReader(fsfile, f.Data); err != nil { + t.Fatal(err) } - }) + if err = fsfile.Close(); err != nil { + t.Fatal(err) + } + // test io.Copy + fsfile, err = fsys.Open(name) + if err != nil { + t.Fatal(err) + } + var buf strings.Builder + n, err := io.Copy(&buf, fsfile) + if err != nil { + t.Fatal(err) + } + if n != int64(len(f.Data)) { + t.Fatalf("bad copy size: %d", n) + } + if buf.String() != string(f.Data) { + t.Fatalf("mismatched contents for io.Copy of %q", name) + } + if err = fsfile.Close(); err != nil { + t.Fatal(err) + } + } + } + a2, err := From(a.FS()) + if err != nil { + t.Fatalf("failed to write fsys for %v: %v", name, err) + } + + if in, out := normalized(a), normalized(a2); in != out { + t.Errorf("From round trip failed: %q != %q", in, out) } } From 8a0475c35d45cdeadf2728aae1627ef63dd38960 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 24 Oct 2023 16:25:15 -0400 Subject: [PATCH 13/17] Rework --- txtar/archive.go | 6 -- txtar/fs.go | 180 +++++++++-------------------------------- txtar/fs_test.go | 205 +++++++++++++++++++++++++++-------------------- 3 files changed, 157 insertions(+), 234 deletions(-) diff --git a/txtar/archive.go b/txtar/archive.go index 8f10f16ad62..fd95f1e64a1 100644 --- a/txtar/archive.go +++ b/txtar/archive.go @@ -34,7 +34,6 @@ package txtar import ( "bytes" "fmt" - "io/fs" "os" "strings" ) @@ -139,8 +138,3 @@ func fixNL(data []byte) []byte { d[len(data)] = '\n' return d } - -// FS returns an fs.FS that reads from the Archive. -func (a *Archive) FS() fs.FS { - return archiveFS{a} -} diff --git a/txtar/fs.go b/txtar/fs.go index 2ede4108642..666c3e9b482 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -1,166 +1,64 @@ -// Copyright 2022 The Go Authors. All rights reserved. +// 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 txtar import ( - "bytes" - "errors" - "io" + "fmt" "io/fs" "path" "strings" + "testing/fstest" "time" ) -type archiveFS struct { - a *Archive -} - -// Open implements fs.FS. -func (fsys archiveFS) Open(name string) (fs.File, error) { - if !fs.ValidPath(name) { - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} - } - - for _, f := range fsys.a.Files { - // In case the txtar has weird filenames - cleanName := path.Clean(f.Name) - if name == cleanName { - return newOpenFile(f), nil - } - } - var entries []fileInfo - dirs := make(map[string]bool) - prefix := name + "/" - if name == "." { - prefix = "" - } - - for _, f := range fsys.a.Files { - cleanName := path.Clean(f.Name) - if !strings.HasPrefix(cleanName, prefix) { - continue +// FS returns the file system form of an Archive. +// It returns an error if any of the file names in the archive +// are not valid file system names. +// It also builds an index of the files and their content: +// adding, removing, or renaming files in the archive +// after calling FS will not affect file system method calls. +// However, FS does not copy the underlying file contents: +// change to file content will be visible in file system method calls. +func FS(a *Archive) (fs.FS, error) { + m := make(fstest.MapFS) + for _, f := range a.Files { + if !unixIsLocal(f.Name) { + return nil, fmt.Errorf("txtar.FS: Archive contains invalid path for fs.File: %q", f.Name) } - felem := cleanName[len(prefix):] - i := strings.Index(felem, "/") - if i < 0 { - entries = append(entries, newFileInfo(f)) - } else { - dirs[felem[:i]] = true + m[path.Clean(f.Name)] = &fstest.MapFile{ + Data: f.Data, + Mode: 0o666, + ModTime: time.Time{}, + Sys: f, } } - // If there are no children of the name, - // then the directory is treated as not existing - // unless the directory is "." - if len(entries) == 0 && len(dirs) == 0 && name != "." { - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} - } - - for name := range dirs { - entries = append(entries, newDirInfo(name)) - } - - return &openDir{newDirInfo(name), entries, 0}, nil + return m, nil } -var _ fs.ReadFileFS = archiveFS{} - -// ReadFile implements fs.ReadFileFS. -func (fsys archiveFS) ReadFile(name string) ([]byte, error) { - if !fs.ValidPath(name) { - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} +// copied from filepath.unixIsLocal +// with modification to use path +func unixIsLocal(p string) bool { + if path.IsAbs(p) || p == "" { + return false } - if name == "." { - return nil, &fs.PathError{Op: "read", Path: name, Err: errors.New("is a directory")} - } - prefix := name + "/" - for _, f := range fsys.a.Files { - if cleanName := path.Clean(f.Name); name == cleanName { - return append(([]byte)(nil), f.Data...), nil - } - // It's a directory - if strings.HasPrefix(f.Name, prefix) { - return nil, &fs.PathError{Op: "read", Path: name, Err: errors.New("is a directory")} + hasDots := false + for p := p; p != ""; { + var part string + part, p, _ = strings.Cut(p, "/") + if part == "." || part == ".." { + hasDots = true + break } } - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} -} - -var ( - _ fs.File = (*openFile)(nil) - _ io.ReadSeekCloser = (*openFile)(nil) - _ io.ReaderAt = (*openFile)(nil) - _ io.WriterTo = (*openFile)(nil) -) - -type openFile struct { - bytes.Reader - fi fileInfo -} - -func newOpenFile(f File) *openFile { - var o openFile - o.Reader.Reset(f.Data) - o.fi = fileInfo{f, 0444} - return &o -} - -func (o *openFile) Stat() (fs.FileInfo, error) { return o.fi, nil } - -func (o *openFile) Close() error { return nil } - -var _ fs.FileInfo = fileInfo{} - -type fileInfo struct { - f File - m fs.FileMode -} - -func newFileInfo(f File) fileInfo { - return fileInfo{f, 0444} -} - -func newDirInfo(name string) fileInfo { - return fileInfo{File{Name: name}, fs.ModeDir | 0555} -} - -func (f fileInfo) Name() string { return path.Base(f.f.Name) } -func (f fileInfo) Size() int64 { return int64(len(f.f.Data)) } -func (f fileInfo) Mode() fs.FileMode { return f.m } -func (f fileInfo) Type() fs.FileMode { return f.m.Type() } -func (f fileInfo) ModTime() time.Time { return time.Time{} } -func (f fileInfo) IsDir() bool { return f.m.IsDir() } -func (f fileInfo) Sys() interface{} { return f.f } -func (f fileInfo) Info() (fs.FileInfo, error) { return f, nil } - -type openDir struct { - dirInfo fileInfo - entries []fileInfo - offset int -} - -func (d *openDir) Stat() (fs.FileInfo, error) { return &d.dirInfo, nil } -func (d *openDir) Close() error { return nil } -func (d *openDir) Read(b []byte) (int, error) { - return 0, &fs.PathError{Op: "read", Path: d.dirInfo.f.Name, Err: errors.New("is a directory")} -} - -func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { - n := len(d.entries) - d.offset - if count > 0 && n > count { - n = count - } - if n == 0 && count > 0 { - return nil, io.EOF + if hasDots { + p = path.Clean(p) } - entries := make([]fs.DirEntry, n) - for i := range entries { - entries[i] = &d.entries[d.offset+i] + if p == ".." || strings.HasPrefix(p, "../") { + return false } - d.offset += n - return entries, nil + return true } // From constructs an Archive with the contents of fsys and an empty Comment. diff --git a/txtar/fs_test.go b/txtar/fs_test.go index dc0a170a234..eda4952535c 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -1,4 +1,4 @@ -// Copyright 2022 The Go Authors. All rights reserved. +// 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. @@ -15,33 +15,37 @@ import ( "testing/iotest" ) -var fstestcases = []struct{ name, input, files string }{ - { - name: "empty", - input: ``, - files: "", - }, - { - name: "one", - input: ` +func TestFS(t *testing.T) { + var fstestcases = []struct { + name, input, files string + invalidNames, invalidRoundtrip bool + }{ + { + name: "empty", + input: ``, + files: "", + }, + { + name: "one", + input: ` -- one.txt -- one `, - files: "one.txt", - }, - { - name: "two", - input: ` + files: "one.txt", + }, + { + name: "two", + input: ` -- one.txt -- one -- two.txt -- two `, - files: "one.txt two.txt", - }, - { - name: "subdirectories", - input: ` + files: "one.txt two.txt", + }, + { + name: "subdirectories", + input: ` -- one.txt -- one -- 2/two.txt -- @@ -51,94 +55,121 @@ three -- 4/four.txt -- three `, - files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", - }, - { - name: "unclean file names", - input: ` + files: "one.txt 2/two.txt 2/3/three.txt 4/four.txt", + }, + { + name: "unclean file names", + input: ` -- 1/../one.txt -- one -- 2/sub/../two.txt -- two `, - files: "one.txt 2/two.txt", - }, -} - -func TestFS(t *testing.T) { - for _, tc := range fstestcases { - t.Run(tc.name, func(t *testing.T) { - testFS(t, tc.name, tc.input, tc.files) - }) + files: "one.txt 2/two.txt", + }, + { + name: "overlapping names", + input: ` +-- 1/../one.txt -- +one +-- 2/../one.txt -- +two +`, + files: "one.txt", + invalidRoundtrip: true, + }, + { + name: "invalid names", + input: ` +-- ../one.txt -- +one +-- ../one.txt -- +two +`, + invalidNames: true, + }, } -} -func FuzzFS(f *testing.F) { for _, tc := range fstestcases { - f.Add(tc.name, tc.input, tc.files) - } - f.Fuzz(testFS) -} - -func testFS(t *testing.T, name, input, fileNames string) { - a := Parse([]byte(input)) - files := strings.Fields(fileNames) - if err := fstest.TestFS(a.FS(), files...); err != nil { - t.Fatal(err) - } - for _, name := range files { - for _, f := range a.Files { - if f.Name != name { - continue - } - fsys := a.FS() - b, err := fs.ReadFile(fsys, name) - if err != nil { - t.Fatal(err) - } - if string(b) != string(f.Data) { - t.Fatalf("mismatched contents for %q", name) + t.Run(tc.name, func(t *testing.T) { + files := strings.Fields(tc.files) + a := Parse([]byte(tc.input)) + fsys, err := FS(a) + if tc.invalidNames { + if err == nil { + t.Fatal("expected error: got nil") + } + return } - // Be careful with n cases, this open is O(n^3) deep - // Do iotest - fsfile, err := fsys.Open(name) if err != nil { t.Fatal(err) } - if err = iotest.TestReader(fsfile, f.Data); err != nil { + if err := fstest.TestFS(fsys, files...); err != nil { t.Fatal(err) } - if err = fsfile.Close(); err != nil { - t.Fatal(err) + for _, name := range files { + for _, f := range a.Files { + if f.Name != name { + continue + } + fsys, err := FS(a) + if err != nil { + continue + } + b, err := fs.ReadFile(fsys, name) + if err != nil { + t.Fatal(err) + } + if string(b) != string(f.Data) { + t.Fatalf("mismatched contents for %q", name) + } + // Do iotest + fsfile, err := fsys.Open(name) + if err != nil { + t.Fatal(err) + } + if err = iotest.TestReader(fsfile, f.Data); err != nil { + t.Fatal(err) + } + if err = fsfile.Close(); err != nil { + t.Fatal(err) + } + // test io.Copy + fsfile, err = fsys.Open(name) + if err != nil { + t.Fatal(err) + } + var buf strings.Builder + n, err := io.Copy(&buf, fsfile) + if err != nil { + t.Fatal(err) + } + if n != int64(len(f.Data)) { + t.Fatalf("bad copy size: %d", n) + } + if buf.String() != string(f.Data) { + t.Fatalf("mismatched contents for io.Copy of %q", name) + } + if err = fsfile.Close(); err != nil { + t.Fatal(err) + } + } } - // test io.Copy - fsfile, err = fsys.Open(name) + fsys2, err := FS(a) if err != nil { t.Fatal(err) } - var buf strings.Builder - n, err := io.Copy(&buf, fsfile) + a2, err := From(fsys2) if err != nil { - t.Fatal(err) + t.Fatalf("failed to write fsys for %v: %v", tc.name, err) } - if n != int64(len(f.Data)) { - t.Fatalf("bad copy size: %d", n) - } - if buf.String() != string(f.Data) { - t.Fatalf("mismatched contents for io.Copy of %q", name) - } - if err = fsfile.Close(); err != nil { - t.Fatal(err) - } - } - } - a2, err := From(a.FS()) - if err != nil { - t.Fatalf("failed to write fsys for %v: %v", name, err) - } - if in, out := normalized(a), normalized(a2); in != out { - t.Errorf("From round trip failed: %q != %q", in, out) + in, out := normalized(a), normalized(a2) + roundTrips := in == out + if roundTrips == tc.invalidRoundtrip { + t.Errorf("Got round trip %v: want %v", roundTrips, !tc.invalidRoundtrip) + } + }) } } From bea681af8bcd3f7235c841eed96a53e1b3efde8c Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Tue, 24 Oct 2023 16:27:47 -0400 Subject: [PATCH 14/17] simpler --- txtar/fs_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/txtar/fs_test.go b/txtar/fs_test.go index eda4952535c..e1d003a3732 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -79,12 +79,10 @@ two invalidRoundtrip: true, }, { - name: "invalid names", + name: "invalid name", input: ` -- ../one.txt -- one --- ../one.txt -- -two `, invalidNames: true, }, From 985433fda4909f5913e8ac40bc74a57303ac7e4b Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Wed, 25 Oct 2023 08:32:44 -0400 Subject: [PATCH 15/17] Use fs.ValidPath --- txtar/fs.go | 32 +++----------------------------- txtar/fs_test.go | 16 +++++++--------- 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/txtar/fs.go b/txtar/fs.go index 666c3e9b482..e239ec1ad19 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -7,8 +7,6 @@ package txtar import ( "fmt" "io/fs" - "path" - "strings" "testing/fstest" "time" ) @@ -24,10 +22,10 @@ import ( func FS(a *Archive) (fs.FS, error) { m := make(fstest.MapFS) for _, f := range a.Files { - if !unixIsLocal(f.Name) { - return nil, fmt.Errorf("txtar.FS: Archive contains invalid path for fs.File: %q", f.Name) + if !fs.ValidPath(f.Name) { + return nil, fmt.Errorf("txtar.FS: Archive contains invalid fs.FS path: %q", f.Name) } - m[path.Clean(f.Name)] = &fstest.MapFile{ + m[f.Name] = &fstest.MapFile{ Data: f.Data, Mode: 0o666, ModTime: time.Time{}, @@ -37,30 +35,6 @@ func FS(a *Archive) (fs.FS, error) { return m, nil } -// copied from filepath.unixIsLocal -// with modification to use path -func unixIsLocal(p string) bool { - if path.IsAbs(p) || p == "" { - return false - } - hasDots := false - for p := p; p != ""; { - var part string - part, p, _ = strings.Cut(p, "/") - if part == "." || part == ".." { - hasDots = true - break - } - } - if hasDots { - p = path.Clean(p) - } - if p == ".." || strings.HasPrefix(p, "../") { - return false - } - return true -} - // From constructs an Archive with the contents of fsys and an empty Comment. // Subsequent changes to fsys are not reflected in the returned archive. // diff --git a/txtar/fs_test.go b/txtar/fs_test.go index e1d003a3732..1b650e76597 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -17,8 +17,8 @@ import ( func TestFS(t *testing.T) { var fstestcases = []struct { - name, input, files string - invalidNames, invalidRoundtrip bool + name, input, files string + invalidNames bool }{ { name: "empty", @@ -65,7 +65,7 @@ one -- 2/sub/../two.txt -- two `, - files: "one.txt 2/two.txt", + invalidNames: true, }, { name: "overlapping names", @@ -75,8 +75,8 @@ one -- 2/../one.txt -- two `, - files: "one.txt", - invalidRoundtrip: true, + files: "one.txt", + invalidNames: true, }, { name: "invalid name", @@ -162,10 +162,8 @@ one t.Fatalf("failed to write fsys for %v: %v", tc.name, err) } - in, out := normalized(a), normalized(a2) - roundTrips := in == out - if roundTrips == tc.invalidRoundtrip { - t.Errorf("Got round trip %v: want %v", roundTrips, !tc.invalidRoundtrip) + if in, out := normalized(a), normalized(a2); in != out { + t.Error("did not round trip") } }) } From b3bedca0e29aa364d1baac505036cdd5ceea7944 Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Wed, 25 Oct 2023 10:03:32 -0400 Subject: [PATCH 16/17] simplify tests --- txtar/fs_test.go | 64 ++++++++---------------------------------------- 1 file changed, 10 insertions(+), 54 deletions(-) diff --git a/txtar/fs_test.go b/txtar/fs_test.go index 1b650e76597..4787ae2a17e 100644 --- a/txtar/fs_test.go +++ b/txtar/fs_test.go @@ -5,14 +5,12 @@ package txtar import ( - "io" "io/fs" "path" "sort" "strings" "testing" "testing/fstest" - "testing/iotest" ) func TestFS(t *testing.T) { @@ -105,61 +103,19 @@ one if err := fstest.TestFS(fsys, files...); err != nil { t.Fatal(err) } - for _, name := range files { - for _, f := range a.Files { - if f.Name != name { - continue - } - fsys, err := FS(a) - if err != nil { - continue - } - b, err := fs.ReadFile(fsys, name) - if err != nil { - t.Fatal(err) - } - if string(b) != string(f.Data) { - t.Fatalf("mismatched contents for %q", name) - } - // Do iotest - fsfile, err := fsys.Open(name) - if err != nil { - t.Fatal(err) - } - if err = iotest.TestReader(fsfile, f.Data); err != nil { - t.Fatal(err) - } - if err = fsfile.Close(); err != nil { - t.Fatal(err) - } - // test io.Copy - fsfile, err = fsys.Open(name) - if err != nil { - t.Fatal(err) - } - var buf strings.Builder - n, err := io.Copy(&buf, fsfile) - if err != nil { - t.Fatal(err) - } - if n != int64(len(f.Data)) { - t.Fatalf("bad copy size: %d", n) - } - if buf.String() != string(f.Data) { - t.Fatalf("mismatched contents for io.Copy of %q", name) - } - if err = fsfile.Close(); err != nil { - t.Fatal(err) - } + + for _, f := range a.Files { + b, err := fs.ReadFile(fsys, f.Name) + if err != nil { + t.Fatalf("could not read %s from fsys: %v", f.Name, err) + } + if string(b) != string(f.Data) { + t.Fatalf("mismatched contents for %q", f.Name) } } - fsys2, err := FS(a) - if err != nil { - t.Fatal(err) - } - a2, err := From(fsys2) + a2, err := From(fsys) if err != nil { - t.Fatalf("failed to write fsys for %v: %v", tc.name, err) + t.Fatalf("error building Archive from fsys: %v", err) } if in, out := normalized(a), normalized(a2); in != out { From d470de7e4c293265c46de5823f5d316bcc95946f Mon Sep 17 00:00:00 2001 From: Carl Johnson Date: Thu, 26 Oct 2023 20:24:43 -0400 Subject: [PATCH 17/17] preallocate --- txtar/fs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/txtar/fs.go b/txtar/fs.go index e239ec1ad19..ac08988dd96 100644 --- a/txtar/fs.go +++ b/txtar/fs.go @@ -20,7 +20,7 @@ import ( // However, FS does not copy the underlying file contents: // change to file content will be visible in file system method calls. func FS(a *Archive) (fs.FS, error) { - m := make(fstest.MapFS) + m := make(fstest.MapFS, len(a.Files)) for _, f := range a.Files { if !fs.ValidPath(f.Name) { return nil, fmt.Errorf("txtar.FS: Archive contains invalid fs.FS path: %q", f.Name)