Bug #15836
closedEscape / convert forward slashes in collection names accessed via WebDAV
Added by Tom Morris almost 5 years ago. Updated over 4 years ago.
Description
We have a customer report that the Mountain Goat WebDAV client doesn't display collections which have slashes (/) in their names. That's not terribly surprising considering it's use as a path delimiter, but it's not great to not be able to access the collection at all. We should consider a configurable option to replace the slash with another character such as underscore (_) or hyphen (-).
Updated by Tom Clegg almost 5 years ago
The WebDAV server doesn't show collection/project names that have slashes (nor ones named "." or "..").
permittedName() -- source:sdk/go/arvados/fs_base.go#L619
I'm not sure this can be fixed with an escape char: either the escape char is an invalid filename char and clients can't send/receive it, or it is and clients get confused when we treat it as an escape char. Exporting it as something horrible like "<solidus>" would at least make it easier to figure out what the problem is, even if it doesn't make web/filesystem clients behave smoothly.
Perhaps the best course is to disallow these names on the API side so users don't fall into the trap at all.
Updated by Ward Vandewege over 4 years ago
Desired outcome is to replace unallowed characters with something else (e.g. _) in webdav, just like we do for FUSE. That way they will be listed in the client. We should still allow creation of collection with those characters, because they are valid (and in use!).
Updated by Peter Amstutz over 4 years ago
- Target version changed from To Be Groomed to 2020-01-15 Sprint
Updated by Tom Clegg over 4 years ago
I think we should allow but discourage this, since it seems impossible to implement it without surprising/confusing corner cases. Perhaps a config variable "string to replace slashes with in readonly webdav listings" -- where "" means don't list them at all.
If the replacement string is "_":- When looking up "/home/foo__bar/baz", we look up "foo__bar" by name, rather than populating the entire "/home" path. If looking up "foo__bar" and that doesn't exist, we'll need to check for collections/projects named "foo_/bar" or "foo//bar" as well, before giving up.
- If a new "foo_bar" collection/project is created in a project where "foo/bar" already exists, URLs that used to reach the old "foo/bar" will now reach the new "foo_bar".
- In future when "mkdir /home/foo_bar" is possible to do via webdav, and "foo/bar" already exists, we'll need to decide whether that should create a new collection that masks the old one, or return "directory already exists" (probably the latter)
Updated by Lucas Di Pentima over 4 years ago
I've tried using Mountain Duck client version 3.3.3 and the replacement string '%2F
' on a collection name is displayed as is and without issues.
Updated by Tom Clegg over 4 years ago
15836-slash-in-collection-name @ 9d79488376e90532512733748eed1aa78af1c125 -- developer-run-tests: #1704
Updated by Tom Clegg over 4 years ago
15836-slash-in-collection-name @ 3c848757efb9b1697962cb51068c87658565908b -- developer-run-tests: #1708
Updated by Peter Amstutz over 4 years ago
- Target version changed from 2020-01-15 Sprint to 2020-01-29 Sprint
Updated by Peter Amstutz over 4 years ago
From the config.yml comment
- Possible values include "%2f" and "{slash}"
Should be "example" values, otherwise could be read as those are the only two possible values.
If the default empty value is used, names containing "/"
cannot be used when creating or renaming a collection or
project.
Passive voice. Should be 'creating or renaming a collection with a name containing "/" will be rejected by the API server'
for tryURL, expectRegexp := range map[string]string{
base: `(?ms).*href="./` + nameShownEscaped + `/"\S+` + nameShown + `.*`,
base + nameShown + "/": `(?ms).*href="./filename"\S+filename.*`,
} {
Should the path "base + nameShownEscaped" be tested as well? Or is the URL encoding happening in url.Parse() or http.Request() ?
Could you merge/rebase master again so I can do some manual testing?
Updated by Tom Clegg over 4 years ago
Peter Amstutz wrote:
From the config.yml comment
- Possible values include "%2f" and "{slash}"
Should be "example" values, otherwise could be read as those are the only two possible values.
Reworded.
# WebDAV. Example values are "%2f" and "{slash}". Names that # contain the substitution string itself may result in confusing # behavior, so a value like "_" is not recommended.
If the default empty value is used, names containing "/"
cannot be used when creating or renaming a collection or
project.Passive voice. Should be 'creating or renaming a collection with a name containing "/" will be rejected by the API server'
Reworded.
# If the default empty value is used, the server will reject # requests to create or rename a collection when the new name # contains "/".
for tryURL, expectRegexp := range map[string]string{
base: `(?ms).*href="./` + nameShownEscaped + `/"\S+` + nameShown + `.*`,
base + nameShown + "/": `(?ms).*href="./filename"\S+filename.*`,
} {Should the path "base + nameShownEscaped" be tested as well? Or is the URL encoding happening in url.Parse() or http.Request() ?
Right, nameShownEscaped is more correct here -- an unescaped "{" is not allowed in an URL path segment, even though it happens to be accepted by Go.
Could you merge/rebase master again so I can do some manual testing?
Sure, merged master.
15836-slash-in-collection-name @ 4164f38ef0f900962e3c365745c59d5c20a63c66
Updated by Tom Clegg over 4 years ago
Added an entry to the upgrade notes.
15836-slash-in-collection-name @ d766bbe3770d56f8ef391999ec51c42cb751b319
Updated by Peter Amstutz over 4 years ago
Tom Clegg wrote:
Added an entry to the upgrade notes.
15836-slash-in-collection-name @ d766bbe3770d56f8ef391999ec51c42cb751b319
This LGTM
Updated by Anonymous over 4 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|4237a24fcc5e0ff8cf6429b04844ca5f8e5b48c5.