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.

Sign up for GitHub

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

Jump to bottom

Add error handling for invalid references #1216

Merged
merged 1 commit into from
Oct 2, 2020
Merged

Add error handling for invalid references #1216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/unreleased/ref-errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Bugfix: Add error handling for invalid references

https://github.com/cs3org/reva/pull/1216
14 changes: 6 additions & 8 deletions internal/grpc/services/gateway/storageprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1626,12 +1626,10 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp

for i, ref := range lcr.Infos {
info, protocol, err := s.checkRef(ctx, ref)
if err != nil {
if _, ok := err.(errtypes.IsNotFound); ok {
return &provider.ListContainerResponse{
Status: status.NewNotFound(ctx, "gateway: reference not found:"+ref.Target),
}, nil
}
if _, ok := err.(errtypes.IsNotFound); ok {
// this might arise when the shared resource has been moved to the recycle bin
continue
} else if err != nil {
return &provider.ListContainerResponse{
Status: status.NewInternal(ctx, err, "gateway: error resolving reference:"+ref.Path),
}, nil
Expand All @@ -1640,8 +1638,8 @@ func (s *svc) listSharesFolder(ctx context.Context) (*provider.ListContainerResp
if protocol == "webdav" {
info, err = s.webdavRefStat(ctx, ref.Target)
if err != nil {
// Might be the case that the webdav token has expired. In that case, use the reference's info
info = ref
// Might be the case that the webdav token has expired
continue
}
}

Expand Down
51 changes: 37 additions & 14 deletions pkg/eosclient/eosclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,9 @@ func getFileFromVersionFolder(p string) string {

func parseRecycleList(raw string) ([]*DeletedEntry, error) {
entries := []*DeletedEntry{}
rawLines := strings.Split(raw, "\n")
rawLines := strings.FieldsFunc(raw, func(c rune) bool {
return c == '\n'
})
for _, rl := range rawLines {
if rl == "" {
continue
Expand All @@ -694,7 +696,9 @@ func parseRecycleList(raw string) ([]*DeletedEntry, error) {
// recycle=ls recycle-bin=/eos/backup/proc/recycle/ uid=gonzalhu gid=it size=0 deletion-time=1510823151 type=recursive-dir keylength.restore-path=45 restore-path=/eos/scratch/user/g/gonzalhu/.sys.v#.app.ico/ restore-key=0000000000a35100
// recycle=ls recycle-bin=/eos/backup/proc/recycle/ uid=gonzalhu gid=it size=381038 deletion-time=1510823151 type=file keylength.restore-path=36 restore-path=/eos/scratch/user/g/gonzalhu/app.ico restore-key=000000002544fdb3
func parseRecycleEntry(raw string) (*DeletedEntry, error) {
partsBySpace := strings.Split(raw, " ")
partsBySpace := strings.FieldsFunc(raw, func(c rune) bool {
return c == ' '
})
restoreKeyPair, partsBySpace := partsBySpace[len(partsBySpace)-1], partsBySpace[:len(partsBySpace)-1]
restorePathPair := strings.Join(partsBySpace[8:], " ")

Expand Down Expand Up @@ -739,7 +743,9 @@ func getMap(partsBySpace []string) map[string]string {

func (c *Client) parseFind(dirPath, raw string) ([]*FileInfo, error) {
finfos := []*FileInfo{}
rawLines := strings.Split(raw, "\n")
rawLines := strings.FieldsFunc(raw, func(c rune) bool {
return c == '\n'
})
for _, rl := range rawLines {
if rl == "" {
continue
Expand All @@ -760,12 +766,16 @@ func (c *Client) parseFind(dirPath, raw string) ([]*FileInfo, error) {
}

func (c Client) parseQuotaLine(line string) map[string]string {
partsBySpace := strings.Split(line, " ")
partsBySpace := strings.FieldsFunc(line, func(c rune) bool {
return c == ' '
})
m := getMap(partsBySpace)
return m
}
func (c *Client) parseQuota(path, raw string) (*QuotaInfo, error) {
rawLines := strings.Split(raw, "\n")
rawLines := strings.FieldsFunc(raw, func(c rune) bool {
return c == '\n'
})
for _, rl := range rawLines {
if rl == "" {
continue
Expand Down Expand Up @@ -817,7 +827,9 @@ func (c *Client) parseFileInfo(raw string) (*FileInfo, error) {
kv["file"] = strings.TrimSuffix(name, "/")

line = line[length+1:]
partsBySpace := strings.Split(line, " ") // we have [size=45 container=3 ...}
partsBySpace := strings.FieldsFunc(line, func(c rune) bool { // we have [size=45 container=3 ...}
return c == ' '
})
var previousXAttr = ""
for _, p := range partsBySpace {
partsByEqual := strings.Split(p, "=") // we have kv pairs like [size 14]
Expand Down Expand Up @@ -900,15 +912,26 @@ func (c *Client) mapToFileInfo(kv map[string]string) (*FileInfo, error) {
}
}

// mtime is split by a dot, we only take the first part, do we need subsec precision?
mtime := strings.Split(kv["mtime"], ".")
mtimesec, err := strconv.ParseUint(mtime[0], 10, 64)
if err != nil {
return nil, err
// look for the stime first as mtime is not updated for parent dirs; if that isn't set, we use mtime
var mtimesec, mtimenanos uint64
var mtimeSet bool
if val, ok := kv["stime"]; ok && val != "" {
stimeSplit := strings.Split(val, ".")
if mtimesec, err = strconv.ParseUint(stimeSplit[0], 10, 64); err == nil {
mtimeSet = true
}
if mtimenanos, err = strconv.ParseUint(stimeSplit[1], 10, 32); err != nil {
mtimeSet = false
}
}
mtimenanos, err := strconv.ParseUint(mtime[1], 10, 32)
if err != nil {
return nil, err
if !mtimeSet {
mtimeSplit := strings.Split(kv["mtime"], ".")
if mtimesec, err = strconv.ParseUint(mtimeSplit[0], 10, 64); err != nil {
return nil, err
}
if mtimenanos, err = strconv.ParseUint(mtimeSplit[1], 10, 32); err != nil {
return nil, err
}
}

isDir := false
Expand Down
102 changes: 63 additions & 39 deletions pkg/storage/utils/eosfs/eosfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,19 @@ func (fs *eosfs) wrap(ctx context.Context, fn string) (internal string) {
return
}

func (fs *eosfs) unwrap(ctx context.Context, internal string) (external string) {
func (fs *eosfs) unwrap(ctx context.Context, internal string) (string, error) {
log := appctx.GetLogger(ctx)
layout := fs.getLayout(ctx)
ns := fs.getNsMatch(internal, []string{fs.conf.Namespace, fs.conf.ShadowNamespace})
external = fs.unwrapInternal(ctx, ns, internal, layout)
ns, err := fs.getNsMatch(internal, []string{fs.conf.Namespace, fs.conf.ShadowNamespace})
if err != nil {
return "", err
}
external, err := fs.unwrapInternal(ctx, ns, internal, layout)
if err != nil {
return "", err
}
log.Debug().Msgf("eos: unwrap: internal=%s external=%s", internal, external)
return
return external, nil
}

func (fs *eosfs) getLayout(ctx context.Context) (layout string) {
Expand All @@ -277,7 +283,7 @@ func (fs *eosfs) getLayout(ctx context.Context) (layout string) {
return
}

func (fs *eosfs) getNsMatch(internal string, nss []string) string {
func (fs *eosfs) getNsMatch(internal string, nss []string) (string, error) {
var match string

for _, ns := range nss {
Expand All @@ -287,29 +293,29 @@ func (fs *eosfs) getNsMatch(internal string, nss []string) string {
}

if match == "" {
panic(fmt.Sprintf("eos: path is outside namespaces: path=%s namespaces=%+v", internal, nss))
return "", errtypes.NotFound(fmt.Sprintf("eos: path is outside namespaces: path=%s namespaces=%+v", internal, nss))
}

return match
return match, nil
}

func (fs *eosfs) unwrapInternal(ctx context.Context, ns, np, layout string) (external string) {
func (fs *eosfs) unwrapInternal(ctx context.Context, ns, np, layout string) (string, error) {
log := appctx.GetLogger(ctx)
trim := path.Join(ns, layout)

if !strings.HasPrefix(np, trim) {
panic("eos: resource is outside the directory of the logged-in user: internal=" + np + " trim=" + trim + " namespace=" + ns)
return "", errtypes.NotFound(fmt.Sprintf("eos: path is outside the directory of the logged-in user: internal=%s trim=%s namespace=%+v", np, trim, ns))
}

external = strings.TrimPrefix(np, trim)
external := strings.TrimPrefix(np, trim)

if external == "" {
external = "/"
}

log.Debug().Msgf("eos: unwrapInternal: trim=%s external=%s ns=%s np=%s", trim, external, ns, np)

return
return external, nil
}

// resolve takes in a request path or request id and returns the unwrappedNominal path.
Expand Down Expand Up @@ -347,7 +353,7 @@ func (fs *eosfs) getPath(ctx context.Context, u *userpb.User, id *provider.Resou
return "", errors.Wrap(err, "eos: error getting file info by inode")
}

return fs.unwrap(ctx, eosFileInfo.File), nil
return fs.unwrap(ctx, eosFileInfo.File)
}

func (fs *eosfs) isShareFolder(ctx context.Context, p string) bool {
Expand Down Expand Up @@ -387,7 +393,7 @@ func (fs *eosfs) GetPathByID(ctx context.Context, id *provider.ResourceId) (stri
return "", errors.Wrap(err, "eos: error getting file info by inode")
}

return fs.unwrap(ctx, eosFileInfo.File), nil
return fs.unwrap(ctx, eosFileInfo.File)
}

func (fs *eosfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) error {
Expand Down Expand Up @@ -594,8 +600,7 @@ func (fs *eosfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []st
return nil, err
}

fi := fs.convertToResourceInfo(ctx, eosFileInfo)
return fi, nil
return fs.convertToResourceInfo(ctx, eosFileInfo)
}

func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string) (*provider.ResourceInfo, error) {
Expand All @@ -618,9 +623,9 @@ func (fs *eosfs) getMDShareFolder(ctx context.Context, p string, mdKeys []string
// TODO(labkode): diff between root (dir) and children (ref)

if fs.isShareFolderRoot(ctx, p) {
return fs.convertToResourceInfo(ctx, eosFileInfo), nil
return fs.convertToResourceInfo(ctx, eosFileInfo)
}
return fs.convertToFileReference(ctx, eosFileInfo), nil
return fs.convertToFileReference(ctx, eosFileInfo)
}

func (fs *eosfs) ListFolder(ctx context.Context, ref *provider.Reference, mdKeys []string) ([]*provider.ResourceInfo, error) {
Expand Down Expand Up @@ -679,7 +684,9 @@ func (fs *eosfs) listWithNominalHome(ctx context.Context, p string) (finfos []*p
}
}

finfos = append(finfos, fs.convertToResourceInfo(ctx, eosFileInfo))
if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo); err != nil {
finfos = append(finfos, finfo)
}
}

return finfos, nil
Expand Down Expand Up @@ -733,9 +740,11 @@ func (fs *eosfs) listHome(ctx context.Context, home string) ([]*provider.Resourc
if hiddenReg.MatchString(base) {
continue
}
}

if finfo, err := fs.convertToResourceInfo(ctx, eosFileInfo); err != nil {
finfos = append(finfos, finfo)
}
finfos = append(finfos, fs.convertToResourceInfo(ctx, eosFileInfo))
}

}
Expand Down Expand Up @@ -769,8 +778,9 @@ func (fs *eosfs) listShareFolderRoot(ctx context.Context, p string) (finfos []*p
}
}

finfo := fs.convertToFileReference(ctx, eosFileInfo)
finfos = append(finfos, finfo)
if finfo, err := fs.convertToFileReference(ctx, eosFileInfo); err != nil {
finfos = append(finfos, finfo)
}
}

return finfos, nil
Expand Down Expand Up @@ -1181,8 +1191,9 @@ func (fs *eosfs) ListRevisions(ctx context.Context, ref *provider.Reference) ([]
}
revisions := []*provider.FileVersion{}
for _, eosRev := range eosRevisions {
rev := fs.convertToRevision(ctx, eosRev)
revisions = append(revisions, rev)
if rev, err := fs.convertToRevision(ctx, eosRev); err != nil {
revisions = append(revisions, rev)
}
}
return revisions, nil
}
Expand Down Expand Up @@ -1280,8 +1291,9 @@ func (fs *eosfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, erro
}

}
recycleItem := fs.convertToRecycleItem(ctx, entry)
recycleEntries = append(recycleEntries, recycleItem)
if recycleItem, err := fs.convertToRecycleItem(ctx, entry); err != nil {
recycleEntries = append(recycleEntries, recycleItem)
}
}
return recycleEntries, nil
}
Expand All @@ -1300,8 +1312,11 @@ func (fs *eosfs) RestoreRecycleItem(ctx context.Context, key string) error {
return fs.c.RestoreDeletedEntry(ctx, uid, gid, key)
}

func (fs *eosfs) convertToRecycleItem(ctx context.Context, eosDeletedItem *eosclient.DeletedEntry) *provider.RecycleItem {
path := fs.unwrap(ctx, eosDeletedItem.RestorePath)
func (fs *eosfs) convertToRecycleItem(ctx context.Context, eosDeletedItem *eosclient.DeletedEntry) (*provider.RecycleItem, error) {
path, err := fs.unwrap(ctx, eosDeletedItem.RestorePath)
if err != nil {
return nil, err
}
recycleItem := &provider.RecycleItem{
Path: path,
Key: eosDeletedItem.RestoreKey,
Expand All @@ -1314,36 +1329,45 @@ func (fs *eosfs) convertToRecycleItem(ctx context.Context, eosDeletedItem *eoscl
// TODO(labkode): if eos returns more types oin the future we need to map them.
recycleItem.Type = provider.ResourceType_RESOURCE_TYPE_FILE
}
return recycleItem
return recycleItem, nil
}

func (fs *eosfs) convertToRevision(ctx context.Context, eosFileInfo *eosclient.FileInfo) *provider.FileVersion {
md := fs.convertToResourceInfo(ctx, eosFileInfo)
func (fs *eosfs) convertToRevision(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.FileVersion, error) {
md, err := fs.convertToResourceInfo(ctx, eosFileInfo)
if err != nil {
return nil, err
}
revision := &provider.FileVersion{
Key: path.Base(md.Path),
Size: md.Size,
Mtime: md.Mtime.Seconds, // TODO do we need nanos here?
}
return revision
return revision, nil
}

func (fs *eosfs) convertToResourceInfo(ctx context.Context, eosFileInfo *eosclient.FileInfo) *provider.ResourceInfo {
func (fs *eosfs) convertToResourceInfo(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) {
return fs.convert(ctx, eosFileInfo)
}

func (fs *eosfs) convertToFileReference(ctx context.Context, eosFileInfo *eosclient.FileInfo) *provider.ResourceInfo {
info := fs.convert(ctx, eosFileInfo)
func (fs *eosfs) convertToFileReference(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) {
info, err := fs.convert(ctx, eosFileInfo)
if err != nil {
return nil, err
}
info.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE
val, ok := eosFileInfo.Attrs["user.reva.target"]
if !ok || val == "" {
panic("eos: reference does not contain target: target=" + val + " file=" + eosFileInfo.File)
return nil, errtypes.InternalError("eos: reference does not contain target: target=" + val + " file=" + eosFileInfo.File)
}
info.Target = val
return info
return info, nil
}

func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo) *provider.ResourceInfo {
path := fs.unwrap(ctx, eosFileInfo.File)
func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo) (*provider.ResourceInfo, error) {
path, err := fs.unwrap(ctx, eosFileInfo.File)
if err != nil {
return nil, err
}

size := eosFileInfo.Size
if eosFileInfo.IsDir {
Expand Down Expand Up @@ -1380,7 +1404,7 @@ func (fs *eosfs) convert(ctx context.Context, eosFileInfo *eosclient.FileInfo) *
}

info.Type = getResourceType(eosFileInfo.IsDir)
return info
return info, nil
}

func getResourceType(isDir bool) provider.ResourceType {
Expand Down