[arvados] created: 2.7.0-5594-g5eefda675a

git repository hosting git at public.arvados.org
Mon Dec 11 15:57:24 UTC 2023


        at  5eefda675a92bba1e1239d43418c30eff1896d6f (commit)


commit 5eefda675a92bba1e1239d43418c30eff1896d6f
Author: Tom Clegg <tom at curii.com>
Date:   Sat Dec 9 15:56:10 2023 -0500

    21214: Prevent filter groups from matching other/same filter groups.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/group.go b/lib/controller/localdb/group.go
index 418fd6b8b7..a21a062db8 100644
--- a/lib/controller/localdb/group.go
+++ b/lib/controller/localdb/group.go
@@ -97,6 +97,12 @@ func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupConten
 				filter.Operand = tmp[2]
 				options.Filters = append(options.Filters, filter)
 			}
+			// Prevent infinite-depth trees caused by a
+			// filter group whose filters match [other
+			// filter groups that match] itself. Such
+			// cycles are supported by sitefs, but they
+			// cause havoc in webdav. See #21214.
+			options.Filters = append(options.Filters, arvados.Filter{"groups.group_class", "=", "project"})
 		} else {
 			return resp, fmt.Errorf("filter unparsable: not an array\n")
 		}

commit 0542eb85c46bf04c4139d6a976d49860f5f7d5e6
Author: Tom Clegg <tom at curii.com>
Date:   Sat Dec 9 15:55:55 2023 -0500

    21214: Implement filter groups in sitefs as symlinks to /by_id/.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/mount/fs.go b/lib/mount/fs.go
index 3c2e628d01..70d02f8134 100644
--- a/lib/mount/fs.go
+++ b/lib/mount/fs.go
@@ -274,6 +274,8 @@ func (fs *keepFS) fillStat(stat *fuse.Stat_t, fi os.FileInfo) {
 	var m uint32
 	if fi.IsDir() {
 		m = m | fuse.S_IFDIR
+	} else if fi.Mod()&os.ModeSymlink != 0 {
+		m = m | fuse.S_IFLNK
 	} else {
 		m = m | fuse.S_IFREG
 	}
@@ -296,6 +298,15 @@ func (fs *keepFS) fillStat(stat *fuse.Stat_t, fi os.FileInfo) {
 	}
 }
 
+func (fs *keepFS) Readlink(path string) (n int, target string) {
+	defer fs.debugPanics()
+	target, err := fs.root.Readlink(path)
+	if err != nil {
+		return fs.errCode(err), ""
+	}
+	return 0, target
+}
+
 func (fs *keepFS) Write(path string, buf []byte, ofst int64, fh uint64) (n int) {
 	defer fs.debugPanics()
 	if fs.ReadOnly {
diff --git a/sdk/go/arvados/fs_base.go b/sdk/go/arvados/fs_base.go
index 274d207022..e6070ef283 100644
--- a/sdk/go/arvados/fs_base.go
+++ b/sdk/go/arvados/fs_base.go
@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -117,8 +118,9 @@ type FileSystem interface {
 	// create a new node with nil parent.
 	newNode(name string, perm os.FileMode, modTime time.Time) (node inode, err error)
 
-	// analogous to os.Stat()
+	// analogous to os.Stat() and os.Lstat()
 	Stat(name string) (os.FileInfo, error)
+	Lstat(name string) (os.FileInfo, error)
 
 	// analogous to os.Create(): create/truncate a file and open it O_RDWR.
 	Create(name string) (File, error)
@@ -142,6 +144,7 @@ type FileSystem interface {
 	Remove(name string) error
 	RemoveAll(name string) error
 	Rename(oldname, newname string) error
+	Readlink(name string) (string, error)
 
 	// Write buffered data from memory to storage, returning when
 	// all updates have been saved to persistent storage.
@@ -387,10 +390,11 @@ func (n *treenode) Size() int64 {
 }
 
 func (n *treenode) FileInfo() os.FileInfo {
-	n.Lock()
-	defer n.Unlock()
-	n.fileinfo.size = int64(len(n.inodes))
-	return n.fileinfo
+	n.RLock()
+	defer n.RUnlock()
+	fi := n.fileinfo
+	fi.size = int64(len(n.inodes))
+	return fi
 }
 
 func (n *treenode) Readdir() (fi []os.FileInfo, err error) {
@@ -467,6 +471,24 @@ func (fs *fileSystem) openFile(name string, flag int, perm os.FileMode) (*fileha
 	if flag&os.O_SYNC != 0 {
 		return nil, ErrSyncNotSupported
 	}
+	if node, _ := rlookupNoCycle(fs.root, name, map[inode]bool{}, false); node != nil && node.FileInfo().Mode()&os.ModeSymlink != 0 {
+		// Currently the only supported symlinks are ones that
+		// point to a /by_id/{uuid} directory, so here we only
+		// need to deal with that case.
+		if flag != os.O_RDONLY {
+			return nil, ErrInvalidArgument
+		}
+		node, err := rlookup(fs.root, name)
+		if err != nil {
+			return nil, err
+		}
+		return &filehandle{
+			inode:    node,
+			append:   false,
+			readable: true,
+			writable: false,
+		}, nil
+	}
 	dirname, name := path.Split(name)
 	parent, err := rlookup(fs.root, dirname)
 	if err != nil {
@@ -582,6 +604,43 @@ func (fs *fileSystem) Stat(name string) (os.FileInfo, error) {
 	return node.FileInfo(), nil
 }
 
+func (fs *fileSystem) Lstat(name string) (os.FileInfo, error) {
+	node, err := rlookupNoCycle(fs.root, name, map[inode]bool{}, false)
+	if err != nil {
+		return nil, err
+	}
+	return node.FileInfo(), nil
+}
+
+func (fs *fileSystem) Readlink(name string) (string, error) {
+	node, err := rlookupNoCycle(fs.root, name, map[inode]bool{}, false)
+	if err != nil {
+		return "", err
+	}
+	if node.FileInfo().Mode()&os.ModeSymlink == 0 {
+		return "", ErrInvalidArgument
+	}
+	target, err := fs.readlink(node)
+	if err != nil {
+		return "", nil
+	}
+	// Returned symlinks must be relative.  Internally, a filter
+	// group in "/users/active/fgname" is a symlink to
+	// "/by_id/{fguuid}"; we expose that externally as a symlink
+	// to "../../by_id/{fguuid}".
+	dirname, _ := filepath.Split(name)
+	return filepath.Rel("/"+dirname, target)
+}
+
+func (fs *fileSystem) readlink(node inode) (string, error) {
+	gn, ok := node.(*getternode)
+	if !ok {
+		return "", fmt.Errorf("unimplemented symlink type %T", node)
+	}
+	target, err := gn.Getter()
+	return string(target), err
+}
+
 func (fs *fileSystem) Rename(oldname, newname string) error {
 	olddir, oldname := path.Split(oldname)
 	if oldname == "" || oldname == "." || oldname == ".." {
@@ -742,8 +801,13 @@ func (fs *fileSystem) MemorySize() int64 {
 // with the given name (which may contain "/" separators). If no such
 // file/directory exists, the returned node is nil.
 func rlookup(start inode, path string) (node inode, err error) {
+	return rlookupNoCycle(start, path, map[inode]bool{}, true)
+}
+
+func rlookupNoCycle(start inode, path string, followed map[inode]bool, resolveSymlink bool) (node inode, err error) {
 	node = start
-	for _, name := range strings.Split(path, "/") {
+	names := strings.Split(path, "/")
+	for idx, name := range names {
 		if node.IsDir() {
 			if name == "." || name == "" {
 				continue
@@ -753,14 +817,32 @@ func rlookup(start inode, path string) (node inode, err error) {
 				continue
 			}
 		}
-		node, err = func() (inode, error) {
-			node.Lock()
-			defer node.Unlock()
-			return node.Child(name, nil)
-		}()
+		node.Lock()
+		child, err := node.Child(name, nil)
+		node.Unlock()
+		node = child
 		if node == nil || err != nil {
 			break
 		}
+		if node.FileInfo().Mode()&os.ModeSymlink != 0 && (idx < len(names)-1 || resolveSymlink) {
+			if followed[node] {
+				return nil, fmt.Errorf("symlink cycle detected in path %q at %q (%d)", path, node.FileInfo().Name(), idx)
+			}
+			gn, ok := node.(*getternode)
+			if !ok {
+				return nil, fmt.Errorf("unimplemented symlink type %T", node)
+			}
+			target, err := gn.Getter()
+			if err != nil {
+				return nil, err
+			}
+			followed[node] = true
+			if len(target) > 0 && target[0] == '/' {
+				node, err = rlookupNoCycle(start, string(target), followed, resolveSymlink)
+			} else {
+				node, err = rlookupNoCycle(node, string(target), followed, resolveSymlink)
+			}
+		}
 	}
 	if node == nil && err == nil {
 		err = os.ErrNotExist
diff --git a/sdk/go/arvados/fs_getternode.go b/sdk/go/arvados/fs_getternode.go
index ddff0c52e7..42a47eedaf 100644
--- a/sdk/go/arvados/fs_getternode.go
+++ b/sdk/go/arvados/fs_getternode.go
@@ -51,8 +51,9 @@ func (gn *getternode) FileInfo() os.FileInfo {
 		size = gn.data.Size()
 	}
 	return fileinfo{
+		name:    gn.treenode.fileinfo.name,
 		modTime: time.Now(),
-		mode:    0444,
+		mode:    0444 | gn.treenode.fileinfo.mode,
 		size:    size,
 	}
 }
diff --git a/sdk/go/arvados/fs_lookup.go b/sdk/go/arvados/fs_lookup.go
index 2bb09995e1..c5d239a8af 100644
--- a/sdk/go/arvados/fs_lookup.go
+++ b/sdk/go/arvados/fs_lookup.go
@@ -42,30 +42,38 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
 	ln.Lock()
 	checkTime := time.Now()
 	if ln.stale(ln.staleAll) {
-		all, err := ln.loadAll(ln)
+		err := ln.refreshAll(checkTime)
 		if err != nil {
 			ln.Unlock()
 			return nil, err
 		}
-		for _, child := range all {
-			_, err = ln.treenode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
-				return child, nil
-			})
-			if err != nil {
-				ln.Unlock()
-				return nil, err
-			}
-		}
-		ln.staleAll = checkTime
-		// No value in ln.staleOne can make a difference to an
-		// "entry is stale?" test now, because no value is
-		// newer than ln.staleAll. Reclaim memory.
-		ln.staleOne = nil
 	}
 	ln.Unlock()
 	return ln.treenode.Readdir()
 }
 
+// Caller must have lock.
+func (ln *lookupnode) refreshAll(checkTime time.Time) error {
+	all, err := ln.loadAll(ln)
+	if err != nil {
+		return err
+	}
+	for _, child := range all {
+		_, err = ln.treenode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
+			return child, nil
+		})
+		if err != nil {
+			return err
+		}
+	}
+	ln.staleAll = checkTime
+	// No value in ln.staleOne can make a difference to an "entry
+	// is stale?" test now, because no value is newer than
+	// ln.staleAll. Reclaim memory.
+	ln.staleOne = nil
+	return nil
+}
+
 // Child rejects (with ErrInvalidOperation) calls to add/replace
 // children, instead calling loadOne when a non-existing child is
 // looked up.
@@ -75,6 +83,10 @@ func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (in
 	var err error
 	if ln.stale(ln.staleAll) && ln.stale(ln.staleOne[name]) {
 		existing, err = ln.treenode.Child(name, func(inode) (inode, error) {
+			if ln.loadOne == nil {
+				err = ln.refreshAll(checkTime)
+				return ln.treenode.inodes[name], err
+			}
 			return ln.loadOne(ln, name)
 		})
 		if err == nil && existing != nil {
diff --git a/sdk/go/arvados/fs_project.go b/sdk/go/arvados/fs_project.go
index a68e83945e..ac1c01b591 100644
--- a/sdk/go/arvados/fs_project.go
+++ b/sdk/go/arvados/fs_project.go
@@ -23,6 +23,20 @@ func (fs *customFileSystem) defaultUUID(uuid string) (string, error) {
 	return resp.UUID, nil
 }
 
+// The groups content endpoint returns Collection and Group (project)
+// objects. This struct lets us load the common Items fields for both
+// types (UUID, Name, ModifiedAt, and Properties), and GroupClass for
+// groups, into one struct.
+type groupContentsResponse struct {
+	Items []struct {
+		UUID       string                 `json:"uuid"`
+		Name       string                 `json:"name"`
+		ModifiedAt time.Time              `json:"modified_at"`
+		GroupClass string                 `json:"group_class"`
+		Properties map[string]interface{} `json:"properties"`
+	}
+}
+
 // loadOneChild loads only the named child, if it exists.
 func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (inode, error) {
 	uuid, err := fs.defaultUUID(uuid)
@@ -30,22 +44,22 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in
 		return nil, err
 	}
 
-	var contents CollectionList
+	var resp groupContentsResponse
 	for _, subst := range []string{"/", fs.forwardSlashNameSubstitution} {
-		contents = CollectionList{}
-		err = fs.RequestAndDecode(&contents, "GET", "arvados/v1/groups/"+uuid+"/contents", nil, ResourceListParams{
+		resp = groupContentsResponse{}
+		err = fs.RequestAndDecode(&resp, "GET", "arvados/v1/groups/"+uuid+"/contents", nil, ResourceListParams{
 			Count: "none",
 			Filters: []Filter{
 				{"name", "=", strings.Replace(name, subst, "/", -1)},
 				{"uuid", "is_a", []string{"arvados#collection", "arvados#group"}},
-				{"groups.group_class", "=", "project"},
+				{"groups.group_class", "in", []string{"project", "filter"}},
 			},
-			Select: []string{"uuid", "name", "modified_at", "properties"},
+			Select: []string{"uuid", "name", "modified_at", "properties", "group_class"},
 		})
 		if err != nil {
 			return nil, err
 		}
-		if len(contents.Items) > 0 || fs.forwardSlashNameSubstitution == "/" || fs.forwardSlashNameSubstitution == "" || !strings.Contains(name, fs.forwardSlashNameSubstitution) {
+		if len(resp.Items) > 0 || fs.forwardSlashNameSubstitution == "/" || fs.forwardSlashNameSubstitution == "" || !strings.Contains(name, fs.forwardSlashNameSubstitution) {
 			break
 		}
 		// If the requested name contains the configured "/"
@@ -58,28 +72,28 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in
 		// Note this doesn't handle items whose names contain
 		// both "/" and the substitution string.
 	}
-	if len(contents.Items) == 0 {
+	if len(resp.Items) == 0 {
 		return nil, nil
 	}
-	coll := contents.Items[0]
-
-	if strings.Contains(coll.UUID, "-j7d0g-") {
-		// Group item was loaded into a Collection var -- but
-		// we only need the Name and UUID anyway, so it's OK.
+	item := resp.Items[0]
+	isGroup := strings.Contains(item.UUID, "-j7d0g-")
+	if strings.Contains(item.UUID, "-4zz18-") {
+		return fs.newDeferredCollectionDir(parent, name, item.UUID, item.ModifiedAt, item.Properties), nil
+	} else if isGroup && item.GroupClass == "filter" {
+		return fs.newCollectionOrProjectSymlink(parent, name, item.UUID, item.ModifiedAt, item.Properties)
+	} else if isGroup && item.GroupClass == "project" {
 		return &hardlink{
-			inode: fs.projectSingleton(coll.UUID, &Group{
-				UUID:       coll.UUID,
-				Name:       coll.Name,
-				ModifiedAt: coll.ModifiedAt,
-				Properties: coll.Properties,
+			inode: fs.projectSingleton(item.UUID, &Group{
+				UUID:       item.UUID,
+				Name:       item.Name,
+				ModifiedAt: item.ModifiedAt,
+				Properties: item.Properties,
 			}),
 			parent: parent,
-			name:   coll.Name,
+			name:   item.Name,
 		}, nil
-	} else if strings.Contains(coll.UUID, "-4zz18-") {
-		return fs.newDeferredCollectionDir(parent, name, coll.UUID, coll.ModifiedAt, coll.Properties), nil
 	} else {
-		log.Printf("group contents: unrecognized UUID in response: %q", coll.UUID)
+		log.Printf("group contents: unrecognized UUID in response: %q", item.UUID)
 		return nil, ErrInvalidArgument
 	}
 }
@@ -104,27 +118,19 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
 			{"uuid", "is_a", class},
 		}
 		if class == "arvados#group" {
-			filters = append(filters, Filter{"group_class", "=", "project"})
+			filters = append(filters, Filter{"groups.group_class", "in", []string{"project", "filter"}})
 		}
 
 		params := ResourceListParams{
 			Count:   "none",
 			Filters: filters,
 			Order:   "uuid",
-			Select:  []string{"uuid", "name", "modified_at", "properties"},
+			Select:  []string{"uuid", "name", "modified_at", "properties", "group_class"},
 			Limit:   &pagesize,
 		}
 
 		for {
-			// The groups content endpoint returns
-			// Collection and Group (project)
-			// objects. This function only accesses the
-			// UUID, Name, and ModifiedAt fields. Both
-			// collections and groups have those fields,
-			// so it is easier to just treat the
-			// ObjectList that comes back as a
-			// CollectionList.
-			var resp CollectionList
+			var resp groupContentsResponse
 			err = fs.RequestAndDecode(&resp, "GET", "arvados/v1/groups/"+uuid+"/contents", nil, params)
 			if err != nil {
 				return nil, err
@@ -139,15 +145,24 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
 				if !permittedName(i.Name) {
 					continue
 				}
-				if strings.Contains(i.UUID, "-j7d0g-") {
+				isGroup := strings.Contains(i.UUID, "-j7d0g-")
+				if strings.Contains(i.UUID, "-4zz18-") {
+					inodes = append(inodes, fs.newDeferredCollectionDir(parent, i.Name, i.UUID, i.ModifiedAt, i.Properties))
+				} else if isGroup && i.GroupClass == "filter" {
+					inode, err := fs.newCollectionOrProjectSymlink(parent, i.Name, i.UUID, i.ModifiedAt, i.Properties)
+					if err != nil {
+						return nil, err
+					}
+					if inode != nil {
+						inodes = append(inodes, inode)
+					}
+				} else if isGroup && i.GroupClass == "project" {
 					inodes = append(inodes, fs.newProjectDir(parent, i.Name, i.UUID, &Group{
 						UUID:       i.UUID,
 						Name:       i.Name,
 						ModifiedAt: i.ModifiedAt,
 						Properties: i.Properties,
 					}))
-				} else if strings.Contains(i.UUID, "-4zz18-") {
-					inodes = append(inodes, fs.newDeferredCollectionDir(parent, i.Name, i.UUID, i.ModifiedAt, i.Properties))
 				} else {
 					log.Printf("group contents: unrecognized UUID in response: %q", i.UUID)
 					return nil, ErrInvalidArgument
@@ -159,6 +174,29 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
 	return inodes, nil
 }
 
+// create a symlink to the given collection or project. If it's not
+// possible to create a symlink because the filesystem does not have a
+// "by_id" mount point to put the target in, return (nil, nil).
+func (fs *customFileSystem) newCollectionOrProjectSymlink(parent inode, name, targetUUID string, modTime time.Time, props map[string]interface{}) (inode, error) {
+	fs.root.treenode.Lock()
+	byIDPath := fs.byIDPath
+	fs.root.treenode.Unlock()
+	if byIDPath == "" {
+		return nil, nil
+	}
+	targetPath := []byte("/" + byIDPath + "/" + targetUUID)
+	return &getternode{
+		Getter: func() ([]byte, error) { return targetPath, nil },
+		treenode: treenode{
+			fileinfo: fileinfo{
+				name:    name,
+				modTime: modTime,
+				mode:    os.ModeSymlink,
+			},
+		},
+	}, nil
+}
+
 func (fs *customFileSystem) newProjectDir(parent inode, name, uuid string, proj *Group) inode {
 	return &hardlink{inode: fs.projectSingleton(uuid, proj), parent: parent, name: name}
 }
diff --git a/sdk/go/arvados/fs_project_test.go b/sdk/go/arvados/fs_project_test.go
index d3dac7a14f..ffc439de49 100644
--- a/sdk/go/arvados/fs_project_test.go
+++ b/sdk/go/arvados/fs_project_test.go
@@ -51,7 +51,7 @@ func (s *SiteFSSuite) TestFilterGroup(c *check.C) {
 	c.Assert(err, check.IsNil)
 
 	_, err = s.fs.OpenFile("/fg/A Project", 0, 0)
-	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(err, check.Equals, os.ErrNotExist)
 
 	// An empty filter means everything that is visible should be returned.
 	s.fs.MountProject("fg2", fixtureAFilterGroupTwoUUID)
@@ -72,7 +72,7 @@ func (s *SiteFSSuite) TestFilterGroup(c *check.C) {
 	c.Assert(err, check.IsNil)
 
 	_, err = s.fs.OpenFile("/fg3/A Subproject", 0, 0)
-	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(err, check.Equals, os.ErrNotExist)
 
 	// An 'exists' 'arvados#collection' filter means only collections with certain properties should be returned.
 	s.fs.MountProject("fg4", fixtureAFilterGroupFourUUID)
@@ -93,10 +93,64 @@ func (s *SiteFSSuite) TestFilterGroup(c *check.C) {
 	c.Assert(err, check.IsNil)
 
 	_, err = s.fs.Stat("/fg5/collection with prop2 5")
-	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(err, check.Equals, os.ErrNotExist)
 
 	_, err = s.fs.Stat("/fg5/collection with list property with even values")
-	c.Assert(err, check.Not(check.IsNil))
+	c.Assert(err, check.Equals, os.ErrNotExist)
+}
+
+func (s *SiteFSSuite) TestFilterGroupByName(c *check.C) {
+	f, err := s.fs.Open("/users/active/This filter group/A Subproject")
+	if c.Check(err, check.IsNil) {
+		ents, err := f.Readdir(-1)
+		c.Check(err, check.IsNil)
+		c.Check(len(ents) > 0, check.Equals, true)
+		err = f.Close()
+		c.Check(err, check.IsNil)
+	}
+
+	f, err = s.fs.Open("/users/active/This filter group/A Project")
+	c.Check(err, check.Equals, os.ErrNotExist)
+
+	f, err = s.fs.Open("/users/active/This filter group")
+	if c.Check(err, check.IsNil) {
+		ents, err := f.Readdir(-1)
+		c.Check(err, check.IsNil)
+		c.Check(len(ents) > 0, check.Equals, true)
+		err = f.Close()
+		c.Check(err, check.IsNil)
+	}
+
+	fi, err := s.fs.Lstat("/users/active/This filter group")
+	if c.Check(err, check.IsNil) {
+		c.Check(fi.Mode()&os.ModeSymlink, check.Not(check.Equals), os.FileMode(0))
+	}
+
+	fi, err = s.fs.Lstat("/users/active/This filter group/.")
+	if c.Check(err, check.IsNil) {
+		c.Check(fi.Mode()&os.ModeSymlink, check.Equals, os.FileMode(0))
+	}
+
+	target, err := s.fs.Readlink("/users/active/This filter group/")
+	c.Check(err, check.NotNil)
+	target, err = s.fs.Readlink("/users/active/This filter group")
+	c.Check(err, check.IsNil)
+	c.Check(target, check.Equals, "../../by_id/"+fixtureThisFilterGroupUUID)
+	target, err = s.fs.Readlink("users/active/This filter group")
+	c.Check(err, check.IsNil)
+	c.Check(target, check.Equals, "../../by_id/"+fixtureThisFilterGroupUUID)
+
+	f, err = s.fs.Open("/users/active/This filter group/baz_file/baz")
+	if c.Check(err, check.IsNil) {
+		err = f.Close()
+		c.Check(err, check.IsNil)
+	}
+
+	f, err = s.fs.Open("/by_id/" + fixtureThisFilterGroupUUID + "/baz_file/baz")
+	if c.Check(err, check.IsNil) {
+		err = f.Close()
+		c.Check(err, check.IsNil)
+	}
 }
 
 func (s *SiteFSSuite) TestCurrentUserHome(c *check.C) {
diff --git a/sdk/go/arvados/fs_site.go b/sdk/go/arvados/fs_site.go
index a4a18837e0..50a012b53d 100644
--- a/sdk/go/arvados/fs_site.go
+++ b/sdk/go/arvados/fs_site.go
@@ -30,9 +30,16 @@ type customFileSystem struct {
 
 	forwardSlashNameSubstitution string
 
+	// invisible directory containing singleton
+	// projects/collections
 	byID     map[string]inode
 	byIDLock sync.Mutex
 	byIDRoot *treenode
+
+	// visible directory containing hardlinks to singleton
+	// projects/collections, typically "by_id" but can be empty if
+	// MountByID() has not been used.
+	byIDPath string
 }
 
 func (c *Client) CustomFileSystem(kc keepClient) CustomFileSystem {
@@ -72,6 +79,7 @@ func (c *Client) CustomFileSystem(kc keepClient) CustomFileSystem {
 func (fs *customFileSystem) MountByID(mount string) {
 	fs.root.treenode.Lock()
 	defer fs.root.treenode.Unlock()
+	fs.byIDPath = mount
 	fs.root.treenode.Child(mount, func(inode) (inode, error) {
 		return &vdirnode{
 			treenode: treenode{
@@ -386,3 +394,7 @@ func (hl *hardlink) FileInfo() os.FileInfo {
 	}
 	return fi
 }
+
+func (hl *hardlink) MemorySize() int64 {
+	return 64
+}
diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index ac12f7ae13..3b8a618fea 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -37,8 +37,9 @@ const (
 	StorageClassesDesiredArchiveConfirmedDefault = "zzzzz-4zz18-3t236wr12769qqa"
 	EmptyCollectionUUID                          = "zzzzz-4zz18-gs9ooj1h9sd5mde"
 
-	AProjectUUID    = "zzzzz-j7d0g-v955i6s2oi1cbso"
-	ASubprojectUUID = "zzzzz-j7d0g-axqo7eu9pwvna1x"
+	AProjectUUID     = "zzzzz-j7d0g-v955i6s2oi1cbso"
+	ASubprojectUUID  = "zzzzz-j7d0g-axqo7eu9pwvna1x"
+	AFilterGroupUUID = "zzzzz-j7d0g-thisfiltergroup"
 
 	FooAndBarFilesInDirUUID = "zzzzz-4zz18-foonbarfilesdir"
 	FooAndBarFilesInDirPDH  = "870369fc72738603c2fad16664e50e2d+58"
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index c14789889d..42bfd80fa5 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1105,6 +1105,17 @@ func (s *IntegrationSuite) TestDirectoryListingWithNoAnonymousToken(c *check.C)
 }
 
 func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
+	// The "ownership cycle" test fixtures are reachable from the
+	// "filter group without filters" group, causing webdav's
+	// walkfs to recurse indefinitely. Avoid that by deleting one
+	// of the bogus fixtures.
+	arv := arvados.NewClientFromEnv()
+	err := arv.RequestAndDecode(nil, "DELETE", "arvados/v1/groups/zzzzz-j7d0g-cx2al9cqkmsf1hs", nil, nil)
+	if err != nil {
+		c.Assert(err, check.FitsTypeOf, &arvados.TransactionError{})
+		c.Check(err.(*arvados.TransactionError).StatusCode, check.Equals, 404)
+	}
+
 	s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com"
 	authHeader := http.Header{
 		"Authorization": {"OAuth2 " + arvadostest.ActiveToken},
@@ -1241,6 +1252,30 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 			expect:  []string{"waz"},
 			cutDirs: 2,
 		},
+		{
+			uri:     "download.example.com/users/active/This filter group/",
+			header:  authHeader,
+			expect:  []string{"A Subproject/"},
+			cutDirs: 3,
+		},
+		{
+			uri:     "download.example.com/users/active/This filter group/A Subproject",
+			header:  authHeader,
+			expect:  []string{"baz_file/"},
+			cutDirs: 4,
+		},
+		{
+			uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID,
+			header:  authHeader,
+			expect:  []string{"A Subproject/"},
+			cutDirs: 2,
+		},
+		{
+			uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID + "/A Subproject",
+			header:  authHeader,
+			expect:  []string{"baz_file/"},
+			cutDirs: 3,
+		},
 	} {
 		comment := check.Commentf("HTML: %q => %q", trial.uri, trial.expect)
 		resp := httptest.NewRecorder()
@@ -1278,6 +1313,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 		} else {
 			c.Check(resp.Code, check.Equals, http.StatusOK, comment)
 			for _, e := range trial.expect {
+				e = strings.Replace(e, " ", "%20", -1)
 				c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./`+e+`".*`, comment)
 			}
 			c.Check(resp.Body.String(), check.Matches, `(?ms).*--cut-dirs=`+fmt.Sprintf("%d", trial.cutDirs)+` .*`, comment)
@@ -1320,6 +1356,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 				} else {
 					e = filepath.Join(u.Path, e)
 				}
+				e = strings.Replace(e, " ", "%20", -1)
 				c.Check(resp.Body.String(), check.Matches, `(?ms).*<D:href>`+e+`</D:href>.*`, comment)
 			}
 		}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list