Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to "chain" BasePathFs #79

Closed
francoishill opened this issue Apr 17, 2016 · 4 comments
Closed

Unable to "chain" BasePathFs #79

francoishill opened this issue Apr 17, 2016 · 4 comments

Comments

@francoishill
Copy link
Contributor

This example does not work as expected:

package main

import (
    "fmt"

    "github.com/spf13/afero"
)

func main() {
    fs1 := afero.NewBasePathFs(afero.NewOsFs(), "/dir1/subdir1")
    fmt.Println(fs1.(*afero.BasePathFs).RealPath("file.txt"))
    fs2 := afero.NewBasePathFs(fs1, "/dir2/subdir2")
    fmt.Println(fs2.(*afero.BasePathFs).RealPath("file.txt"))
}

The above output is:

/dir1/subdir1/file.txt
/dir2/subdir2/file.txt

Instead I expected fs2 to have nested paths as it uses fs1 as its source:

/dir1/subdir1/file.txt
/dir1/subdir1/dir2/subdir2/file.txt
@vetinari
Copy link
Collaborator

If the example would print the nested path, an app which just gets the fs2 from your example above, would be able to reveal too much about the system, IMO.
If all agree to extend the RealPath() to check for the base's type and call then that RealPath(), I can add it (@spf13 , @mbertschler please :))

@francoishill
Copy link
Contributor Author

Your point makes sense. The problem, however, is that the RealPath method is used by all other api methods (like Mkdir, Open, etc). Regarding your concern, perhaps make the RealPath method private?

Before reading your comment I did however implement pull request 80 to resolve this.

@mbertschler
Copy link
Collaborator

Hmm... #80 looks nice and works, but the security issue is probably also a valid point. Maybe we could leave the RealPath() behaviour like this, but add a private fullPath() method that works like in your pull request and then replace all internal usages of RealPath() with this?

@francoishill
Copy link
Contributor Author

Sure, I like that

desdeel2d0m added a commit to desdeel2d0m/afero that referenced this issue Jul 1, 2024
Added fix to support "nested" `BasePathFs`. Unit-tests are also included.
desdeel2d0m added a commit to desdeel2d0m/afero that referenced this issue Jul 1, 2024
Rather keep the `RealPath` behaviour as originally implemented. Added a new method `fullPath` which is used by all other methods like `MkDir`.
desdeel2d0m added a commit to desdeel2d0m/afero that referenced this issue Jul 1, 2024
Amendment to previous commit, fixed the related test
desdeel2d0m added a commit to desdeel2d0m/afero that referenced this issue Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants