path/filepath: IsAbs inconsistently handles Windows reserved filenames

[neild]

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.

[ianlancetaylor]

CC @golang/windows

[bcmills]

For reference, the list of reserved filenames is here:
https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions

However, it's a little fuzzy on the behavior for names with extensions (emphasis mine)

Also avoid these names followed immediately by an extension; for example, NUL.txt is not recommended.

That suggests that maybe a name with an extension doesn't actually end up referring to the global device?

[neild]

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

[martin-sucha]

As an additional data point, according to .NET docs it seems that COM1.TXT\file1.txt also refers to COM1 device.

[mattn]

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

I don't still understand what is wrong here. As far as i can see, current implementation handle NUL as null device but not handle NUL.txt as null device. os.Create(NUL.txt/file.txt) works fine for me.

[mattn]

using System;
using System.IO;
 
class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine(Path.GetFullPath("CON"));
        Console.WriteLine(Path.GetFullPath("CON.txt"));
        Console.WriteLine(Path.GetFullPath(@"COM1.TXT\file1.txt"));
    }
}

Path.GetFullPath seems to return results different from documentation says.

[neild]

os.Create(NUL.txt/file.txt) works fine for me.

What does "works fine for me" mean?

I can't create a directory named NUL.txt. Trying to rename a directory NUL.txt from the explorer generates a "The specified device name is invalid" error. mkdir NUL.txt in a command window returns no error, but the directory is not created.

[mattn]

Really?

[neild]

Really.

What version of Windows are you using?

[mattn]

What version of Windows are you using?

Windows 11 Home 21H2 Build 22000.1098

[mattn]

This is another result provided from my friend.

Microsoft Windows [Version 10.0.22000.1098]
Windows 11 Pro 21H2

[mattn]

Hmm, Windows 10 have this issue.

Microsoft Windows [Version 10.0.19043.2075]

[mattn]

additionaly, Windows 7 also broken.

[mattn]

So, I believe that a possible course of action could be any of the following three.

• IsAbs always return true if filename has prefix NUL like "NUL.txt".
• IsAbs return true if filename has prefix NUL like "NUL.txt" and Windows version is older.
• Don't fix IsAbs and add new func filepath.Sanitize. It might check Windows version.

@alexbrainman What do you think?

[bcmills]

IsAbs return true if filename has prefix NUL like "NUL.txt" and Windows version is older.

I suspect that the behavior here may depend on the filesystem configuration, not just the Windows version.

Would it make sense to Stat the path (or path prefix) that might collide and use some combination of the Mode field and/or os.SameFile to get the OS itself to tell us whether it is a legacy DOS device?

[neild]

It'd be nice to know exactly when and why this changed.

The conservative approach would be for IsAbs to return true for paths that refer to legacy device files on any version of Windows. This might generate false positives on (newer?) Windows systems that narrow the number of legacy device paths, but perhaps that's preferable when a path's validity changes based on the version of the operating system?

[neild]

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.

Is there a reason why we shouldn't do this?

@qmuntal for another possible opinion on this.

[qmuntal]

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.

[neild]

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.

[neild]

Discovered a number of other exciting bugs and possible bugs while fixing this, filed #56336.

[gopherbot]

Change https://go.dev/cl/444279 mentions this issue: path/filepath: use syscall.FullPath to detect reserved Windows file names

[qmuntal]

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: dotnet/runtime#52282.

[qmuntal]

@neild I'm moving the discussion from CL 444279 back to this issue, so it has more visibility. Hope you don't mind.

Quoting @neild:

Leaving IsAbs as-is isn't an option: IsAbs(nul) and IsAbs(.\nul) must have the same result. Anything else is hazardous.

We could maintain consistency by changing IsAbs to always return false for reserved names, but I think that reporting reserved names as absolute is principled, safer and more correct. GetFullPathName rewrites relative, reserved paths like ./nul into an absolute path under \.\ because these paths aren't really relative to the current directory.

These are the benchmark @neild did to bench CL 444279:

func BenchmarkIsAbsSafe(b *testing.B) {
        for i := 0; i < b.N; i++ {
                filepath.IsAbs("/a/b/c/d/e/f/g")
        }
}

func BenchmarkIsAbsNul(b *testing.B) {
        for i := 0; i < b.N; i++ {
                filepath.IsAbs("nul")
        }
}

