Skip to content

Commit

Permalink
fix: handle symlinks by reading/following as needed
Browse files Browse the repository at this point in the history 
Avoid using filepath.EvalSymlinks and let the caller
fail appropriately for symlinks.

Fixes minio#3011
  • Loading branch information
@harshavardhana
harshavardhana committed Jan 23, 2020
1 parent 0baee49 commit 96f595f
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 157 deletions.
173 changes: 18 additions & 155 deletions cmd/client-fs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"context"
"errors"
"io"
"io/ioutil"
"os"
Expand Down Expand Up @@ -431,57 +432,24 @@ func (f *fsClient) ShareUpload(startsWith bool, expires time.Duration, contentTy
})
}

// readFile reads and returns the data inside the file located
// at the provided filepath.
func readFile(fpath string) (io.ReadCloser, error) {
// Golang strips trailing / if you clean(..) or
// EvalSymlinks(..). Adding '.' prevents it from doing so.
if strings.HasSuffix(fpath, "/") {
fpath = fpath + "."
}
fpath, e := filepath.EvalSymlinks(fpath)
if e != nil {
return nil, e
}
fileData, e := os.Open(fpath)
if e != nil {
return nil, e
}
return fileData, nil
}

// Copy - copy data from source to destination
func (f *fsClient) Copy(source string, size int64, progress io.Reader, srcSSE, tgtSSE encrypt.ServerSide, metadata map[string]string) *probe.Error {
destination := f.PathURL.Path
rc, e := readFile(source)
rc, e := os.Open(source)
if e != nil {
err := f.toClientError(e, destination)
return err.Trace(destination)
err := f.toClientError(e, source)
return err.Trace(source)
}
defer rc.Close()

_, err := f.put(rc, size, map[string][]string{}, progress)
if err != nil {
destination := f.PathURL.Path
if _, err := f.put(rc, size, map[string][]string{}, progress); err != nil {
return err.Trace(destination, source)
}
return nil
}

// get - get wrapper returning object reader.
func (f *fsClient) get() (io.ReadCloser, *probe.Error) {
tmppath := f.PathURL.Path
// Golang strips trailing / if you clean(..) or
// EvalSymlinks(..). Adding '.' prevents it from doing so.
if strings.HasSuffix(tmppath, string(f.PathURL.Separator)) {
tmppath = tmppath + "."
}

// Resolve symlinks.
_, e := filepath.EvalSymlinks(tmppath)
if e != nil {
err := f.toClientError(e, f.PathURL.Path)
return nil, err.Trace(f.PathURL.Path)
}
// Get returns reader and any additional metadata.
func (f *fsClient) Get(sse encrypt.ServerSide) (io.ReadCloser, *probe.Error) {
fileData, e := os.Open(f.PathURL.Path)
if e != nil {
err := f.toClientError(e, f.PathURL.Path)
Expand All @@ -490,11 +458,6 @@ func (f *fsClient) get() (io.ReadCloser, *probe.Error) {
return fileData, nil
}

// Get returns reader and any additional metadata.
func (f *fsClient) Get(sse encrypt.ServerSide) (io.ReadCloser, *probe.Error) {
return f.get()
}

// Check if the given error corresponds to ENOTEMPTY for unix
// and ERROR_DIR_NOT_EMPTY for windows (directory not empty).
func isSysErrNotEmpty(err error) bool {
Expand Down Expand Up @@ -659,7 +622,6 @@ func (f *fsClient) listPrefixes(prefix string, contentCh chan<- *clientContent)
}
return
}
pathURL := *f.PathURL
for _, fi := range files {
// Skip ignored files.
if isIgnoredFile(fi.Name()) {
Expand All @@ -670,36 +632,8 @@ func (f *fsClient) listPrefixes(prefix string, contentCh chan<- *clientContent)
if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
st, e := os.Stat(file)
if e != nil {
if os.IsPermission(e) {
contentCh <- &clientContent{
Err: probe.NewError(PathInsufficientPermission{
Path: pathURL.Path,
}),
}
continue
}
if os.IsNotExist(e) {
contentCh <- &clientContent{
Err: probe.NewError(BrokenSymlink{
Path: pathURL.Path,
}),
}
continue
}
if strings.Contains(e.Error(), "too many levels of symbolic links") {
contentCh <- &clientContent{
Err: probe.NewError(TooManyLevelsSymlink{
Path: pathURL.Path,
}),
}
continue
}
if e != nil {
contentCh <- &clientContent{
Err: probe.NewError(e),
}
continue
}
// Ignore any errors on symlink
continue
}
if strings.HasPrefix(file, prefix) {
contentCh <- &clientContent{
Expand Down Expand Up @@ -765,34 +699,8 @@ func (f *fsClient) listInRoutine(contentCh chan<- *clientContent, isMetadata boo
if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
fp := filepath.Join(fpath, fi.Name())
fi, e = os.Stat(fp)
if os.IsPermission(e) {
contentCh <- &clientContent{
Err: probe.NewError(PathInsufficientPermission{Path: pathURL.Path}),
}
continue
}
if os.IsNotExist(e) {
// Lstat makes no attempt to follow the broken link.
_, e = os.Lstat(fp)
contentCh <- &clientContent{
URL: pathURL,
Size: -1,
Err: probe.NewError(e),
}
continue
}
if e != nil {
if strings.Contains(e.Error(), "too many levels of symbolic links") {
contentCh <- &clientContent{
Err: probe.NewError(TooManyLevelsSymlink{
Path: pathURL.Path,
}),
}
} else {
contentCh <- &clientContent{
Err: probe.NewError(e),
}
}
// Ignore all errors on symlinks
continue
}
}
Expand Down Expand Up @@ -960,30 +868,8 @@ func (f *fsClient) listRecursiveInRoutine(contentCh chan *clientContent, isMetad
if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
fi, e = os.Stat(fp)
if e != nil {
if os.IsPermission(e) {
contentCh <- &clientContent{
Err: probe.NewError(e),
}
return nil
}
if os.IsNotExist(e) {
// Lstat makes no attempt to follow the broken link.
_, e = os.Lstat(fp)
contentCh <- &clientContent{
URL: *newClientURL(fp),
Size: -1,
Err: probe.NewError(e),
}
return nil
}
if e != nil {
if strings.Contains(e.Error(), "too many levels of symbolic links") {
contentCh <- &clientContent{
Err: probe.NewError(TooManyLevelsSymlink{Path: fp}),
}
}
}
return e
// Ignore any errors for symlink
return nil
}
}
if fi.Mode().IsRegular() {
Expand Down Expand Up @@ -1173,6 +1059,9 @@ func (f *fsClient) toClientError(e error, fpath string) *probe.Error {
if os.IsNotExist(e) {
return probe.NewError(PathNotFound{Path: fpath})
}
if errors.Is(e, syscall.ELOOP) {
return probe.NewError(TooManyLevelsSymlink{Path: fpath})
}
return probe.NewError(e)
}

Expand All @@ -1191,38 +1080,12 @@ func (f *fsClient) fsStat(isIncomplete bool) (os.FileInfo, *probe.Error) {
fpath += partSuffix
}

// Golang strips trailing / if you clean(..) or
// EvalSymlinks(..). Adding '.' prevents it from doing so.
if strings.HasSuffix(fpath, string(f.PathURL.Separator)) {
fpath = fpath + "."
}
fpath, e = filepath.EvalSymlinks(fpath)
if e != nil {
if os.IsPermission(e) {
return nil, probe.NewError(PathInsufficientPermission{Path: f.PathURL.Path})
}
if strings.Contains(e.Error(), "too many levels of symbolic links") {
return nil, probe.NewError(TooManyLevelsSymlink{Path: f.PathURL.Path})
}
err := f.toClientError(e, f.PathURL.Path)
return nil, err.Trace(fpath)
}

st, e = os.Stat(fpath)
if e != nil {
if os.IsPermission(e) {
return nil, probe.NewError(PathInsufficientPermission{Path: f.PathURL.Path})
}
if os.IsNotExist(e) {
return nil, probe.NewError(PathNotFound{Path: f.PathURL.Path})
}
if strings.Contains(e.Error(), "too many levels of symbolic links") {
return nil, probe.NewError(TooManyLevelsSymlink{Path: f.PathURL.Path})
}
return nil, probe.NewError(e)
return nil, f.toClientError(e, fpath)
}
return st, nil
}

func (f *fsClient) AddUserAgent(app, version string) {
func (f *fsClient) AddUserAgent(_, _ string) {
}
2 changes: 1 addition & 1 deletion cmd/difference.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func difference(sourceClnt, targetClnt Client, sourceURL, targetURL string, isMe
if err != nil {
// handle this specifically for filesystem related errors.
switch err.ToGoError().(type) {
case BrokenSymlink, TooManyLevelsSymlink, PathNotFound, PathInsufficientPermission:
case PathNotFound, PathInsufficientPermission:
diffCh <- diffMessage{
Error: err,
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/mattn/go-runewidth v0.0.5 // indirect
github.com/minio/cli v1.22.0
github.com/minio/minio v0.0.0-20200118222113-b849fd7a756d
github.com/minio/minio-go/v6 v6.0.45-0.20200117140906-66cf57d21ba4
github.com/minio/minio-go/v6 v6.0.45-0.20200122224049-9d882c9f270a
github.com/minio/sha256-simd v0.1.1
github.com/mitchellh/go-homedir v1.1.0
github.com/pkg/profile v1.3.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ github.com/minio/minio v0.0.0-20200118222113-b849fd7a756d/go.mod h1:f8MCueGjpiby
github.com/minio/minio-go/v6 v6.0.44/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg=
github.com/minio/minio-go/v6 v6.0.45-0.20200117140906-66cf57d21ba4 h1:zjzoKgiRJm7k0J+IXz36u+ltbbU/iRTBlbm5/vZFt1w=
github.com/minio/minio-go/v6 v6.0.45-0.20200117140906-66cf57d21ba4/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg=
github.com/minio/minio-go/v6 v6.0.45-0.20200122224049-9d882c9f270a h1:maMiFcn7cmW9OOgevMH8oB08LuFo4vzMPk7rFai/NMw=
github.com/minio/minio-go/v6 v6.0.45-0.20200122224049-9d882c9f270a/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg=
github.com/minio/parquet-go v0.0.0-20191231003236-20b3c07bcd2c h1:PXr/55MEoFh0G6NeV0plWrdpTDr5xaDTxP3SpavSdGc=
github.com/minio/parquet-go v0.0.0-20191231003236-20b3c07bcd2c/go.mod h1:sl82d+TnCE7qeaNJazHdNoG9Gpyl9SZYfleDAQWrsls=
github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
Expand Down

0 comments on commit 96f595f

Please sign in to comment.