Skip to content

Conversation

@amenzhinsky
Copy link

@amenzhinsky amenzhinsky commented Feb 13, 2017

Hi there!

In some cases it would be really helpful to have possibility to use custom FS for opening files.
The best example is go-bindata integration when we need to embed file assets in go binary.

type BindataFS struct{}

// Open opens the named file
func (fs *BindataFS) Open(name string) (http.File, error) {
	b, err := bindata.Asset(name)
	if err != nil {
		return nil, err
	}

	return &file{
		Reader: bytes.NewReader(b),
		name:   name,
	}, nil
}

type file struct {
	*bytes.Reader
	name string
}

func (f *file) Close() error {
	return nil
}

func (f *file) Stat() (os.FileInfo, error) {
	return bindata.AssetInfo(f.name)
}

func (f *file) Readdir(count int) ([]os.FileInfo, error) {
	return nil, nil
}

Then we just change our filesystem and it works like a charm!

e := echo.New()
e.FS = &BindataFS{}

@coveralls
Copy link

coveralls commented Feb 13, 2017

Coverage Status

Coverage increased (+0.007%) to 85.256% when pulling 04dc4b9 on amenzhinsky:fs into a8b6864 on labstack:master.

@amenzhinsky
Copy link
Author

I'm not sure about the FS default value e.FS = http.Dir(".") because we won't be able to serve files outside of cwd, but at the same time this is a security enhancement.

func fileHandler(c echo.Context) error {
	return c.File("/etc/passwd") // won't work
}

If someone needs to make it work, FS has to be changed to Dir("/") or it can be default value.

@vishr vishr self-requested a review February 21, 2017 23:26
@vishr vishr self-assigned this Feb 21, 2017
@vishr
Copy link
Member

vishr commented Feb 22, 2017

@amenzhinsky Like you mentioned, people won't be able to serve files outside cwd, I think it is a breaking change. Even in our test cases we are using relative path, which actually failed.

@coveralls
Copy link

coveralls commented Feb 22, 2017

Coverage Status

Coverage increased (+0.03%) to 84.701% when pulling e9eeddf on amenzhinsky:fs into 7a858a7 on labstack:master.

@amenzhinsky
Copy link
Author

@vishr added generic FS implementation, which is basically wrapper of os.Open, please have a look.

@coveralls
Copy link

coveralls commented Feb 22, 2017

Coverage Status

Coverage increased (+0.03%) to 84.701% when pulling 1ce8e18 on amenzhinsky:fs into 7a858a7 on labstack:master.

@vishr vishr force-pushed the master branch 3 times, most recently from f652f79 to 3fafadf Compare June 10, 2017 02:28
@stale
Copy link

stale bot commented Nov 30, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 30, 2018
@stale stale bot closed this Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants