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

Most upvoted comments

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.

IsAbs was changed to return true for nul and 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:

if !filepath.IsAbs(target) {
        target = filepath.Join(base.Cwd(), target)
}
// ...
if target == os.DevNull {
        // ...
}

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.Join to 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 with filepath.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 with mknod. On Windows systems, the null device is most often referred to as NUL, 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 of nul.

The third issue is that this code assumes the name of the null device in os.DevNull is an absolute path, but NUL is a relative path.

While https://go.dev/cl/145220 attempts to correct the above code by adding NUL to 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 string NUL.

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, IsAbs returns 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 like System.IO.Path.IsPathFullyQualified report false for NUL.

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 IsAbs detecting reserved device names. However, so far as I can tell this behavior has never been documented, and IsAbs does not reliably detect reserved names, missing ./nul and others.

This does leave the question of what the correct way is to write code like this in a platform-agnostic fashion:

if !filepath.IsAbs(target) {
        target = filepath.Join(base.Cwd(), target)
}

On Windows using .NET, I believe the filepath.Join operation here would be written with the system.IO.Path.GetFullPath function, which takes an absolute path and a relative path and returns an absolute path. This is the operation performed by filepath.Join on Unix systems. Perhaps this is what filepath.Join should 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 for filepath.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?

FullPath(`C:\a\b`, `c`) = `C:\a\b\c`
FullPath(`C:\a\b`, `\c`) = `C:\c`
FullPath(`C:\a\b`, `D:c`) = error, or do we look up the working directory for D:?

To summarize the above:

  1. I believe we should change IsAbs to once again return false for reserved device names like NUL.
  2. We should fix the cmd/go issue in #28035 by fixing cmd/go’s path handling, not changing filepath.
  3. Maybe we need a new filepath function that sensibly operates on Windows relative paths, but I don’t yet know what the name or semantics would be.

IsAbs("C:\nul.txt\a") is already true

Right; pretend I said \nul.txt\a.

We can do so by calling GetFullPathName and check if the returned path follows the pattern \\.\devicename.

This sounds like the right approach to me. filepath.Abs already uses GetFullPathName, so extending this to filepath.IsAbs seems like the right approach.

Empirically, nul.txt and c:\path\to\nul.txt refer to the null device, at least when using the os package.

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.

My inclination is to take the most conservative approach: IsAbs will report a path as absolute if it might refer to a reserved device filename on any version of Windows. Thus IsAbs(“C:\nul.txt\a”) is true.

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.