diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index daa98bdbc..2fb03af91 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -42,18 +42,20 @@ jobs: windows-build: runs-on: windows-latest env: - TMP: D:\a\tmp + # note: the tmp dir is set to C: so that it's not on the same drive as the + # repo, which is on D: - this will expose bugs with path handling! + TMP: C:\tmp steps: - run: pwd + - uses: actions/checkout@v4 + with: + fetch-depth: 0 - uses: actions/setup-go@v4 with: - go-version: '1.21' + go-version: '1.21.4' - run: | git config --global user.email "bogus@example.com" git config --global user.name "Someone" - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - run: make build - name: Save binary uses: actions/upload-artifact@v3 diff --git a/data/datasource_git.go b/data/datasource_git.go index 02df14be6..3859d3443 100644 --- a/data/datasource_git.go +++ b/data/datasource_git.go @@ -241,7 +241,7 @@ func (g gitsource) clone(ctx context.Context, repoURL *url.URL, depth int) (bill return g.clone(ctx, u, depth) } if err != nil { - return nil, nil, fmt.Errorf("git clone for %v failed: %w", repoURL, err) + return nil, nil, fmt.Errorf("git clone %s: %w", u, err) } return fs, repo, nil } diff --git a/internal/datafs/fsys.go b/internal/datafs/fsys.go index ac4c22c5e..fb88b0da1 100644 --- a/internal/datafs/fsys.go +++ b/internal/datafs/fsys.go @@ -7,6 +7,7 @@ import ( "net/url" "path" "path/filepath" + "strings" "github.com/hairyhenderson/go-fsimpl" ) @@ -81,7 +82,21 @@ func FSysForPath(ctx context.Context, path string) (fs.FS, error) { return nil, fmt.Errorf("no filesystem provider in context") } - fsys, err := fsp.New(&url.URL{Scheme: u.Scheme, Path: "/"}) + // default to "/" so we have a rooted filesystem for all schemes, but also + // support volumes on Windows + origPath := u.Path + if u.Scheme == "file" || strings.HasSuffix(u.Scheme, "+file") || u.Scheme == "" { + u.Path, _, err = ResolveLocalPath(origPath) + if err != nil { + return nil, fmt.Errorf("resolve local path %q: %w", origPath, err) + } + // if this is a drive letter, add a trailing slash + if u.Path[0] != '/' { + u.Path += "/" + } + } + + fsys, err := fsp.New(u) if err != nil { return nil, fmt.Errorf("filesystem provider for %q unavailable: %w", path, err) } diff --git a/internal/datafs/wdfs.go b/internal/datafs/wdfs.go index 79c2b0efe..e696e7d1b 100644 --- a/internal/datafs/wdfs.go +++ b/internal/datafs/wdfs.go @@ -6,6 +6,7 @@ import ( "net/url" "os" "path/filepath" + "strings" "github.com/hack-pad/hackpadfs" osfs "github.com/hack-pad/hackpadfs/os" @@ -20,15 +21,15 @@ import ( // // TODO: maybe take fsys as an argument, and if it's a wdFS, use its vol instead // of calling os.Getwd? -func ResolveLocalPath(name string) (root, resolved string) { +func ResolveLocalPath(name string) (root, resolved string, err error) { // ignore empty names if len(name) == 0 { - return "", "" + return "", "", nil } wd, err := os.Getwd() if err != nil { - panic(err) + return "", "", fmt.Errorf("getwd: %w", err) } vol := filepath.VolumeName(wd) @@ -40,10 +41,10 @@ func ResolveLocalPath(name string) (root, resolved string) { return f.resolveLocalPath(name) } -func (w *wdFS) resolveLocalPath(name string) (root, resolved string) { +func (w *wdFS) resolveLocalPath(name string) (root, resolved string, err error) { // ignore empty names if len(name) == 0 { - return "", "" + return "", "", nil } // we want to assume a / separator regardless of the OS @@ -51,17 +52,22 @@ func (w *wdFS) resolveLocalPath(name string) (root, resolved string) { // special-case for (Windows) paths that start with '/' but have no volume // name (e.g. "/foo/bar"). UNC paths (beginning with "//") are ignored. - if name[0] == '/' && (len(name) == 1 || name[1] != '/') { + if name[0] == '/' && (len(name) == 1 || (name[1] != '/' && name[1] != '?')) { name = filepath.Join(w.vol, name) + // TODO: maybe this can be reduced to just '!filepath.IsAbs(name)'? } else if name[0] != '/' && !filepath.IsAbs(name) { - wd, err := os.Getwd() + abs := "" + abs, err = filepath.Abs(name) if err != nil { - panic(err) + return "", "", fmt.Errorf("abs %q: %w", name, err) } - name = filepath.Join(wd, name) + name = abs } - name = filepath.ToSlash(name) + name, err = normalizeWindowsPath(name) + if err != nil { + return "", "", fmt.Errorf("normalize %q: %w", name, err) + } vol := filepath.VolumeName(name) if vol != "" && name != vol { @@ -80,15 +86,124 @@ func (w *wdFS) resolveLocalPath(name string) (root, resolved string) { name = "." } - return root, name + return root, name, nil +} + +// normalizeWindowsPath - converts the various types of Windows paths to either +// a rooted or relative path, depending on the type of path. +func normalizeWindowsPath(name string) (string, error) { + name = strings.ReplaceAll(name, `\`, "/") + + switch win32PathType(name) { + case winPathUnknown, winPathRootLocalDevice, winPathDriveRelative, winPathNT: + return "", fmt.Errorf("unsupported path %q: %w", name, fs.ErrInvalid) + case winPathDriveAbsolute, winPathRelative, winPathRooted: + // absolute/relative returned as-is + return name, nil + case winPathUncAbsolute: + // UNC paths are returned as-is + return name, nil + case winPathLocalDevice: + // local device paths have the prefix stripped + return name[4:], nil + default: + return "", fmt.Errorf("unknown path type %q: %w", name, fs.ErrInvalid) + } +} + +type winPathtype int + +// There are 8 types of "DOS" paths in Windows (as opposed to NT paths): +// +// NT paths begin with a "\??\" prefix, and are implicitly absolute. +const ( + // - Unknown - e.g. "" or some other invalid path + winPathUnknown winPathtype = iota + // - Drive Absolute - e.g. C:\foo\bar + winPathDriveAbsolute + // - Drive Relative - e.g. C:foo\bar + winPathDriveRelative + // - Rooted - e.g. \foo\bar + winPathRooted + // - Relative - e.g. foo\bar, .\foo\bar, ..\foo\bar + winPathRelative + // - UNC Absolute - e.g. \\foo\bar + winPathUncAbsolute + // - Local Device - e.g. \\.\C:\foo\bar, \\.\COM1, \\?\C:\foo\bar + winPathLocalDevice + // - Root Local Device - e.g. \\. or \\? + winPathRootLocalDevice + // - NT path - e.g. \??\C:\foo\bar or \??\UNC\foo\bar + winPathNT +) + +// win32PathType - returns the type of path, as defined by the win32Path enum +// See https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html +// for details on the different types +func win32PathType(name string) winPathtype { + if name == "" { + return winPathUnknown + } + + // not using filepath.ToSlash here, because we want to be able to test this + // on non-Windows systems too + name = strings.ReplaceAll(name, `\`, "/") + + // if the first character is a slash, it's either rooted, a UNC, a local device, or root local device path + if name[0] == '/' { + switch { + case len(name) == 1 || (name[1] != '/' && name[1] != '?'): + return winPathRooted + case len(name) == 2 || (name[2] != '.' && name[2] != '?'): + return winPathUncAbsolute + case len(name) >= 4 && name[1:4] == "??/": + return winPathNT + case len(name) >= 4 && name[3] == '/': + return winPathLocalDevice + default: + return winPathRootLocalDevice + } + } + + switch { + case len(name) == 1 || name[1] != ':': + return winPathRelative + case len(name) == 2 || name[2] != '/': + return winPathDriveRelative + default: + return winPathDriveAbsolute + } +} + +func isSupportedPath(name string) bool { + switch win32PathType(name) { + case winPathUnknown, winPathRootLocalDevice, winPathDriveRelative, winPathNT: + return false + default: + return true + } } // WdFS is a filesystem provider that creates local filesystems which support // absolute paths beginning with '/', and interpret relative paths as relative -// to the current working directory (as reported by [os.Getwd]) +// to the current working directory (as reported by [os.Getwd]). +// +// On Windows, certain types of paths are not supported, and will return an +// error. These are: +// - Drive Relative - e.g. C:foo\bar +// - Root Local - e.g. \\. or \\? +// - non-drive Local Devices - e.g. \\.\COM1, \\.\pipe\foo +// - NT Paths - e.g. \??\C:\foo\bar or \??\UNC\foo\bar var WdFS = fsimpl.FSProviderFunc( func(u *url.URL) (fs.FS, error) { - vol, _ := ResolveLocalPath(u.Path) + if !isSupportedPath(u.Path) { + return nil, fmt.Errorf("unsupported path %q: %w", u.Path, fs.ErrInvalid) + } + + vol, _, err := ResolveLocalPath(u.Path) + if err != nil { + return nil, fmt.Errorf("resolve %q: %w", u.Path, err) + } var fsys fs.FS if vol == "" || vol == "/" { @@ -165,7 +280,10 @@ func (w *wdFS) fsysFor(vol string) (fs.FS, error) { } func (w *wdFS) Open(name string) (fs.File, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -174,7 +292,10 @@ func (w *wdFS) Open(name string) (fs.File, error) { } func (w *wdFS) Stat(name string) (fs.FileInfo, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -183,7 +304,10 @@ func (w *wdFS) Stat(name string) (fs.FileInfo, error) { } func (w *wdFS) ReadFile(name string) ([]byte, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -192,7 +316,10 @@ func (w *wdFS) ReadFile(name string) ([]byte, error) { } func (w *wdFS) ReadDir(name string) ([]fs.DirEntry, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -217,7 +344,10 @@ func (w *wdFS) Glob(_ string) ([]string, error) { } func (w *wdFS) Create(name string) (fs.File, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -226,7 +356,10 @@ func (w *wdFS) Create(name string) (fs.File, error) { } func (w *wdFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return nil, fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return nil, err @@ -235,7 +368,10 @@ func (w *wdFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error } func (w *wdFS) Mkdir(name string, perm fs.FileMode) error { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return err @@ -248,7 +384,10 @@ func (w *wdFS) Mkdir(name string, perm fs.FileMode) error { } func (w *wdFS) MkdirAll(name string, perm fs.FileMode) error { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return err @@ -257,7 +396,10 @@ func (w *wdFS) MkdirAll(name string, perm fs.FileMode) error { } func (w *wdFS) Remove(name string) error { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return err @@ -266,7 +408,10 @@ func (w *wdFS) Remove(name string) error { } func (w *wdFS) Chmod(name string, mode fs.FileMode) error { - root, resolved := w.resolveLocalPath(name) + root, resolved, err := w.resolveLocalPath(name) + if err != nil { + return fmt.Errorf("resolve: %w", err) + } fsys, err := w.fsysFor(root) if err != nil { return err diff --git a/internal/datafs/wdfs_test.go b/internal/datafs/wdfs_test.go index 610920af8..768ec386e 100644 --- a/internal/datafs/wdfs_test.go +++ b/internal/datafs/wdfs_test.go @@ -170,12 +170,17 @@ func TestResolveLocalPath_NonWindows(t *testing.T) { {"tmp/foo", wd + "/tmp/foo"}, {"./tmp/foo", wd + "/tmp/foo"}, {"tmp/../foo", wd + "/foo"}, + {"/", "."}, } for _, td := range testdata { - root, path := ResolveLocalPath(td.path) - assert.Equal(t, "/", root) - assert.Equal(t, td.expected, path) + td := td + t.Run(td.path, func(t *testing.T) { + root, path, err := ResolveLocalPath(td.path) + require.NoError(t, err) + assert.Equal(t, "/", root) + assert.Equal(t, td.expected, path) + }) } } @@ -203,7 +208,8 @@ func TestResolveLocalPath_Windows(t *testing.T) { for _, td := range testdata { td := td t.Run(td.path, func(t *testing.T) { - root, path := ResolveLocalPath(td.path) + root, path, err := ResolveLocalPath(td.path) + require.NoError(t, err) assert.Equal(t, td.expRoot, root) assert.Equal(t, td.expected, path) }) @@ -224,12 +230,14 @@ func TestWdFS_ResolveLocalPath_NonWindows(t *testing.T) { {"tmp/foo", wd + "/tmp/foo"}, {"./tmp/foo", wd + "/tmp/foo"}, {"tmp/../foo", wd + "/foo"}, + {"/", "."}, } fsys := &wdFS{} for _, td := range testdata { - root, path := fsys.resolveLocalPath(td.path) + root, path, err := fsys.resolveLocalPath(td.path) + require.NoError(t, err) assert.Equal(t, "/", root) assert.Equal(t, td.expected, path) } @@ -249,14 +257,14 @@ func TestWdFS_ResolveLocalPath_Windows(t *testing.T) { expected string }{ {"C:/tmp/foo", "C:", "tmp/foo"}, - {"D:\\tmp\\foo", "D:", "tmp/foo"}, + {`D:\tmp\foo`, "D:", "tmp/foo"}, {"/tmp/foo", volname, "tmp/foo"}, {"tmp/foo", volname, wd + "/tmp/foo"}, {"./tmp/foo", volname, wd + "/tmp/foo"}, {"tmp/../foo", volname, wd + "/foo"}, - {`\\?\C:\tmp\foo`, "//?/C:", "tmp/foo"}, + {`\\?\C:\tmp\foo`, "C:", "tmp/foo"}, {`\\somehost\share\foo\bar`, "//somehost/share", "foo/bar"}, - {`//?/C:/tmp/foo`, "//?/C:", "tmp/foo"}, + {`//?/C:/tmp/foo`, "C:", "tmp/foo"}, {`//somehost/share/foo/bar`, "//somehost/share", "foo/bar"}, } @@ -265,9 +273,45 @@ func TestWdFS_ResolveLocalPath_Windows(t *testing.T) { for _, td := range testdata { td := td t.Run(td.path, func(t *testing.T) { - root, path := fsys.resolveLocalPath(td.path) + root, path, err := fsys.resolveLocalPath(td.path) + require.NoError(t, err) assert.Equal(t, td.expRoot, root) assert.Equal(t, td.expected, path) }) } } + +func TestWin32PathType(t *testing.T) { + testdata := []struct { + path string + expected winPathtype + }{ + {"", winPathUnknown}, + {`\`, winPathRooted}, + {`\\`, winPathUncAbsolute}, + {`x`, winPathRelative}, + {`x:`, winPathDriveRelative}, + {"C:/tmp/foo", winPathDriveAbsolute}, + {`D:\tmp\foo`, winPathDriveAbsolute}, + {"/tmp/foo", winPathRooted}, + {"tmp/foo", winPathRelative}, + {"./tmp/foo", winPathRelative}, + {"tmp/../foo", winPathRelative}, + {`\\?\C:\tmp\foo`, winPathLocalDevice}, + {`\\somehost\share\foo\bar`, winPathUncAbsolute}, + {`//./C:/tmp/foo`, winPathLocalDevice}, + {`//./pipe/foo`, winPathLocalDevice}, + {`//./COM2`, winPathLocalDevice}, + {`\\.`, winPathRootLocalDevice}, + {`//?`, winPathRootLocalDevice}, + {`/??/C:/`, winPathNT}, + {`/??/UNC/server/foo`, winPathNT}, + } + + for _, td := range testdata { + td := td + t.Run(td.path, func(t *testing.T) { + assert.Equal(t, td.expected, win32PathType(td.path)) + }) + } +} diff --git a/internal/tests/integration/datasources_git_test.go b/internal/tests/integration/datasources_git_test.go index 236e35f94..fbc8e85dc 100644 --- a/internal/tests/integration/datasources_git_test.go +++ b/internal/tests/integration/datasources_git_test.go @@ -33,8 +33,8 @@ func setupDatasourcesGitTest(t *testing.T) *fs.Dir { repoPath := tmpDir.Join("repo") - icmd.RunCommand("git", "init", repoPath). - Assert(t, icmd.Expected{Out: "Initialized empty Git repository"}) + icmd.RunCmd(icmd.Command("git", "init", repoPath), icmd.Dir(repoPath)). + Assert(t, icmd.Expected{Out: "Initialized empty Git repository in "}) icmd.RunCmd(icmd.Command("git", "add", "config.json"), icmd.Dir(repoPath)).Assert(t, icmd.Expected{}) icmd.RunCmd(icmd.Command("git", "add", "jsonfile"), icmd.Dir(repoPath)).Assert(t, icmd.Expected{}) icmd.RunCmd(icmd.Command("git", "add", "subdir"), icmd.Dir(repoPath)).Assert(t, icmd.Expected{}) diff --git a/template.go b/template.go index 7aad0694f..66fc4e54a 100644 --- a/template.go +++ b/template.go @@ -90,7 +90,12 @@ func parseNestedTemplates(ctx context.Context, nested config.Templates, tmpl *te } // TODO: maybe need to do something with root here? - if _, reldir := datafs.ResolveLocalPath(u.Path); reldir != "" && reldir != "." { + _, reldir, err := datafs.ResolveLocalPath(u.Path) + if err != nil { + return fmt.Errorf("resolveLocalPath: %w", err) + } + + if reldir != "" && reldir != "." { fsys, err = fs.Sub(fsys, reldir) if err != nil { return fmt.Errorf("sub filesystem for %q unavailable: %w", &u, err) @@ -220,7 +225,10 @@ func walkDir(ctx context.Context, cfg *config.Config, dir string, outFileNamer f // we need dir to be relative to the root of fsys // TODO: maybe need to do something with root here? - _, reldir := datafs.ResolveLocalPath(dir) + _, reldir, err := datafs.ResolveLocalPath(dir) + if err != nil { + return nil, fmt.Errorf("resolveLocalPath: %w", err) + } subfsys, err := fs.Sub(fsys, reldir) if err != nil {