Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Decide on handling path starting with / on *nix #104

Closed
davidfowl opened this issue Apr 26, 2015 · 13 comments
Closed

Decide on handling path starting with / on *nix #104

davidfowl opened this issue Apr 26, 2015 · 13 comments
Assignees
Labels
Milestone

Comments

@davidfowl
Copy link
Member

It's a broken assumption since absolute paths on *nix can start with /

@Eilon
Copy link
Contributor

Eilon commented Apr 26, 2015

@davidfowl we talked in theory about swapping these two checks: https://github.com/aspnet/FileSystem/blob/dev/src/Microsoft.AspNet.FileProviders.Physical/PhysicalFileProvider.cs#L85-L94

However, I don't think that's right either. The real problem is that on Windows it's easy to tell the difference between a volume-rooted path (C:\...) and an absolute path (\foo\bar), and on *nix there's no way to differentiate.

Honestly not quite sure how we could fix this even in theory...

@davidfowl
Copy link
Member Author

I think we have to drop this behavior completely.

@glennc glennc added this to the 1.0.0 backlog milestone Jul 17, 2015
@cesarblum
Copy link
Contributor

This somehow affects aspnet/Universe#304. I had to disable a unit test on *nix which ideally should be run on that platform. However the test gets incorrect results because of the way "/" is being treated.

@muratg
Copy link

muratg commented Nov 23, 2015

Let's reach a decision here. @davidfowl do you suggest we remove support for relative paths?

Can we just enforce a . in front of relative paths? (i.e. ./foo/bar/ instead of /foo/bar) to differentiate it from absolute paths?

@davidfowl
Copy link
Member Author

@davidfowl do you suggest we remove support for relative paths?

No.

Can we just enforce a . in front of relative paths? (i.e. ./foo/bar/ instead of /foo/bar) to differentiate it from absolute paths?

That's ugly. We can just not support paths that begin with /. Relative paths should just start with nothing. .e.g foo/bar.

@muratg
Copy link

muratg commented Nov 25, 2015

That sounds good to me.

@muratg
Copy link

muratg commented Nov 25, 2015

@pakrym this also prevents CoreCLR x-plat test runs. Please look into this after your current item.

@pranavkm
Copy link
Contributor

It's a broken assumption since absolute paths on *nix can start with /

Doesn't the IFileProvider root you at the rooted path? It's not meant to be a general purpose representation of the native file system.

@davidfowl
Copy link
Member Author

It's about what we allow in the methods. We want to fail for absolute paths.

@muratg
Copy link

muratg commented Nov 26, 2015

So do we fail on Windows for paths like C:\foo\bar as well?

@davidfowl
Copy link
Member Author

Yes I believe we already do

@pakrym
Copy link
Contributor

pakrym commented Dec 1, 2015

Decided that current beheviour is okay, just changing supress message.
#150

@muratg
Copy link

muratg commented Dec 1, 2015

Cool, please update the bug title to reflect the decision before checking in.

@pakrym pakrym changed the title Don't assume relative paths start with / Decide on handling path starting with / on *nix Dec 1, 2015
@pakrym pakrym closed this as completed Dec 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants