From 84b64dcdf9014044f1d0ba6b17e2001ac77f07ba Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 14 Nov 2024 16:20:06 +0000 Subject: [PATCH] Revert "Merge commit from fork" This reverts commit 1e2b354456882ed22d8674b8bf37de55e0069514. --- backend/local/lchmod.go | 16 ------ backend/local/lchmod_unix.go | 41 --------------- backend/local/lchtimes.go | 2 +- backend/local/lchtimes_windows.go | 19 ------- backend/local/local_internal_test.go | 76 ++++------------------------ backend/local/metadata.go | 23 ++------- backend/local/setbtime.go | 6 --- backend/local/setbtime_windows.go | 37 ++------------ 8 files changed, 18 insertions(+), 202 deletions(-) delete mode 100644 backend/local/lchmod.go delete mode 100644 backend/local/lchmod_unix.go delete mode 100644 backend/local/lchtimes_windows.go diff --git a/backend/local/lchmod.go b/backend/local/lchmod.go deleted file mode 100644 index 823718dfe..000000000 --- a/backend/local/lchmod.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build windows || plan9 || js || linux - -package local - -import "os" - -const haveLChmod = false - -// lChmod changes the mode of the named file to mode. If the file is a symbolic -// link, it changes the link, not the target. If there is an error, -// it will be of type *PathError. -func lChmod(name string, mode os.FileMode) error { - // Can't do this safely on this OS - chmoding a symlink always - // changes the destination. - return nil -} diff --git a/backend/local/lchmod_unix.go b/backend/local/lchmod_unix.go deleted file mode 100644 index f1fdc4745..000000000 --- a/backend/local/lchmod_unix.go +++ /dev/null @@ -1,41 +0,0 @@ -//go:build !windows && !plan9 && !js && !linux - -package local - -import ( - "os" - "syscall" - - "golang.org/x/sys/unix" -) - -const haveLChmod = true - -// syscallMode returns the syscall-specific mode bits from Go's portable mode bits. -// -// Borrowed from the syscall source since it isn't public. -func syscallMode(i os.FileMode) (o uint32) { - o |= uint32(i.Perm()) - if i&os.ModeSetuid != 0 { - o |= syscall.S_ISUID - } - if i&os.ModeSetgid != 0 { - o |= syscall.S_ISGID - } - if i&os.ModeSticky != 0 { - o |= syscall.S_ISVTX - } - return o -} - -// lChmod changes the mode of the named file to mode. If the file is a symbolic -// link, it changes the link, not the target. If there is an error, -// it will be of type *PathError. -func lChmod(name string, mode os.FileMode) error { - // NB linux does not support AT_SYMLINK_NOFOLLOW as a parameter to fchmodat - // and returns ENOTSUP if you try, so we don't support this on linux - if e := unix.Fchmodat(unix.AT_FDCWD, name, syscallMode(mode), unix.AT_SYMLINK_NOFOLLOW); e != nil { - return &os.PathError{Op: "lChmod", Path: name, Err: e} - } - return nil -} diff --git a/backend/local/lchtimes.go b/backend/local/lchtimes.go index fcabdcc34..c8f03ef46 100644 --- a/backend/local/lchtimes.go +++ b/backend/local/lchtimes.go @@ -1,4 +1,4 @@ -//go:build plan9 || js +//go:build windows || plan9 || js package local diff --git a/backend/local/lchtimes_windows.go b/backend/local/lchtimes_windows.go deleted file mode 100644 index a6dec9a12..000000000 --- a/backend/local/lchtimes_windows.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build windows - -package local - -import ( - "time" -) - -const haveLChtimes = true - -// lChtimes changes the access and modification times of the named -// link, similar to the Unix utime() or utimes() functions. -// -// The underlying filesystem may truncate or round the values to a -// less precise time unit. -// If there is an error, it will be of type *PathError. -func lChtimes(name string, atime time.Time, mtime time.Time) error { - return setTimes(name, atime, mtime, time.Time{}, true) -} diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go index b3b9b8ba1..ea0fdc765 100644 --- a/backend/local/local_internal_test.go +++ b/backend/local/local_internal_test.go @@ -268,66 +268,22 @@ func TestMetadata(t *testing.T) { r := fstest.NewRun(t) const filePath = "metafile.txt" when := time.Now() + const dayLength = len("2001-01-01") + whenRFC := when.Format(time.RFC3339Nano) r.WriteFile(filePath, "metadata file contents", when) f := r.Flocal.(*Fs) - // Set fs into "-l" / "--links" mode - f.opt.TranslateSymlinks = true - - // Write a symlink to the file - symlinkPath := "metafile-link.txt" - osSymlinkPath := filepath.Join(f.root, symlinkPath) - symlinkPath += linkSuffix - require.NoError(t, os.Symlink(filePath, osSymlinkPath)) - symlinkModTime := fstest.Time("2002-02-03T04:05:10.123123123Z") - require.NoError(t, lChtimes(osSymlinkPath, symlinkModTime, symlinkModTime)) - // Get the object obj, err := f.NewObject(ctx, filePath) require.NoError(t, err) o := obj.(*Object) - // Get the symlink object - symlinkObj, err := f.NewObject(ctx, symlinkPath) - require.NoError(t, err) - symlinkO := symlinkObj.(*Object) - - // Record metadata for o - oMeta, err := o.Metadata(ctx) - require.NoError(t, err) - - // Test symlink first to check it doesn't mess up file - t.Run("Symlink", func(t *testing.T) { - testMetadata(t, r, symlinkO, symlinkModTime) - }) - - // Read it again - oMetaNew, err := o.Metadata(ctx) - require.NoError(t, err) - - // Check that operating on the symlink didn't change the file it was pointing to - // See: https://github.com/rclone/rclone/security/advisories/GHSA-hrxh-9w67-g4cv - assert.Equal(t, oMeta, oMetaNew, "metadata setting on symlink messed up file") - - // Now run the same tests on the file - t.Run("File", func(t *testing.T) { - testMetadata(t, r, o, when) - }) -} - -func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { - ctx := context.Background() - whenRFC := when.Format(time.RFC3339Nano) - const dayLength = len("2001-01-01") - - f := r.Flocal.(*Fs) features := f.Features() - var hasXID, hasAtime, hasBtime, canSetXattrOnLinks bool + var hasXID, hasAtime, hasBtime bool switch runtime.GOOS { case "darwin", "freebsd", "netbsd", "linux": hasXID, hasAtime, hasBtime = true, true, true - canSetXattrOnLinks = runtime.GOOS != "linux" case "openbsd", "solaris": hasXID, hasAtime = true, true case "windows": @@ -350,10 +306,6 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { require.NoError(t, err) assert.Nil(t, m) - if !canSetXattrOnLinks && o.translatedLink { - t.Skip("Skip remainder of test as can't set xattr on symlinks on this OS") - } - inM := fs.Metadata{ "potato": "chips", "cabbage": "soup", @@ -368,21 +320,18 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { }) checkTime := func(m fs.Metadata, key string, when time.Time) { - t.Helper() mt, ok := o.parseMetadataTime(m, key) assert.True(t, ok) dt := mt.Sub(when) precision := time.Second - assert.True(t, dt >= -precision && dt <= precision, fmt.Sprintf("%s: dt %v outside +/- precision %v want %v got %v", key, dt, precision, mt, when)) + assert.True(t, dt >= -precision && dt <= precision, fmt.Sprintf("%s: dt %v outside +/- precision %v", key, dt, precision)) } checkInt := func(m fs.Metadata, key string, base int) int { - t.Helper() value, ok := o.parseMetadataInt(m, key, base) assert.True(t, ok) return value } - t.Run("Read", func(t *testing.T) { m, err := o.Metadata(ctx) require.NoError(t, err) @@ -392,12 +341,13 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { checkInt(m, "mode", 8) checkTime(m, "mtime", when) + assert.Equal(t, len(whenRFC), len(m["mtime"])) assert.Equal(t, whenRFC[:dayLength], m["mtime"][:dayLength]) - if hasAtime && !o.translatedLink { // symlinks generally don't record atime + if hasAtime { checkTime(m, "atime", when) } - if hasBtime && !o.translatedLink { // symlinks generally don't record btime + if hasBtime { checkTime(m, "btime", when) } if hasXID { @@ -421,10 +371,6 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { "mode": "0767", "potato": "wedges", } - if !canSetXattrOnLinks && o.translatedLink { - // Don't change xattr if not supported on symlinks - delete(newM, "potato") - } err := o.writeMetadata(newM) require.NoError(t, err) @@ -434,11 +380,7 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { mode := checkInt(m, "mode", 8) if runtime.GOOS != "windows" { - expectedMode := 0767 - if o.translatedLink && runtime.GOOS == "linux" { - expectedMode = 0777 // perms of symlinks always read as 0777 on linux - } - assert.Equal(t, expectedMode, mode&0777, fmt.Sprintf("mode wrong - expecting 0%o got 0%o", expectedMode, mode&0777)) + assert.Equal(t, 0767, mode&0777, fmt.Sprintf("mode wrong - expecting 0767 got 0%o", mode&0777)) } checkTime(m, "mtime", newMtime) @@ -448,7 +390,7 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { if haveSetBTime { checkTime(m, "btime", newBtime) } - if xattrSupported && (canSetXattrOnLinks || !o.translatedLink) { + if xattrSupported { assert.Equal(t, "wedges", m["potato"]) } }) diff --git a/backend/local/metadata.go b/backend/local/metadata.go index 75b195e64..7ab69af30 100644 --- a/backend/local/metadata.go +++ b/backend/local/metadata.go @@ -105,11 +105,7 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { } if haveSetBTime { if btimeOK { - if o.translatedLink { - err = lsetBTime(o.path, btime) - } else { - err = setBTime(o.path, btime) - } + err = setBTime(o.path, btime) if err != nil { outErr = fmt.Errorf("failed to set birth (creation) time: %w", err) } @@ -125,11 +121,7 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { if runtime.GOOS == "windows" || runtime.GOOS == "plan9" { fs.Debugf(o, "Ignoring request to set ownership %o.%o on this OS", gid, uid) } else { - if o.translatedLink { - err = os.Lchown(o.path, uid, gid) - } else { - err = os.Chown(o.path, uid, gid) - } + err = os.Chown(o.path, uid, gid) if err != nil { outErr = fmt.Errorf("failed to change ownership: %w", err) } @@ -140,16 +132,7 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { if mode >= 0 { umode := uint(mode) if umode <= math.MaxUint32 { - if o.translatedLink { - if haveLChmod { - err = lChmod(o.path, os.FileMode(umode)) - } else { - fs.Debugf(o, "Unable to set mode %v on a symlink on this OS", os.FileMode(umode)) - err = nil - } - } else { - err = os.Chmod(o.path, os.FileMode(umode)) - } + err = os.Chmod(o.path, os.FileMode(umode)) if err != nil { outErr = fmt.Errorf("failed to change permissions: %w", err) } diff --git a/backend/local/setbtime.go b/backend/local/setbtime.go index bb37b1735..5c946348f 100644 --- a/backend/local/setbtime.go +++ b/backend/local/setbtime.go @@ -13,9 +13,3 @@ func setBTime(name string, btime time.Time) error { // Does nothing return nil } - -// lsetBTime changes the birth time of the link passed in -func lsetBTime(name string, btime time.Time) error { - // Does nothing - return nil -} diff --git a/backend/local/setbtime_windows.go b/backend/local/setbtime_windows.go index 8ae499826..510a4da6a 100644 --- a/backend/local/setbtime_windows.go +++ b/backend/local/setbtime_windows.go @@ -9,20 +9,15 @@ import ( const haveSetBTime = true -// setTimes sets any of atime, mtime or btime -// if link is set it sets a link rather than the target -func setTimes(name string, atime, mtime, btime time.Time, link bool) (err error) { +// setBTime sets the birth time of the file passed in +func setBTime(name string, btime time.Time) (err error) { pathp, err := syscall.UTF16PtrFromString(name) if err != nil { return err } - fileFlag := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS) - if link { - fileFlag |= syscall.FILE_FLAG_OPEN_REPARSE_POINT - } h, err := syscall.CreateFile(pathp, syscall.FILE_WRITE_ATTRIBUTES, syscall.FILE_SHARE_WRITE, nil, - syscall.OPEN_EXISTING, fileFlag, 0) + syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS, 0) if err != nil { return err } @@ -32,28 +27,6 @@ func setTimes(name string, atime, mtime, btime time.Time, link bool) (err error) err = closeErr } }() - var patime, pmtime, pbtime *syscall.Filetime - if !atime.IsZero() { - t := syscall.NsecToFiletime(atime.UnixNano()) - patime = &t - } - if !mtime.IsZero() { - t := syscall.NsecToFiletime(mtime.UnixNano()) - pmtime = &t - } - if !btime.IsZero() { - t := syscall.NsecToFiletime(btime.UnixNano()) - pbtime = &t - } - return syscall.SetFileTime(h, pbtime, patime, pmtime) -} - -// setBTime sets the birth time of the file passed in -func setBTime(name string, btime time.Time) (err error) { - return setTimes(name, time.Time{}, time.Time{}, btime, false) -} - -// lsetBTime changes the birth time of the link passed in -func lsetBTime(name string, btime time.Time) error { - return setTimes(name, time.Time{}, time.Time{}, btime, true) + bFileTime := syscall.NsecToFiletime(btime.UnixNano()) + return syscall.SetFileTime(h, &bFileTime, nil, nil) }