-
Notifications
You must be signed in to change notification settings - Fork 109
fix: don't expect ipfs to preserve a leading slash #440
Conversation
I'm glad these are now normalized, we'll have to submit a PR to JS IPFS to implement this. It needs some tests to ensure the normalisation happens. Separately, I think this change breaks the path given to resolve later in the test: |
@Stebalien would you mind pointing me in the direction of the PR(s) that implemented this? I just want to make sure we do the same thing...For instance, I'd like to know what happens with paths that try to traverse above root e.g. '../foo' => 'foo' or '../foo' => error? Also windows style paths i.e. backslashes and |
See ipfs/go-ipfs-files#6 and https://golang.org/pkg/path/filepath/#Clean
|
(got to get better about actually running tests) This changed (incidentally) in ipfs/go-ipfs-files#6. Normalized with https://golang.org/pkg/path/#Clean. Actually, I think we're going to need to think through this a bit. (edited: I found the wrong |
51cb37b
to
cdecbba
Compare
Go-ipfs now normalizes paths before adding files. This will: * remove any leading slashes. * remove any `/./` components. * normalize any `/../` components. That breaks this test which assumes that paths are returned _exactly_ as specified.
cdecbba
to
fecf781
Compare
Ah, that's why I didn't. Didn't want to learn how to replace npm deps. |
Do you mean For the former, I don't have strong feelings on it. It's pretty common to not error when attempting to navigate up from root, go-lang For the latter, I guess it's just convenient to allow this when adding and it's easy to do the resolve. People will just expect it to work so why get in the way? I noticed that |
Both? The request body feels like a declarative filesystem layout. For example, I I'd expect But I don't really fell all that strongly about this. |
Go-ipfs now normalizes paths before adding files. This will:
/./
components./../
components.That breaks this test which assumes that paths are returned exactly as specified.