A concrete example of where this could be useful:
https://cs.opensource.google/go/x/build/+/master:cmd/buildlet/buildlet.go;l=1633-1670;drc=e73b639045d6176f6de4efb526c3d55475c065c5

I'm starting to get confused about which of these new filepath functions should be used when.

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

I'm starting to get confused about which of these new filepath functions should be used when.

I am very sympathetic to that confusion.

Counting IsLocal added in 1.20, we have three new functions:

• filepath.IsLocal checks to see if a OS path is hinky. A hinky path is one that refers to something outside the current directory, or something weird like COM1 on Windows. I've run across a number of sanitization functions along the lines of strings.TrimPrefix(filepath.Clean("/"+p), "/") to ensure that a path is neither absolute nor contains .. components; IsLocal is intended to be a more robust and portable version of accomplishing the same goal. It's a somewhat specialized function that most users will not need, but there are so many subtleties (particularly surrounding Windows device names) that I'm still convinced it was worthy of inclusion into the standard library.
• filepath.FromFS converts a non-OS-specific /-separated path as one might find in a URL and converts it into an OS-specific path. Unlike FromSlash, it reports an error if the path can't be represented locally. Everyone who uses FromSlash today should use FromFS instead.
• filepath.IsReserved isn't particularly useful on its own, in my opinion, but both IsLocal and FromFS need to be aware of Windows reserved device names. A function which answers the limited question of whether a name is reserved seems like a useful building block to expose, especially since getting this right is quite tricky.

You should use FromFS when converting a /-separated path into an OS path.

You should use IsLocal to verify that a path from an untrusted source doesn't refer to anything surprising.

You should use IsReserved when building a sanitization function along these lines, if the above aren't sufficient.

It is unclear to me why the function is called FromFS.

The proposal says that it supports paths that path package supports. The path package supports absolute paths. So it seems that filepath.FromFS would support absolute paths. However, I'm just guessing here since the proposal does not specify the desired behavior.

Then there is fs.ValidPath that rejects absolute paths.
Based on the name, I would expect that FromFS returns an error for any input where fs.ValidPath returns false.

If FromFS is supposed to support absolute paths and since FromFS is intended to replace FromSlash, perhaps it should just be called FromSlashV2? If FromFS is not supposed to support absolute paths, we should document that explicitly.

I don't think it's a given that everyone who uses FromSlash today should use FromFS.

Today, FromSlash converts slashes to the canonical form for the host OS but preserves the meaning of the existing path. So for example on Windows today, FromSlash("c:/foo") gives you c:\foo, which is the canonical form of its input. Similarly on a Mac you get FromSlash("c:/foo") is c:/foo, and in both cases the result of os.Open(p) and os.Open(FromSlash(p)) are the same.

It sounds like FromFS would not do that. You should use FromFS when the input is meant to be a "portable" slash-separated path as opposed to a slash-separated path interpreted according to the local OS. Programs that accept a file name on the command line but want to convert to native conventions should probably keep using FromSlash. The compiler does this sometimes for arranging canonical outputs and then inverting them. It should keep using FromSlash and ToSlash.

Maybe something reading from a zip file should use FromFS, but why not just have it use IsLocal+FromSlash instead of FromFS?

Or is it just programs implementing an fs.FS that need to use FromFS?

I don't think the exact scope is clearly defined yet.

It seems like we are stuck on the name here. I noticed that internal/safepath.FromFS is called with paths beginning with / (like /foo) but those are not actually io/fs paths. I'm not sure if the proposed FromFS rejects those or not, but safepath.FromFS does not.

Also it's probably too indirect a meaning to use "FS" here.

There are fundamentally two kinds of FromSlash: ones that are canonicalizing the OS interpretation and ones that are converting from "portable" to "local OS". The current FromSlash does the former. We need a name for the latter.

Maybe the From prefix is tripping us up and we should name this operation with some verb that can be the function name.

filepath.Localize ?

