lib/file: improve error message when attempting to create dir on nonexistent drive on windows

This replaces built-in os.MkdirAll with a patched version that stops the recursion
when reaching the volume part of the path. The original version would continue recursion,
and for extended length paths end up with \\? as the top-level directory, and the error
message would then be something like:
mkdir \\?: The filename, directory name, or volume label syntax is incorrect.
This commit is contained in:
albertony 2021-06-11 00:46:36 +02:00
parent b30731c9d0
commit fbc7f2e61b
21 changed files with 261 additions and 27 deletions

View File

@ -569,9 +569,8 @@ func (f *Fs) PutStream(ctx context.Context, in io.Reader, src fs.ObjectInfo, opt
// Mkdir creates the directory if it doesn't exist
func (f *Fs) Mkdir(ctx context.Context, dir string) error {
// FIXME: https://github.com/syncthing/syncthing/blob/master/lib/osutil/mkdirall_windows.go
localPath := f.localPath(dir)
err := os.MkdirAll(localPath, 0777)
err := file.MkdirAll(localPath, 0777)
if err != nil {
return err
}
@ -765,7 +764,7 @@ func (f *Fs) DirMove(ctx context.Context, src fs.Fs, srcRemote, dstRemote string
// Create parent of destination
dstParentPath := filepath.Dir(dstPath)
err = os.MkdirAll(dstParentPath, 0777)
err = file.MkdirAll(dstParentPath, 0777)
if err != nil {
return err
}
@ -1099,7 +1098,7 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read
// mkdirAll makes all the directories needed to store the object
func (o *Object) mkdirAll() error {
dir := filepath.Dir(o.path)
return os.MkdirAll(dir, 0777)
return file.MkdirAll(dir, 0777)
}
type nopWriterCloser struct {

View File

@ -78,6 +78,7 @@ recently very efficiently like this:
**Note**: Use the |--dry-run| or the |--interactive|/|-i| flag to test without copying anything.
`, "|", "`"),
Run: func(command *cobra.Command, args []string) {
cmd.CheckArgs(2, 2, command, args)
fsrc, srcFileName, fdst := cmd.NewFsSrcFileDst(args)
cmd.Run(true, true, command, func() error {

View File

@ -13,6 +13,7 @@ import (
"time"
"github.com/rclone/rclone/cmd"
"github.com/rclone/rclone/lib/file"
"github.com/spf13/cobra"
"github.com/spf13/cobra/doc"
"github.com/spf13/pflag"
@ -55,7 +56,7 @@ rclone.org website.`,
// Create the directory structure
root := args[0]
out := filepath.Join(root, "commands")
err := os.MkdirAll(out, 0777)
err := file.MkdirAll(out, 0777)
if err != nil {
return err
}

View File

@ -16,6 +16,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fstest/testy"
"github.com/rclone/rclone/lib/file"
"github.com/rclone/rclone/lib/random"
"github.com/stretchr/testify/assert"
)
@ -52,7 +53,7 @@ func makeTestDir() (testDir string, err error) {
for attempt := 0; attempt < maxAttempts; attempt++ {
testDir = testDirBase + random.String(4)
err = os.MkdirAll(testDir, os.ModePerm)
err = file.MkdirAll(testDir, os.ModePerm)
if err == nil {
break
}

View File

@ -20,6 +20,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/fstest"
"github.com/rclone/rclone/lib/file"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@ -36,7 +37,7 @@ func initialise(ctx context.Context, t *testing.T) (string, fs.Fs) {
// Make test cache directory
testDir, err := fstest.LocalRemote()
require.NoError(t, err)
err = os.MkdirAll(testDir, 0755)
err = file.MkdirAll(testDir, 0755)
require.NoError(t, err)
// Make test file system
@ -350,7 +351,7 @@ func testMountAPI(t *testing.T, sockAddr string) {
// Run test sequence
path1 := filepath.Join(testDir, "path1")
require.NoError(t, os.MkdirAll(path1, 0755))
require.NoError(t, file.MkdirAll(path1, 0755))
mount1 := filepath.Join(testDir, "vol1")
res := ""

View File

@ -19,6 +19,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/lib/atexit"
"github.com/rclone/rclone/lib/file"
"github.com/rclone/rclone/vfs/vfscommon"
"github.com/rclone/rclone/vfs/vfsflags"
)
@ -44,12 +45,12 @@ func NewDriver(ctx context.Context, root string, mntOpt *mountlib.Options, vfsOp
if err != nil {
return nil, errors.Wrap(err, "failed to make --cache-dir absolute")
}
err = os.MkdirAll(cacheDir, 0700)
err = file.MkdirAll(cacheDir, 0700)
if err != nil {
return nil, errors.Wrapf(err, "failed to create cache directory: %s", cacheDir)
}
//err = os.MkdirAll(root, 0755)
//err = file.MkdirAll(root, 0755)
if err != nil {
return nil, errors.Wrapf(err, "failed to create mount root: %s", root)
}

View File

@ -13,6 +13,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/lib/atexit"
"github.com/rclone/rclone/lib/file"
)
// Server connects plugin with docker daemon by protocol
@ -87,7 +88,7 @@ func writeSpecFile(addr, proto, specDir string) (string, error) {
if specDir == "" {
specDir = defSpecDir
}
if err := os.MkdirAll(specDir, 0755); err != nil {
if err := file.MkdirAll(specDir, 0755); err != nil {
return "", err
}
specFile := filepath.Join(specDir, "rclone.spec")

View File

@ -8,6 +8,8 @@ import (
"net"
"os"
"path/filepath"
"github.com/rclone/rclone/lib/file"
)
func newUnixListener(path string, gid int) (net.Listener, string, error) {
@ -31,7 +33,7 @@ func newUnixListener(path string, gid int) (net.Listener, string, error) {
path = filepath.Join(sockDir, path)
}
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
if err := file.MkdirAll(filepath.Dir(path), 0755); err != nil {
return nil, "", err
}
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {

View File

@ -14,6 +14,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/fs/rc"
"github.com/rclone/rclone/lib/file"
)
// Errors
@ -153,7 +154,7 @@ func (vol *Volume) checkMountpoint() error {
}
_, err := os.Lstat(path)
if os.IsNotExist(err) {
if err = os.MkdirAll(path, 0700); err != nil {
if err = file.MkdirAll(path, 0700); err != nil {
return errors.Wrapf(err, "failed to create mountpoint: %s", path)
}
} else if err != nil {

View File

@ -25,6 +25,7 @@ import (
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/lib/env"
"github.com/rclone/rclone/lib/file"
"github.com/rclone/rclone/vfs"
"github.com/rclone/rclone/vfs/vfsflags"
"golang.org/x/crypto/ssh"
@ -227,7 +228,7 @@ func (s *server) serve() (err error) {
if err != nil && len(s.opt.HostKeys) == 0 {
fs.Debugf(nil, "Failed to load %q: %v", keyPath, err)
// If loading a cached key failed, make the keys and retry
err = os.MkdirAll(cachePath, 0700)
err = file.MkdirAll(cachePath, 0700)
if err != nil {
return errors.Wrap(err, "failed to create cache path")
}

View File

@ -14,6 +14,7 @@ import (
"github.com/rclone/rclone/cmd/test"
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config/flags"
"github.com/rclone/rclone/lib/file"
"github.com/rclone/rclone/lib/random"
"github.com/spf13/cobra"
)
@ -134,7 +135,7 @@ func (d *dir) list(path string, output []string) []string {
// writeFile writes a random file at dir/name
func writeFile(dir, name string) int64 {
err := os.MkdirAll(dir, 0777)
err := file.MkdirAll(dir, 0777)
if err != nil {
log.Fatalf("Failed to make directory %q: %v", dir, err)
}

View File

@ -275,7 +275,7 @@ func makeConfigPath() string {
return configFile
}
var mkdirErr error
if mkdirErr = os.MkdirAll(configDir, os.ModePerm); mkdirErr == nil {
if mkdirErr = file.MkdirAll(configDir, os.ModePerm); mkdirErr == nil {
return configFile
}
// Problem: Try a fallback location. If we did find a home directory then

View File

@ -13,6 +13,7 @@ import (
"github.com/pkg/errors"
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/fs/config"
"github.com/rclone/rclone/lib/file"
)
// Install installs the config file handler
@ -109,7 +110,7 @@ func (s *Storage) Save() error {
}
dir, name := filepath.Split(configPath)
err := os.MkdirAll(dir, os.ModePerm)
err := file.MkdirAll(dir, os.ModePerm)
if err != nil {
return errors.Wrap(err, "failed to create config directory")
}

View File

@ -17,6 +17,7 @@ import (
"github.com/pkg/errors"
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/lib/file"
)
// GetLatestReleaseURL returns the latest release details of the rclone-webui-react
@ -95,7 +96,7 @@ func CheckAndDownloadWebGUIRelease(checkUpdate bool, forceUpdate bool, fetchURL
cachePathExist, cachePathStat, _ := exists(cachePath)
if !cachePathExist {
if err := os.MkdirAll(cachePath, 0755); err != nil {
if err := file.MkdirAll(cachePath, 0755); err != nil {
return errors.New("Error creating cache directory: " + cachePath)
}
}
@ -174,7 +175,7 @@ func Unzip(src, dest string) (err error) {
}
defer fs.CheckClose(r, &err)
if err := os.MkdirAll(dest, 0755); err != nil {
if err := file.MkdirAll(dest, 0755); err != nil {
return err
}
@ -193,14 +194,14 @@ func Unzip(src, dest string) (err error) {
defer fs.CheckClose(rc, &err)
if f.FileInfo().IsDir() {
if err := os.MkdirAll(path, 0755); err != nil {
if err := file.MkdirAll(path, 0755); err != nil {
return err
}
} else {
if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
if err := file.MkdirAll(filepath.Dir(path), 0755); err != nil {
return err
}
f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
f, err := file.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
return err
}
@ -239,7 +240,7 @@ func exists(path string) (existence bool, stat os.FileInfo, err error) {
func CreatePathIfNotExist(path string) (err error) {
exists, stat, _ := exists(path)
if !exists {
if err := os.MkdirAll(path, 0755); err != nil {
if err := file.MkdirAll(path, 0755); err != nil {
return errors.New("Error creating : " + path)
}
}

View File

@ -44,6 +44,7 @@ import (
"github.com/rclone/rclone/fs/hash"
"github.com/rclone/rclone/fs/object"
"github.com/rclone/rclone/fs/walk"
"github.com/rclone/rclone/lib/file"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -212,7 +213,7 @@ func (r *Run) WriteFile(filePath, content string, t time.Time) Item {
// FIXME make directories?
filePath = path.Join(r.LocalName, filePath)
dirPath := path.Dir(filePath)
err := os.MkdirAll(dirPath, 0770)
err := file.MkdirAll(dirPath, 0770)
if err != nil {
r.Fatalf("Failed to make directories %q: %v", dirPath, err)
}

View File

@ -15,6 +15,7 @@ import (
"time"
"github.com/rclone/rclone/fs"
"github.com/rclone/rclone/lib/file"
"github.com/skratchdot/open-golang/open"
)
@ -75,7 +76,7 @@ func NewReport() *Report {
// Create output directory for logs and report
r.LogDir = path.Join(*outputDir, r.DateTime)
err = os.MkdirAll(r.LogDir, 0777)
err = file.MkdirAll(r.LogDir, 0777)
if err != nil {
log.Fatalf("Failed to make log directory: %v", err)
}

11
lib/file/mkdir_other.go Normal file
View File

@ -0,0 +1,11 @@
//go:build !windows
// +build !windows
package file
import "os"
// MkdirAll just calls os.MkdirAll on non-Windows.
func MkdirAll(path string, perm os.FileMode) error {
return os.MkdirAll(path, perm)
}

77
lib/file/mkdir_windows.go Normal file
View File

@ -0,0 +1,77 @@
//go:build windows
// +build windows
package file
import (
"os"
"path/filepath"
"syscall"
)
// MkdirAll creates a directory named path, along with any necessary parents.
//
// Improves os.MkdirAll by avoiding trying to create a folder \\? when the
// volume of a given extended length path does not exist.
//
// Based on source code from golang's os.MkdirAll
// (https://github.com/golang/go/blob/master/src/os/path.go)
func MkdirAll(path string, perm os.FileMode) error {
// Fast path: if we can tell whether path is a directory or file, stop with success or error.
dir, err := os.Stat(path)
if err == nil {
if dir.IsDir() {
return nil
}
return &os.PathError{
Op: "mkdir",
Path: path,
Err: syscall.ENOTDIR,
}
}
// Slow path: make sure parent exists and then call Mkdir for path.
i := len(path)
for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator.
i--
}
if i > 0 {
path = path[:i]
if path == filepath.VolumeName(path) {
// Make reference to a drive's root directory include the trailing slash.
// In extended-length form without trailing slash ("\\?\C:"), os.Stat
// and os.Mkdir both fails. With trailing slash ("\\?\C:\") works,
// and regular paths with or without it ("C:" and "C:\") both works.
path = path + string(os.PathSeparator)
} else {
// See if there is a parent to be created first.
// Not when path refer to a drive's root directory, because we don't
// want to return noninformative error trying to create \\?.
j := i
for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element.
j--
}
if j > 1 {
// Create parent.
err = MkdirAll(path[:j-1], perm)
if err != nil {
return err
}
}
}
}
// Parent now exists; invoke Mkdir and use its result.
err = os.Mkdir(path, perm)
if err != nil {
// Handle arguments like "foo/." by
// double-checking that directory doesn't exist.
dir, err1 := os.Lstat(path)
if err1 == nil && dir.IsDir() {
return nil
}
return err
}
return nil
}

View File

@ -0,0 +1,132 @@
//go:build windows
// +build windows
package file
import (
"fmt"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Basic test from golang's os/path_test.go
func TestMkdirAll(t *testing.T) {
tmpDir, tidy := testDir(t)
defer tidy()
path := tmpDir + "/dir/./dir2"
err := MkdirAll(path, 0777)
if err != nil {
t.Fatalf("MkdirAll %q: %s", path, err)
}
// Already exists, should succeed.
err = MkdirAll(path, 0777)
if err != nil {
t.Fatalf("MkdirAll %q (second time): %s", path, err)
}
// Make file.
fpath := path + "/file"
f, err := Create(fpath)
if err != nil {
t.Fatalf("create %q: %s", fpath, err)
}
defer f.Close()
// Can't make directory named after file.
err = MkdirAll(fpath, 0777)
if err == nil {
t.Fatalf("MkdirAll %q: no error", fpath)
}
perr, ok := err.(*os.PathError)
if !ok {
t.Fatalf("MkdirAll %q returned %T, not *PathError", fpath, err)
}
if filepath.Clean(perr.Path) != filepath.Clean(fpath) {
t.Fatalf("MkdirAll %q returned wrong error path: %q not %q", fpath, filepath.Clean(perr.Path), filepath.Clean(fpath))
}
// Can't make subdirectory of file.
ffpath := fpath + "/subdir"
err = MkdirAll(ffpath, 0777)
if err == nil {
t.Fatalf("MkdirAll %q: no error", ffpath)
}
perr, ok = err.(*os.PathError)
if !ok {
t.Fatalf("MkdirAll %q returned %T, not *PathError", ffpath, err)
}
if filepath.Clean(perr.Path) != filepath.Clean(fpath) {
t.Fatalf("MkdirAll %q returned wrong error path: %q not %q", ffpath, filepath.Clean(perr.Path), filepath.Clean(fpath))
}
path = tmpDir + `\dir\.\dir2\`
err = MkdirAll(path, 0777)
if err != nil {
t.Fatalf("MkdirAll %q: %s", path, err)
}
}
func unusedDrive(t *testing.T) string {
letter := FindUnusedDriveLetter()
require.NotEqual(t, letter, 0)
return string(letter) + ":"
}
func checkMkdirAll(t *testing.T, path string, valid bool, errormsg string) {
if valid {
assert.NoError(t, MkdirAll(path, 0777))
} else {
err := MkdirAll(path, 0777)
assert.Error(t, err)
assert.Equal(t, errormsg, err.Error())
}
}
func checkMkdirAllSubdirs(t *testing.T, path string, valid bool, errormsg string) {
checkMkdirAll(t, path, valid, errormsg)
checkMkdirAll(t, path+`\`, valid, errormsg)
checkMkdirAll(t, path+`\parent`, valid, errormsg)
checkMkdirAll(t, path+`\parent\`, valid, errormsg)
checkMkdirAll(t, path+`\parent\child`, valid, errormsg)
checkMkdirAll(t, path+`\parent\child\`, valid, errormsg)
}
// Testing paths on existing drive
func TestMkdirAllOnDrive(t *testing.T) {
path, tidy := testDir(t)
defer tidy()
dir, err := os.Stat(path)
require.NoError(t, err)
require.True(t, dir.IsDir())
drive := filepath.VolumeName(path)
checkMkdirAll(t, drive, true, "")
checkMkdirAll(t, drive+`\`, true, "")
checkMkdirAll(t, `\\?\`+drive, true, "")
checkMkdirAll(t, `\\?\`+drive+`\`, true, "")
checkMkdirAllSubdirs(t, path, true, "")
checkMkdirAllSubdirs(t, `\\?\`+path, true, "")
}
// Testing paths on unused drive
// This is where there is a difference from golang's os.MkdirAll. It would
// recurse extended-length paths down to the "\\?" prefix and return the
// noninformative error:
// "mkdir \\?: The filename, directory name, or volume label syntax is incorrect."
// Our version stops the recursion at drive's root directory, and reports:
// "mkdir \\?\A:\: The system cannot find the path specified."
func TestMkdirAllOnUnusedDrive(t *testing.T) {
path := unusedDrive(t)
errormsg := fmt.Sprintf("mkdir %s\\: The system cannot find the path specified.", path)
checkMkdirAllSubdirs(t, path, false, errormsg)
errormsg = fmt.Sprintf("mkdir \\\\?\\%s\\: The system cannot find the path specified.", path)
checkMkdirAllSubdirs(t, `\\?\`+path, false, errormsg)
}

View File

@ -296,7 +296,7 @@ func main() {
log.Fatalf("%s: Syntax [opts] <directory>", os.Args[0])
}
dir := args[0]
_ = os.MkdirAll(dir, 0777)
_ = file.MkdirAll(dir, 0777)
var (
wg sync.WaitGroup

View File

@ -150,7 +150,7 @@ func New(ctx context.Context, fremote fs.Fs, opt *vfscommon.Options, avFn AddVir
// createDir creates a directory path, along with any necessary parents
func createDir(dir string) error {
return os.MkdirAll(dir, 0700)
return file.MkdirAll(dir, 0700)
}
// createRootDir creates a single cache root directory