Skip to content

Commit

Permalink
Add error handling for invalid references (#1216)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ishank011
ishank011 authored Oct 2, 2020
1 parent 5831dc0 commit ea47561
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 61 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/ref-errors.md
View file
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
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
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
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

0 comments on commit ea47561

Please sign in to comment.