acd,box,onedrive,opendrive,ploud: fix Features() retaining the original receiver

Before this change the Features() method would return a different Fs
to that the Features() method was called on if the remote was
instantiated on a file.

The practical effect of this is that optional features, eg `rclone
about` wouldn't work properly when called on a file, and likely this
has been causing low level problems for users of these backends for
ages.

Ideally there would be a test for this, but it turns out that this is
really hard, so instead of that all the backends have been converted
to not copy the Fs and a big warning comment inserted for future
readers.

Fixes #2182
This commit is contained in:
Nick Craig-Wood 2018-10-14 14:41:26 +01:00
parent dcce84714e
commit 0f2a5403db
5 changed files with 56 additions and 31 deletions

View File

@ -312,16 +312,16 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
if err != nil { if err != nil {
// Assume it is a file // Assume it is a file
newRoot, remote := dircache.SplitPath(root) newRoot, remote := dircache.SplitPath(root)
newF := *f tempF := *f
newF.dirCache = dircache.New(newRoot, f.trueRootID, &newF) tempF.dirCache = dircache.New(newRoot, f.trueRootID, &tempF)
newF.root = newRoot tempF.root = newRoot
// Make new Fs which is the parent // Make new Fs which is the parent
err = newF.dirCache.FindRoot(false) err = tempF.dirCache.FindRoot(false)
if err != nil { if err != nil {
// No root so return old f // No root so return old f
return f, nil return f, nil
} }
_, err := newF.newObjectWithInfo(remote, nil) _, err := tempF.newObjectWithInfo(remote, nil)
if err != nil { if err != nil {
if err == fs.ErrorObjectNotFound { if err == fs.ErrorObjectNotFound {
// File doesn't exist so return old f // File doesn't exist so return old f
@ -329,8 +329,13 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
} }
return nil, err return nil, err
} }
// XXX: update the old f here instead of returning tempF, since
// `features` were already filled with functions having *f as a receiver.
// See https://github.com/ncw/rclone/issues/2182
f.dirCache = tempF.dirCache
f.root = tempF.root
// return an error with an fs which points to the parent // return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile return f, fs.ErrorIsFile
} }
return f, nil return f, nil
} }

View File

@ -283,16 +283,16 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
if err != nil { if err != nil {
// Assume it is a file // Assume it is a file
newRoot, remote := dircache.SplitPath(root) newRoot, remote := dircache.SplitPath(root)
newF := *f tempF := *f
newF.dirCache = dircache.New(newRoot, rootID, &newF) tempF.dirCache = dircache.New(newRoot, rootID, &tempF)
newF.root = newRoot tempF.root = newRoot
// Make new Fs which is the parent // Make new Fs which is the parent
err = newF.dirCache.FindRoot(false) err = tempF.dirCache.FindRoot(false)
if err != nil { if err != nil {
// No root so return old f // No root so return old f
return f, nil return f, nil
} }
_, err := newF.newObjectWithInfo(remote, nil) _, err := tempF.newObjectWithInfo(remote, nil)
if err != nil { if err != nil {
if err == fs.ErrorObjectNotFound { if err == fs.ErrorObjectNotFound {
// File doesn't exist so return old f // File doesn't exist so return old f
@ -300,9 +300,14 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
} }
return nil, err return nil, err
} }
f.features.Fill(&newF) f.features.Fill(&tempF)
// XXX: update the old f here instead of returning tempF, since
// `features` were already filled with functions having *f as a receiver.
// See https://github.com/ncw/rclone/issues/2182
f.dirCache = tempF.dirCache
f.root = tempF.root
// return an error with an fs which points to the parent // return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile return f, fs.ErrorIsFile
} }
return f, nil return f, nil
} }

View File

@ -448,16 +448,16 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
if err != nil { if err != nil {
// Assume it is a file // Assume it is a file
newRoot, remote := dircache.SplitPath(root) newRoot, remote := dircache.SplitPath(root)
newF := *f tempF := *f
newF.dirCache = dircache.New(newRoot, rootInfo.ID, &newF) tempF.dirCache = dircache.New(newRoot, rootInfo.ID, &tempF)
newF.root = newRoot tempF.root = newRoot
// Make new Fs which is the parent // Make new Fs which is the parent
err = newF.dirCache.FindRoot(false) err = tempF.dirCache.FindRoot(false)
if err != nil { if err != nil {
// No root so return old f // No root so return old f
return f, nil return f, nil
} }
_, err := newF.newObjectWithInfo(remote, nil) _, err := tempF.newObjectWithInfo(remote, nil)
if err != nil { if err != nil {
if err == fs.ErrorObjectNotFound { if err == fs.ErrorObjectNotFound {
// File doesn't exist so return old f // File doesn't exist so return old f
@ -465,8 +465,13 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
} }
return nil, err return nil, err
} }
// XXX: update the old f here instead of returning tempF, since
// `features` were already filled with functions having *f as a receiver.
// See https://github.com/ncw/rclone/issues/2182
f.dirCache = tempF.dirCache
f.root = tempF.root
// return an error with an fs which points to the parent // return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile return f, fs.ErrorIsFile
} }
return f, nil return f, nil
} }

View File

@ -177,17 +177,17 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
if err != nil { if err != nil {
// Assume it is a file // Assume it is a file
newRoot, remote := dircache.SplitPath(root) newRoot, remote := dircache.SplitPath(root)
newF := *f tempF := *f
newF.dirCache = dircache.New(newRoot, "0", &newF) tempF.dirCache = dircache.New(newRoot, "0", &tempF)
newF.root = newRoot tempF.root = newRoot
// Make new Fs which is the parent // Make new Fs which is the parent
err = newF.dirCache.FindRoot(false) err = tempF.dirCache.FindRoot(false)
if err != nil { if err != nil {
// No root so return old f // No root so return old f
return f, nil return f, nil
} }
_, err := newF.newObjectWithInfo(remote, nil) _, err := tempF.newObjectWithInfo(remote, nil)
if err != nil { if err != nil {
if err == fs.ErrorObjectNotFound { if err == fs.ErrorObjectNotFound {
// File doesn't exist so return old f // File doesn't exist so return old f
@ -195,8 +195,13 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
} }
return nil, err return nil, err
} }
// XXX: update the old f here instead of returning tempF, since
// `features` were already filled with functions having *f as a receiver.
// See https://github.com/ncw/rclone/issues/2182
f.dirCache = tempF.dirCache
f.root = tempF.root
// return an error with an fs which points to the parent // return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile return f, fs.ErrorIsFile
} }
return f, nil return f, nil
} }

View File

@ -276,16 +276,16 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
if err != nil { if err != nil {
// Assume it is a file // Assume it is a file
newRoot, remote := dircache.SplitPath(root) newRoot, remote := dircache.SplitPath(root)
newF := *f tempF := *f
newF.dirCache = dircache.New(newRoot, rootID, &newF) tempF.dirCache = dircache.New(newRoot, rootID, &tempF)
newF.root = newRoot tempF.root = newRoot
// Make new Fs which is the parent // Make new Fs which is the parent
err = newF.dirCache.FindRoot(false) err = tempF.dirCache.FindRoot(false)
if err != nil { if err != nil {
// No root so return old f // No root so return old f
return f, nil return f, nil
} }
_, err := newF.newObjectWithInfo(remote, nil) _, err := tempF.newObjectWithInfo(remote, nil)
if err != nil { if err != nil {
if err == fs.ErrorObjectNotFound { if err == fs.ErrorObjectNotFound {
// File doesn't exist so return old f // File doesn't exist so return old f
@ -293,8 +293,13 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
} }
return nil, err return nil, err
} }
// XXX: update the old f here instead of returning tempF, since
// `features` were already filled with functions having *f as a receiver.
// See https://github.com/ncw/rclone/issues/2182
f.dirCache = tempF.dirCache
f.root = tempF.root
// return an error with an fs which points to the parent // return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile return f, fs.ErrorIsFile
} }
return f, nil return f, nil
} }