func BenchmarkIsAbsLongNul(b *testing.B) {
        for i := 0; i < b.N; i++ {
                filepath.IsAbs("/a/b/c/d/e/f/g/nul")
        }
}

name            old time/op    new time/op    delta
IsAbsSafe-8       6.97ns ± 3%   68.20ns ± 1%    +877.93%  (p=0.000 n=10+9)
IsAbsNul-8        6.55ns ± 2%  674.15ns ± 9%  +10185.61%  (p=0.000 n=10+10)
IsAbsLongNul-8    7.01ns ± 2%  776.82ns ± 5%  +10976.54%  (p=0.000 n=10+10)

I agree with @neild that IsAbs should return the same for IsAbs("nul") and IsAbs(".\nul"), but a performance slowdown of 800% on such a foundational API makes me wonder if it really pays off being 100% correct in this case.

Reserved names are a legacy thing from 1970 (see this post to understand why they exist) and using them in path segments or with file extensions is not recommended nor well supported. See, for example, this comment from a .Net team member:

I don't, however, recommend doing so. As you point out, the Win32 API will treat these differently. Most applications aren't going to have logic to allow for legacy device names used as file names. I would also be shocked if .NET was bug free with these- too many places where filenames are recombined with base paths.

My opinion is that IsAbs should not special-case reserved names at all, but just implement what's specified by Win32 docs, which is the approach taken by .Net equivalent Path.IsPathFullyQualified. This would mean revisiting #24556 to fix it by special casing reserved names while joining paths (haven't followed the discussion in that issue, so I might be wrong here).

It is probably too late for doing that, so my next option would be to state that we don't fully support legacy reserved names. I'm worried that we might never get to fully supporting them, it being a moving target (see #56217 (comment)), and we will lose performance on the road.

Merge CL 444279 is still ok for me, but I wanted to give more context so we take a conscious decision.

[neild]

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 cmd/go: go test -o NUL fails on Windows #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.

[qmuntal]

Great summary. Some thoughts:

I believe we should change IsAbs to once again return false for reserved device names like NUL.

Agree that IsAbs should not special case reserved named, but strictly follow Win32 docs.

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.

This is what path/filepath says in the doc header:

Package filepath implements utility routines for manipulating filename paths in a way compatible with the target operating system-defined file paths.

Following this description, filepath.Join("a", "/b") should return /b, any other result is a bug. As a data point, some time ago .Net also realized that they had messed up file path normalization by implementing custom rules instead of deferring to the operating system API, so they decided to break behavior and properly defer to the OS, old behavior could still be enabled via feature flag. See Mitigation: Path Normalization for more info.

Given all the issues stated by @neild, would it make sense to do something similar? I mean, refactor path/filepath so it defers to the appropriate Win32 APIs when necessary, and hide old behavior behind a GODEBUG env var or a go.mod go version directive.

[neild]

Following this description, filepath.Join("a", "/b") should return /b, any other result is a bug.

filepath.Join is underspecified, but its established behavior is to join a list of elements with a separator and Clean the result, nothing more. Too much code will break if filepath.Join("a", "/b") returns anything other than a/b or a\b.

[alexbrainman]

Thank you @neild for taking time to investigate this issue.

• I believe we should change IsAbs to once again return false for reserved device names like NUL.

What do you propose we do? I am happy to revert https://go.dev/cl/145220 . Is that all you plan to do?

• We should fix the cmd/go issue in cmd/go: go test -o NUL fails on Windows #28035 by fixing cmd/go's path handling, not changing filepath.

SGTM.

• 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.

I see no problem adding any new APIs to path/filepath.

Alex

[gopherbot]

Change https://go.dev/cl/448897 mentions this issue: os: remove special casing of NUL in Windows file operations

[gopherbot]

Change https://go.dev/cl/448898 mentions this issue: Revert "path/filepath: change IsAbs("NUL") to return true"

[gopherbot]

Change https://go.dev/cl/448896 mentions this issue: cmd/go/testdata: don't set GOPATH=NUL in test

[gopherbot]

Change https://go.dev/cl/449117 mentions this issue: cmd/go: improve handling of os.DevNull on Windows

[gopherbot]

Change https://go.dev/cl/450296 mentions this issue: os: don't request read access from CreateFile in Stat