From fc2c46e82f14f1bdd2730fba141d2ee42a00e582 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Wed, 19 Aug 2020 19:58:51 +0200 Subject: [PATCH] lib/config, lib/fs: Make junction behaviour configurable (ref #6606) (#6907) --- lib/config/config.go | 2 +- lib/config/config_test.go | 1 + lib/config/folderconfiguration.go | 7 +++++- lib/config/migrations.go | 7 ++++++ lib/fs/basicfs.go | 25 ++++++++++++++++--- ...stat_broken.go => basicfs_lstat_broken.go} | 2 +- ...at_regular.go => basicfs_lstat_regular.go} | 2 +- ...at_windows.go => basicfs_lstat_windows.go} | 6 ++--- lib/fs/fakefs.go | 2 +- lib/fs/filesystem.go | 8 +++--- lib/fs/walkfs_test.go | 4 +-- 11 files changed, 49 insertions(+), 17 deletions(-) rename lib/fs/{lstat_broken.go => basicfs_lstat_broken.go} (89%) rename lib/fs/{lstat_regular.go => basicfs_lstat_regular.go} (80%) rename lib/fs/{lstat_windows.go => basicfs_lstat_windows.go} (91%) diff --git a/lib/config/config.go b/lib/config/config.go index 1319615ea..f0ef83b34 100644 --- a/lib/config/config.go +++ b/lib/config/config.go @@ -31,7 +31,7 @@ import ( const ( OldestHandledVersion = 10 - CurrentVersion = 31 + CurrentVersion = 32 MaxRescanIntervalS = 365 * 24 * 60 * 60 ) diff --git a/lib/config/config_test.go b/lib/config/config_test.go index 7b0dce22c..5c4c84537 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -126,6 +126,7 @@ func TestDeviceConfig(t *testing.T) { }, WeakHashThresholdPct: 25, MarkerName: DefaultMarkerName, + JunctionsAsDirs: true, }, } diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go index 3026543a2..3dd8414af 100644 --- a/lib/config/folderconfiguration.go +++ b/lib/config/folderconfiguration.go @@ -62,6 +62,7 @@ type FolderConfiguration struct { BlockPullOrder BlockPullOrder `xml:"blockPullOrder" json:"blockPullOrder"` CopyRangeMethod fs.CopyRangeMethod `xml:"copyRangeMethod" json:"copyRangeMethod" default:"standard"` CaseSensitiveFS bool `xml:"caseSensitiveFS" json:"caseSensitiveFS"` + JunctionsAsDirs bool `xml:"junctionsAsDirs" json:"junctionsAsDirs"` cachedModTimeWindow time.Duration @@ -101,7 +102,11 @@ func (f FolderConfiguration) Copy() FolderConfiguration { func (f FolderConfiguration) Filesystem() fs.Filesystem { // This is intentionally not a pointer method, because things like // cfg.Folders["default"].Filesystem() should be valid. - filesystem := fs.NewFilesystem(f.FilesystemType, f.Path) + var opts []fs.Option + if f.FilesystemType == fs.FilesystemTypeBasic && f.JunctionsAsDirs { + opts = append(opts, fs.WithJunctionsAsDirs()) + } + filesystem := fs.NewFilesystem(f.FilesystemType, f.Path, opts...) if !f.CaseSensitiveFS { filesystem = fs.NewCaseFilesystem(filesystem) } diff --git a/lib/config/migrations.go b/lib/config/migrations.go index 185287603..5f47b0d37 100644 --- a/lib/config/migrations.go +++ b/lib/config/migrations.go @@ -25,6 +25,7 @@ import ( // update the config version. The order of migrations doesn't matter here, // put the newest on top for readability. var migrations = migrationSet{ + {32, migrateToConfigV32}, {31, migrateToConfigV31}, {30, migrateToConfigV30}, {29, migrateToConfigV29}, @@ -91,6 +92,12 @@ func migrateToConfigV31(cfg *Configuration) { cfg.Options.UnackedNotificationIDs = append(cfg.Options.UnackedNotificationIDs, "authenticationUserAndPassword") } +func migrateToConfigV32(cfg *Configuration) { + for i := range cfg.Folders { + cfg.Folders[i].JunctionsAsDirs = true + } +} + func migrateToConfigV30(cfg *Configuration) { // The "max concurrent scans" option is now spelled "max folder concurrency" // to be more general. diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index 91020ce5b..46ad15bf0 100644 --- a/lib/fs/basicfs.go +++ b/lib/fs/basicfs.go @@ -23,13 +23,24 @@ var ( ErrNotRelative = errors.New("not a relative path") ) +func WithJunctionsAsDirs() Option { + return func(fs Filesystem) { + if basic, ok := fs.(*BasicFilesystem); !ok { + l.Warnln("WithJunctionsAsDirs must only be used with FilesystemTypeBasic") + } else { + basic.junctionsAsDirs = true + } + } +} + // The BasicFilesystem implements all aspects by delegating to package os. // All paths are relative to the root and cannot (should not) escape the root directory. type BasicFilesystem struct { - root string + root string + junctionsAsDirs bool } -func newBasicFilesystem(root string) *BasicFilesystem { +func newBasicFilesystem(root string, opts ...Option) *BasicFilesystem { if root == "" { root = "." // Otherwise "" becomes "/" below } @@ -64,7 +75,13 @@ func newBasicFilesystem(root string) *BasicFilesystem { root = longFilenameSupport(root) } - return &BasicFilesystem{root} + fs := &BasicFilesystem{ + root: root, + } + for _, opt := range opts { + opt(fs) + } + return fs } // rooted expands the relative path to the full path that is then used with os @@ -145,7 +162,7 @@ func (f *BasicFilesystem) Lstat(name string) (FileInfo, error) { if err != nil { return nil, err } - fi, err := underlyingLstat(name) + fi, err := f.underlyingLstat(name) if err != nil { return nil, err } diff --git a/lib/fs/lstat_broken.go b/lib/fs/basicfs_lstat_broken.go similarity index 89% rename from lib/fs/lstat_broken.go rename to lib/fs/basicfs_lstat_broken.go index a35c74f4c..e9c4ceee9 100644 --- a/lib/fs/lstat_broken.go +++ b/lib/fs/basicfs_lstat_broken.go @@ -16,7 +16,7 @@ import ( // Lstat is like os.Lstat, except lobotomized for Android. See // https://forum.syncthing.net/t/2395 -func underlyingLstat(name string) (fi os.FileInfo, err error) { +func (*BasicFilesystem) underlyingLstat(name string) (fi os.FileInfo, err error) { for i := 0; i < 10; i++ { // We have to draw the line somewhere fi, err = os.Lstat(name) if err, ok := err.(*os.PathError); ok && err.Err == syscall.EINTR { diff --git a/lib/fs/lstat_regular.go b/lib/fs/basicfs_lstat_regular.go similarity index 80% rename from lib/fs/lstat_regular.go rename to lib/fs/basicfs_lstat_regular.go index cd31091a8..70ec2f8be 100644 --- a/lib/fs/lstat_regular.go +++ b/lib/fs/basicfs_lstat_regular.go @@ -10,6 +10,6 @@ package fs import "os" -func underlyingLstat(name string) (fi os.FileInfo, err error) { +func (*BasicFilesystem) underlyingLstat(name string) (fi os.FileInfo, err error) { return os.Lstat(name) } diff --git a/lib/fs/lstat_windows.go b/lib/fs/basicfs_lstat_windows.go similarity index 91% rename from lib/fs/lstat_windows.go rename to lib/fs/basicfs_lstat_windows.go index 14730894b..ccf113814 100644 --- a/lib/fs/lstat_windows.go +++ b/lib/fs/basicfs_lstat_windows.go @@ -66,12 +66,12 @@ func (fi *dirJunctFileInfo) IsDir() bool { return true } -func underlyingLstat(name string) (os.FileInfo, error) { +func (f *BasicFilesystem) underlyingLstat(name string) (os.FileInfo, error) { var fi, err = os.Lstat(name) - // NTFS directory junctions are treated as ordinary directories, + // NTFS directory junctions can be treated as ordinary directories, // see https://forum.syncthing.net/t/option-to-follow-directory-junctions-symbolic-links/14750 - if err == nil && fi.Mode()&os.ModeSymlink != 0 { + if err == nil && f.junctionsAsDirs && fi.Mode()&os.ModeSymlink != 0 { var isJunct bool isJunct, err = isDirectoryJunction(name) if err == nil && isJunct { diff --git a/lib/fs/fakefs.go b/lib/fs/fakefs.go index 7f29ab327..ab9163622 100644 --- a/lib/fs/fakefs.go +++ b/lib/fs/fakefs.go @@ -66,7 +66,7 @@ var ( fakefsFs = make(map[string]*fakefs) ) -func newFakeFilesystem(rootURI string) *fakefs { +func newFakeFilesystem(rootURI string, _ ...Option) *fakefs { fakefsMut.Lock() defer fakefsMut.Unlock() diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go index 0bfc6ff21..db61f4406 100644 --- a/lib/fs/filesystem.go +++ b/lib/fs/filesystem.go @@ -178,13 +178,15 @@ var IsPermission = os.IsPermission // IsPathSeparator is the equivalent of os.IsPathSeparator var IsPathSeparator = os.IsPathSeparator -func NewFilesystem(fsType FilesystemType, uri string) Filesystem { +type Option func(Filesystem) + +func NewFilesystem(fsType FilesystemType, uri string, opts ...Option) Filesystem { var fs Filesystem switch fsType { case FilesystemTypeBasic: - fs = newBasicFilesystem(uri) + fs = newBasicFilesystem(uri, opts...) case FilesystemTypeFake: - fs = newFakeFilesystem(uri) + fs = newFakeFilesystem(uri, opts...) default: l.Debugln("Unknown filesystem", fsType, uri) fs = &errorFilesystem{ diff --git a/lib/fs/walkfs_test.go b/lib/fs/walkfs_test.go index 934d96c42..3dd44cb8d 100644 --- a/lib/fs/walkfs_test.go +++ b/lib/fs/walkfs_test.go @@ -57,7 +57,7 @@ func testWalkTraverseDirJunct(t *testing.T, fsType FilesystemType, uri string) { t.Skip("Directory junctions are available and tested on windows only") } - fs := NewFilesystem(fsType, uri) + fs := NewFilesystem(fsType, uri, WithJunctionsAsDirs()) if err := fs.MkdirAll("target/foo", 0); err != nil { t.Fatal(err) @@ -90,7 +90,7 @@ func testWalkInfiniteRecursion(t *testing.T, fsType FilesystemType, uri string) t.Skip("Infinite recursion detection is tested on windows only") } - fs := NewFilesystem(fsType, uri) + fs := NewFilesystem(fsType, uri, WithJunctionsAsDirs()) if err := fs.MkdirAll("target/foo", 0); err != nil { t.Fatal(err)