I can't come up with a verb, because filepath.Verb implies an operation on host OS paths. This is an operation on a non-host path.

filepath.SafeFromPath?

filepath.FromSlashPath?

FromSlash seems like the right name for this operation; the problem is that the existing function doesn't have an error return.

Probably impractical: Keep filepath.FromSlash, but change it to return "" if replacing slashes in the input path changes the path's meaning.

Localize means "restrict (something) to a particular place" and can also be interpreted as converting to local syntax, which it also does. That seems like a good name. Any better names?

+1 for this proposal. I always fear using the path/filepath packages because I never truly understand what to expect.

As for Localize, it makes perfect sense that it should return a value based on the OS.

How does Localize relate to IsLocal? I am worried that it might be confusing to mix those two. If I understand correctly, FromFS/Localize allows absolute paths, so the following would be possible:

s, err := Localize("/a/b/c")  // s = "/a/b/c", err = nil
IsLocal(s) // false

In other words, the result of Localize could be non-local, because local paths cannot be absolute.

Since @rsc mentioned

ones that are converting from "portable" to "local OS""

maybe we should define what a portable path is and use something like ParsePortable? FromPortable?

It seems having a definition of all the supported path variants in the package docs would be helpful in any case.

Localize and IsLocal are unrelated, which is indeed unfortunate.

Given that io/fs does not accept /a/b/c as a path and we started at FromFS meaning "from io/fs paths", my understanding was that Localize would reject /a/b/c and ../a/b/c, at which point Localize would guarantee to return a path satisfying IsLocal.

If we do that, is Localize no longer useful enough?

Suppose we have func Localize(string) (string, error), which converts the input to be a local file system path, satisfying IsLocal, or else returns an error. Localize("/a/b/c") and Localize("../a/b/c") are both errors, as are things like Localize("com1") on Windows.

Does that work for the use cases we want it for?
Is that good enough?

Suppose we have func Localize(string) (string, error), which converts the input to be a local file system path, satisfying IsLocal, or else returns an error.

This is blending two different concerns: converting a slash-separated path into an operating system path, and enforcing constraints on the path (e.g., not absolute). It feels to me like we're arriving at a behavior because of the ambiguous meaning of "localize", not because we want that particular behavior.

But along those lines, how about the original name FromFS, defined to operate only on paths which fs.ValidPath considers valid? No ".." components, no leading or trailing "/". If you want, you can use path.Clean(path) to resolve relative path components before conversion.

I wrote:

Given that io/fs does not accept /a/b/c as a path and we started at FromFS meaning "from io/fs paths", my understanding was that Localize would reject /a/b/c and ../a/b/c, at which point Localize would guarantee to return a path satisfying IsLocal.

Then Damien wrote:

But along those lines, how about the original name FromFS, defined to operate only on paths which fs.ValidPath considers valid? No ".." components, no leading or trailing "/". If you want, you can use path.Clean(path) to resolve relative path components before conversion.

Are these two descriptions of the same behavior with different names, or are the behaviors different? It appears they are the same.

But if the behaviors are the same, then Localize seems like a clearer name than FromFS, because lots of people using filepath are unfamiliar with io/fs and won't think of FS as indicating io/fs. They might think it means something about the local operating system's file system. filepath.Localize in contrast sounds very much like a conversion to local conventions (and it is), in addition to being a containment of what the path can refer to (and it is).

Are these two descriptions of the same behavior with different names, or are the behaviors different? It appears they are the same.

What should Localize("a/../b") return?

If it's an error, because the input isn't a valid path according to "io/fs".IsValid, then these are same behavior with different names.

I think this behavior works for the cases we care about. I'm okay with either the name Localize or FromFS for the function.

Yes, I was assuming any mention of .. in Localize's argument is an error.

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

Change https://go.dev/cl/520268 mentions this issue: path/filepath: add Localize

Change https://go.dev/cl/531677 mentions this issue: path/filepath: add Localize