15836: Substitute configured string for "/" in WebDAV listings.
authorTom Clegg <tom@tomclegg.ca>
Tue, 7 Jan 2020 15:28:03 +0000 (10:28 -0500)
committerTom Clegg <tom@tomclegg.ca>
Tue, 7 Jan 2020 15:28:03 +0000 (10:28 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

sdk/go/arvados/fs_project.go
sdk/go/arvados/fs_project_test.go
sdk/go/arvados/fs_site.go
sdk/python/tests/run_test_server.py
services/keep-web/handler.go
services/keep-web/handler_test.go

index 92995510c7eaefaa4de69ea42f6c5f050e6828bd..c5eb03360a3877b577168611a8e579329b6abfa8 100644 (file)
@@ -30,16 +30,31 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in
        }
 
        var contents CollectionList
-       err = fs.RequestAndDecode(&contents, "GET", "arvados/v1/groups/"+uuid+"/contents", nil, ResourceListParams{
-               Count: "none",
-               Filters: []Filter{
-                       {"name", "=", name},
-                       {"uuid", "is_a", []string{"arvados#collection", "arvados#group"}},
-                       {"groups.group_class", "=", "project"},
-               },
-       })
-       if err != nil {
-               return nil, err
+       for _, subst := range []string{"/", fs.forwardSlashNameSubstitution} {
+               contents = CollectionList{}
+               err = fs.RequestAndDecode(&contents, "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"},
+                       },
+               })
+               if err != nil {
+                       return nil, err
+               }
+               if len(contents.Items) > 0 || fs.forwardSlashNameSubstitution == "/" || fs.forwardSlashNameSubstitution == "" || !strings.Contains(name, fs.forwardSlashNameSubstitution) {
+                       break
+               }
+               // If the requested name contains the configured "/"
+               // replacement string and didn't match a
+               // project/collection exactly, we'll try again with
+               // "/" in its place, so a lookup of a munged name
+               // works regardless of whether the directory listing
+               // has been populated with escaped names.
+               //
+               // Note this doesn't handle items whose names contain
+               // both "/" and the substitution string.
        }
        if len(contents.Items) == 0 {
                return nil, os.ErrNotExist
@@ -86,6 +101,9 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
                }
                for _, i := range resp.Items {
                        coll := i
+                       if fs.forwardSlashNameSubstitution != "" {
+                               coll.Name = strings.Replace(coll.Name, "/", fs.forwardSlashNameSubstitution, -1)
+                       }
                        if !permittedName(coll.Name) {
                                continue
                        }
@@ -106,6 +124,9 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
                        break
                }
                for _, group := range resp.Items {
+                       if fs.forwardSlashNameSubstitution != "" {
+                               group.Name = strings.Replace(group.Name, "/", fs.forwardSlashNameSubstitution, -1)
+                       }
                        if !permittedName(group.Name) {
                                continue
                        }
index 91b8222cdc631d6ac80bf005c3f07c84fe641b92..5628dcc9c43b42ef6560ddfa1eba2bc0482001d3 100644 (file)
@@ -147,6 +147,21 @@ func (s *SiteFSSuite) TestSlashInName(c *check.C) {
                c.Logf("fi.Name() == %q", fi.Name())
                c.Check(strings.Contains(fi.Name(), "/"), check.Equals, false)
        }
+
+       // Make a new fs (otherwise content will still be cached from
+       // above) and enable "/" replacement string.
+       s.fs = s.client.SiteFileSystem(s.kc)
+       s.fs.ForwardSlashNameSubstitution("___")
+       dir, err = s.fs.Open("/users/active/A Project/bad___collection")
+       if c.Check(err, check.IsNil) {
+               _, err = dir.Readdir(-1)
+               c.Check(err, check.IsNil)
+       }
+       dir, err = s.fs.Open("/users/active/A Project/bad___project")
+       if c.Check(err, check.IsNil) {
+               _, err = dir.Readdir(-1)
+               c.Check(err, check.IsNil)
+       }
 }
 
 func (s *SiteFSSuite) TestProjectUpdatedByOther(c *check.C) {
index 4264be4fa600be7abb05351f506feb00dccac6cc..7826d335c81fa93cfe54bf39a81a66335a65d336 100644 (file)
@@ -16,6 +16,7 @@ type CustomFileSystem interface {
        MountByID(mount string)
        MountProject(mount, uuid string)
        MountUsers(mount string)
+       ForwardSlashNameSubstitution(string)
 }
 
 type customFileSystem struct {
@@ -25,6 +26,8 @@ type customFileSystem struct {
 
        staleThreshold time.Time
        staleLock      sync.Mutex
+
+       forwardSlashNameSubstitution string
 }
 
 func (c *Client) CustomFileSystem(kc keepClient) CustomFileSystem {
@@ -94,6 +97,10 @@ func (fs *customFileSystem) MountUsers(mount string) {
        })
 }
 
+func (fs *customFileSystem) ForwardSlashNameSubstitution(repl string) {
+       fs.forwardSlashNameSubstitution = repl
+}
+
 // SiteFileSystem returns a FileSystem that maps collections and other
 // Arvados objects onto a filesystem layout.
 //
index 2e093b396dfa22991332368db61a8e9f5904a5d5..9e9b12f98ca6d09ed1fb65eb5c768acf23454bf5 100644 (file)
@@ -738,6 +738,7 @@ def setup_config():
                 "Collections": {
                     "BlobSigningKey": "zfhgfenhffzltr9dixws36j1yhksjoll2grmku38mi7yxd66h5j4q9w4jzanezacp8s6q0ro3hxakfye02152hncy6zml2ed0uc",
                     "TrustAllContent": True,
+                    "ForwardSlashNameSubstitution": "/",
                 },
                 "Git": {
                     "Repositories": "%s/test" % os.path.join(SERVICES_SRC_DIR, 'api', 'tmp', 'git'),
index bbbbd8f97bfce63a0e22da02cc5fbe32a1e5794e..563a59df014b8f642b545a68bc23f2e72e1a57d1 100644 (file)
@@ -537,6 +537,7 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s
                Insecure:  arv.ApiInsecure,
        }).WithRequestID(r.Header.Get("X-Request-Id"))
        fs := client.SiteFileSystem(kc)
+       fs.ForwardSlashNameSubstitution(h.Config.cluster.Collections.ForwardSlashNameSubstitution)
        f, err := fs.Open(r.URL.Path)
        if os.IsNotExist(err) {
                http.Error(w, err.Error(), http.StatusNotFound)
index ec3df9e298e1798f5f8f440b5ce2a1bf4ab51164..7d820c8a0adb61b97befe8d02477fb34b31615f4 100644 (file)
@@ -520,6 +520,56 @@ func (s *IntegrationSuite) TestSpecialCharsInPath(c *check.C) {
        c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./https:%5c%22odd%27%20path%20chars"\S+https:\\&#34;odd&#39; path chars.*`)
 }
 
+func (s *IntegrationSuite) TestForwardSlashSubstitution(c *check.C) {
+       arv := arvados.NewClientFromEnv()
+       s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com"
+       s.testServer.Config.cluster.Collections.ForwardSlashNameSubstitution = "{SOLIDUS}"
+       name := "foo/bar/baz"
+       nameShown := strings.Replace(name, "/", "{SOLIDUS}", -1)
+       nameShownEscaped := strings.Replace(name, "/", "%7bSOLIDUS%7d", -1)
+
+       client := s.testServer.Config.Client
+       client.AuthToken = arvadostest.ActiveToken
+       fs, err := (&arvados.Collection{}).FileSystem(&client, nil)
+       c.Assert(err, check.IsNil)
+       f, err := fs.OpenFile("filename", os.O_CREATE, 0777)
+       c.Assert(err, check.IsNil)
+       f.Close()
+       mtxt, err := fs.MarshalManifest(".")
+       c.Assert(err, check.IsNil)
+       var coll arvados.Collection
+       err = client.RequestAndDecode(&coll, "POST", "arvados/v1/collections", nil, map[string]interface{}{
+               "collection": map[string]string{
+                       "manifest_text": mtxt,
+                       "name":          name,
+                       "owner_uuid":    arvadostest.AProjectUUID,
+               },
+       })
+       c.Assert(err, check.IsNil)
+       defer arv.RequestAndDecode(&coll, "DELETE", "arvados/v1/collections/"+coll.UUID, nil, nil)
+
+       base := "http://download.example.com/by_id/" + coll.OwnerUUID + "/"
+       for tryURL, expectRegexp := range map[string]string{
+               base:                   `(?ms).*href="./` + nameShownEscaped + `/"\S+` + nameShown + `.*`,
+               base + nameShown + "/": `(?ms).*href="./filename"\S+filename.*`,
+       } {
+               u, _ := url.Parse(tryURL)
+               req := &http.Request{
+                       Method:     "GET",
+                       Host:       u.Host,
+                       URL:        u,
+                       RequestURI: u.RequestURI(),
+                       Header: http.Header{
+                               "Authorization": {"Bearer " + client.AuthToken},
+                       },
+               }
+               resp := httptest.NewRecorder()
+               s.testServer.Handler.ServeHTTP(resp, req)
+               c.Check(resp.Code, check.Equals, http.StatusOK)
+               c.Check(resp.Body.String(), check.Matches, expectRegexp)
+       }
+}
+
 // XHRs can't follow redirect-with-cookie so they rely on method=POST
 // and disposition=attachment (telling us it's acceptable to respond
 // with content instead of a redirect) and an Origin header that gets