go: path/filepath: `IsAbs` inconsistently handles Windows reserved filenames
On Windows, filepath.IsAbs returns true for reserved filenames like CON, NUL, or LPT1: https://go.googlesource.com/go/+/refs/tags/go1.19.2/src/path/filepath/path_windows.go#16
This behavior is not documented, but seems reasonable in that these filenames reference objects outside of the current directory. The check for these filenames was added in 2018 in https://go.dev/cl/145220; see that CL for some more history on how we got to the current state.
However, this check is not sufficient.
The filename NUL is the null device. So is NUL.txt. So is C:\Path\To\NUL.a.b.c.
If filepath.IsAbs("NUL") returns true, then IsAbs should also return true for any path that contains a reserved name as a directory component (including reserved names with extensions).
This also affects the special-case handling of NUL added to os.Mkdir as a resolution for #24556.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 2
- Comments: 32 (26 by maintainers)
Commits related to this issue
- cmd/go/testdata: don't set GOPATH=NUL in test An upcoming change to the filepath package to make IsAbs("NUL")==false on Windows will cause this test to fail, since it sets GOPATH=NUL and GOPATH must ... — committed to golang/go by neild 2 years ago
- cmd/go: improve handling of os.DevNull on Windows The "go test" and "go build" commands have special-case behavior when passed "-o /dev/null". These checks are case-sensitive and assume that os.DevNu... — committed to golang/go by neild 2 years ago
- os: remove special casing of NUL in Windows file operations Some file operations, notably Stat and Mkdir, special cased their behavior when operating on a file named "NUL" (case-insensitive). This ch... — committed to golang/go by neild 2 years ago
- Revert "path/filepath: change IsAbs("NUL") to return true" This reverts commit d154ef60a0c88be98c70bbe1c5735fb7b1f45250. This change made IsAbs return true for certain reserved filenames, but does n... — committed to golang/go by neild 2 years ago
- os: don't request read access from CreateFile in Stat CL 448897 changed os.Stat to request GENERIC_READ access when using CreateFile to examine a file. This is unnecessary; access flags of 0 will per... — committed to golang/go by neild 2 years ago
I’m not convinced by the argument from efficiency. If
IsAbs("./nul")should return true, then failing to do so is a bug and we must correct it. An incorrect function can always outperform a correct one.However, I dug further into the history of how we came about the current behavior, and I no longer believe
IsAbs("nul")should return true.IsAbswas changed to return true fornuland other reserved device names in https://go.dev/cl/145220.This change was made to fix #28035. The root cause of #28035 is this code in
cmd/go/internal/test/test.go:This code is incorrect on Windows. It was incorrect before https://go.dev/cl/145220, and it is still incorrect now.
The first issue in this code is that it assumes an absolute base path may be joined to a relative path with
filepath.Jointo produce an absolute path.On Unix systems, all paths are either absolute or relative to the current directory. An absolute base path may be joined to a relative path to produce an absolute path, and this code is correct.
On Windows systems, however, relative paths come in a variety of flavors: relative to the current directory (
x), relative to the current drive (/x), and relative to the current directory of a specified drive (c:x). Only the first form may be sensibly appended to a base path withfilepath.Join; the others will produce incorrect results.The second issue in this code is that it assumes there is a single name for
os.DevNull. This is not true even on Unix systems–you can create any number of null devices withmknod. On Windows systems, the null device is most often referred to asNUL, but is canonically named//./NUL. Furthermore, Windows filenames are case-insensitive, so even considering just the simplest and most common names, the above code will fail to detect a target ofnul.The third issue is that this code assumes the name of the null device in
os.DevNullis an absolute path, butNULis a relative path.While https://go.dev/cl/145220 attempts to correct the above code by adding
NULto the set of absolute paths, this fixes only the third of these issues and only in the extremely limited case of the target being specified as the exact, case-sensitive stringNUL.So while there may be some argument for treating reserved device names as absolute paths, fixing #28035 is not that argument, because the root cause of that issue is not actually fixed by this change.
I think we should therefore consider rolling back https://go.dev/cl/145220. Windows has a definition of what paths are absolute (https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats). On Windows,
IsAbsreturns false for surprising-to-Unix-programmers cases like/x, following the Windows definition that this path is relative to the root of the current drive. Windows APIs which report whether a path is absolute likeSystem.IO.Path.IsPathFullyQualifiedreport false forNUL.The argument against rolling back https://go.dev/cl/145220 is that this change is four years old at this point, and some users may be relying on
IsAbsdetecting reserved device names. However, so far as I can tell this behavior has never been documented, andIsAbsdoes not reliably detect reserved names, missing./nuland others.This does leave the question of what the correct way is to write code like this in a platform-agnostic fashion:
On Windows using .NET, I believe the
filepath.Joinoperation here would be written with thesystem.IO.Path.GetFullPathfunction, which takes an absolute path and a relative path and returns an absolute path. This is the operation performed byfilepath.Joinon Unix systems. Perhaps this is whatfilepath.Joinshould have done on Windows, but I don’t think we can change that function at this time. It would be too much of a change forfilepath.Join("a", "/b")to now return/b.Perhaps we need a new function for this operation. I’m not sure what we would call it.
FullPath?To summarize the above:
IsAbsto once again return false for reserved device names likeNUL.cmd/goissue in #28035 by fixingcmd/go’s path handling, not changingfilepath.filepathfunction that sensibly operates on Windows relative paths, but I don’t yet know what the name or semantics would be.Right; pretend I said
\nul.txt\a.This sounds like the right approach to me.
filepath.Absalready usesGetFullPathName, so extending this tofilepath.IsAbsseems like the right approach.Empirically,
nul.txtandc:\path\to\nul.txtrefer to the null device, at least when using theospackage.I’ve done some more research and it seems that reserved names in path segments (e.g.
./NUL.txt) are allowed since an unknown (to me) Windows 10 patch. Indirect source: https://github.com/dotnet/runtime/pull/52282.IsAbs("C:\nul.txt\a")is already true, because it starts with a valid drive letter and a colon. We should only special case paths which have a reserved file name, with or without extension, in the first segment of the path.Given the comments in this issue, it is clear that there is no general rule to decide if those cases will be handled as device paths, so we should let Windows decide. We can do so by calling GetFullPathName and check if the returned path follows the pattern
\\.\devicename. This approach is documented in .Net docs and also followed by OpenJDK.