Unable to handle keys with namespaces

[dignifiedquire]

I came across this when trying to put something with a key like "/foo/bar" this will result in a panic, as only the foo directory is created but not the bar.

Digging deeper the reason is that encode does not handle namespaces in keys properly.

This is my current test output which shows how the sharding function messes up the namespaced key:

=== RUN   TestPutGetNamespaces
=== RUN   TestPutGetNamespaces/prefix
key: /q/u/u/x
getDir: q/
noslash: q/u/u/x
=== RUN   TestPutGetNamespaces/suffix
key: /q/u/u/x
getDir: /x
noslash: q/u/u/x
=== RUN   TestPutGetNamespaces/next-to-last
key: /q/u/u/x
getDir: u/
noslash: q/u/u/x
--- FAIL: TestPutGetNamespaces (0.00s)
    --- FAIL: TestPutGetNamespaces/prefix (0.00s)
    	flatfs_test.go:88: Put fail: rename /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-652809544/q/put-518678023 /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-652809544/q/q/u/u/x.data: no such file or directory
    --- FAIL: TestPutGetNamespaces/suffix (0.00s)
    	flatfs_test.go:88: Put fail: rename /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-073166778/x/put-852409297 /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-073166778/x/q/u/u/x.data: no such file or directory
    --- FAIL: TestPutGetNamespaces/next-to-last (0.00s)
    	flatfs_test.go:88: Put fail: rename /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-267857660/u/put-194386475 /var/folders/v2/4jf047qs3vvgz1b8ty6khx6c0000gn/T/test-datastore-flatfs-267857660/u/q/u/u/x.data: no such file or directory

[Kubuxu]

That is true, go-ds-flatfs was never used with namespaces. IDK if it was ever planned, but we for sure shouldn't let it panic. pinging @whyrusleeping

[whyrusleeping]

cc @kevina

[kevina]

We can (1) Reject them in Put, this will be the simplest option (2) Escape them somehow, (3) somehow create directory entries.

Note that on windows, flatfs will have problems with keys that have any of the disallowed characters in filenames on windows.

I think (1) is the easiest. (2) would be more complicated and I am not sure how to escape this. The original code I believe converted the entire key to hex, but we pushed that encoding (which in now base32) up to the blockstore level. (3) is probably doable but I would not recommend as it will conflict with sharing.

[dignifiedquire]

I think doing (1) reduces the usability of this package when using this as intended from datastore concept, so that is not really a fix but rather keeping it unfinished.
I think it should be the packages job to make sure that keys are escaped in such a fashion they can be written to disk, otherwise you have to think ahead about what datastore you use and change/escape manually keys in those cases. So my vote would be to go with a straightforward escape that makes sure on each os only valid characters are used in the name of the file.

[Kubuxu]

@dignifiedquire flatfs is already quite specialized package because the sharding won't work unless the keys you input are spread out over keyspace. My point is flatfs is specific datastrore system that has one job and does it well.

[kevina]

Note that to be really safe we have to also worry about case-insensitivity of some filesystems. Really the only really safe thing to do is to do the base32 encoding at datastore level rather then the filestore one, but then we have to use binary keys (or have to deal with encoding the CID twice) and the use of binary keys is the entire reason we moved the encoding to the blockstore level.

[Stebalien]

This also significantly hurts the usefulness of Query{Prefix: "..."}.

[Stebalien]

Ah. Nevermind, one namespace is allowed apparently.

[aschmahmann]

@Stebalien should we close this issue? My understanding is that there is no longer a panic anymore, but that this issue is essentially a "won't fix".

[Stebalien]

You're right. This was "fixed" in #64.