From 01e50eb3fa376b6275d1fa6aae3bd21f757a1f0b Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 22 Dec 2016 23:04:53 +0000 Subject: [PATCH] lib/model: Handle filename conflicts on Windows. GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3810 LGTM: calmh --- lib/model/model.go | 18 +++- lib/model/rwfolder.go | 24 +++++ lib/osutil/name_conflict.go | 17 ++++ lib/osutil/name_conflict_windows.go | 48 ++++++++++ lib/osutil/name_conflict_windows_test.go | 108 +++++++++++++++++++++++ lib/scanner/walk.go | 8 ++ 6 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 lib/osutil/name_conflict.go create mode 100644 lib/osutil/name_conflict_windows.go create mode 100644 lib/osutil/name_conflict_windows_test.go diff --git a/lib/model/model.go b/lib/model/model.go index 574915c8..d1cb73c5 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -122,6 +122,7 @@ var ( errDeviceIgnored = errors.New("device is ignored") errNotRelative = errors.New("not a relative path") errNotDir = errors.New("parent is not a directory") + errNameConflict = errors.New("filename collides with existing file") ) // NewModel creates and starts a new model. The model starts in read-only mode, @@ -1159,6 +1160,11 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset return protocol.ErrNoSuchFile } + if !osutil.CheckNameConflict(folderPath, name) { + l.Debugf("%v REQ(in) for file not in dir: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf)) + return protocol.ErrNoSuchFile + } + // Only check temp files if the flag is set, and if we are set to advertise // the temp indexes. if fromTemporary && !folderCfg.DisableTempIndexes { @@ -1896,7 +1902,12 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error // The file is valid and not deleted. Lets check if it's // still here. - if _, err := mtimefs.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil { + var exists bool + if !osutil.IsDir(folderCfg.Path(), filepath.Dir(f.Name)) { + exists = false + } else if !osutil.CheckNameConflict(folderCfg.Path(), f.Name) { + exists = false + } else if _, err := mtimefs.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil { // We don't specifically verify that the error is // os.IsNotExist because there is a corner case when a // directory is suddenly transformed into a file. When that @@ -1904,6 +1915,11 @@ func (m *Model) internalScanFolderSubdirs(folder string, subDirs []string) error // file) are deleted but will return a confusing error ("not a // directory") when we try to Lstat() them. + exists = false + } else { + exists = true + } + if !exists { nf := protocol.FileInfo{ Name: f.Name, Type: f.Type, diff --git a/lib/model/rwfolder.go b/lib/model/rwfolder.go index fab982b9..ad9b56b7 100644 --- a/lib/model/rwfolder.go +++ b/lib/model/rwfolder.go @@ -435,6 +435,13 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int { continue } + // Verify that we handle the right thing and not something whose name + // collides. + if !osutil.CheckNameConflict(f.dir, fi.Name) { + f.newError(fi.Name, errNameConflict) + continue + } + switch { case fi.IsDeleted(): // A deleted file, directory or symlink @@ -524,6 +531,13 @@ nextFile: continue } + // Verify that we handle the right thing and not something whose name + // collides. + if !osutil.CheckNameConflict(f.dir, fi.Name) { + f.newError(fi.Name, errNameConflict) + continue + } + // Check our list of files to be removed for a match, in which case // we can just do a rename instead. key := string(fi.Blocks[0].Hash) @@ -1273,6 +1287,16 @@ func (f *sendReceiveFolder) copierRoutine(in <-chan copyBlocksState, pullChan ch if err != nil { return false } + // The following checks are racy + if !osutil.IsDir(folderRoots[folder], filepath.Dir(file)) { + return false + } + if !osutil.CheckNameConflict(folderRoots[folder], file) { + return false + } + if info, err := osutil.Lstat(inFile); err != nil || !info.Mode().IsRegular() { + return false + } fd, err := os.Open(inFile) if err != nil { return false diff --git a/lib/osutil/name_conflict.go b/lib/osutil/name_conflict.go new file mode 100644 index 00000000..a3edd8f0 --- /dev/null +++ b/lib/osutil/name_conflict.go @@ -0,0 +1,17 @@ +// Copyright (C) 2016 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +// +build !windows + +package osutil + +// CheckNameConflict returns true if every path component of name up to and +// including filepath.Join(base, name) doesn't conflict with any existing +// files or folders with different names. Base and name must both be clean and +// name must be relative to base. +func CheckNameConflict(base, name string) bool { + return true +} diff --git a/lib/osutil/name_conflict_windows.go b/lib/osutil/name_conflict_windows.go new file mode 100644 index 00000000..1014c92e --- /dev/null +++ b/lib/osutil/name_conflict_windows.go @@ -0,0 +1,48 @@ +// Copyright (C) 2016 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +// +build windows + +package osutil + +import ( + "os" + "path/filepath" + "strings" + "syscall" +) + +// CheckNameConflict returns true if every path component of name up to and +// including filepath.Join(base, name) doesn't conflict with any existing +// files or folders with different names. Base and name must both be clean and +// name must be relative to base. +func CheckNameConflict(base, name string) bool { + // Conflicts can be caused by different casing (e.g. foo and FOO) or + // by the use of short names (e.g. foo.barbaz and FOO~1.BAR). + path := base + parts := strings.Split(name, string(os.PathSeparator)) + for _, part := range parts { + path = filepath.Join(path, part) + pathp, err := syscall.UTF16PtrFromString(path) + if err != nil { + return false + } + var data syscall.Win32finddata + handle, err := syscall.FindFirstFile(pathp, &data) + if err == syscall.ERROR_FILE_NOT_FOUND { + return true + } + if err != nil { + return false + } + syscall.FindClose(handle) + fileName := syscall.UTF16ToString(data.FileName[:]) + if part != fileName { + return false + } + } + return true +} diff --git a/lib/osutil/name_conflict_windows_test.go b/lib/osutil/name_conflict_windows_test.go new file mode 100644 index 00000000..f8e6aace --- /dev/null +++ b/lib/osutil/name_conflict_windows_test.go @@ -0,0 +1,108 @@ +// Copyright (C) 2016 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +// +build windows + +package osutil_test + +import ( + "os" + "path" + "path/filepath" + "syscall" + "testing" + "unsafe" + + "github.com/syncthing/syncthing/lib/osutil" +) + +func TestCheckNameConflictCasing(t *testing.T) { + os.RemoveAll("testdata") + defer os.RemoveAll("testdata") + os.MkdirAll("testdata/Foo/BAR/baz", 0755) + // check if the file system is case-sensitive + if _, err := os.Lstat("testdata/foo"); err != nil { + t.Skip("pointless test") + return + } + + cases := []struct { + name string + conflictFree bool + }{ + // Exists + {"Foo", true}, + {"Foo/BAR", true}, + {"Foo/BAR/baz", true}, + // Doesn't exist + {"bar", true}, + {"Foo/baz", true}, + // Conflicts + {"foo", false}, + {"foo/BAR", false}, + {"Foo/bar", false}, + {"Foo/BAR/BAZ", false}, + } + + for _, tc := range cases { + nativeName := filepath.FromSlash(tc.name) + if res := osutil.CheckNameConflict("testdata", nativeName); res != tc.conflictFree { + t.Errorf("CheckNameConflict(%q) = %v, should be %v", tc.name, res, tc.conflictFree) + } + } +} + +func TestCheckNameConflictShortName(t *testing.T) { + os.RemoveAll("testdata") + defer os.RemoveAll("testdata") + os.MkdirAll("testdata/foobarbaz/qux", 0755) + ppath, err := syscall.UTF16PtrFromString("testdata/foobarbaz") + if err != nil { + t.Fatal("unexpected error", err) + } + // check if the file system supports short names + bufferSize, err := syscall.GetShortPathName(ppath, nil, 0) + if err != nil { + t.Skip("pointless test") + return + } + + // get the short name + buffer := make([]uint16, bufferSize) + length, err := syscall.GetShortPathName(ppath, + (*uint16)(unsafe.Pointer(&buffer[0])), bufferSize) + if err != nil { + t.Fatal("unexpected error", err) + } + // on success length doesn't contain the terminating null character + if bufferSize != length+1 { + t.Fatal("length of short name changed") + } + shortName := filepath.Base(syscall.UTF16ToString(buffer)) + + cases := []struct { + name string + conflictFree bool + }{ + // Exists + {"foobarbaz", true}, + {"foobarbaz/qux", true}, + // Doesn't exist + {"foo", true}, + {"foobarbaz/quux", true}, + // Conflicts + {shortName, false}, + {path.Join(shortName, "qux"), false}, + {path.Join(shortName, "quux"), false}, + } + + for _, tc := range cases { + nativeName := filepath.FromSlash(tc.name) + if res := osutil.CheckNameConflict("testdata", nativeName); res != tc.conflictFree { + t.Errorf("CheckNameConflict(%q) = %v, should be %v", tc.name, res, tc.conflictFree) + } + } +} diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index 7017508f..dff4751d 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -130,6 +130,14 @@ func (w *walker) walk() (chan protocol.FileInfo, error) { filepath.Walk(w.Dir, hashFiles) } else { for _, sub := range w.Subs { + if !osutil.IsDir(w.Dir, filepath.Dir(sub)) { + l.Infoln("Skipping sub path that is not in a directory", w.Dir, sub) + continue + } + if !osutil.CheckNameConflict(w.Dir, sub) { + l.Infoln("Skipping sub path that collides", w.Dir, sub) + continue + } filepath.Walk(filepath.Join(w.Dir, sub), hashFiles) } }