os: File.ReadDir returns "The parameter is incorrect." when passed Windows device path (1.22 regression)

[Hakkin]

Go version

1.22.4 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Hakkin\AppData\Local\go-build
set GOENV=C:\Users\Hakkin\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Hakkin\Desktop\Programs\Misc\Projects\Go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Hakkin\Desktop\Programs\Misc\Projects\Go
set GOPRIVATE=
set GOPROXY=
set GOROOT=C:\Program Files\Go
set GOSUMDB=
set GOTMPDIR=
set GOTOOLCHAIN=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.4
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Hakkin\AppData\Local\Temp\go-build2088173783=/tmp/go-build -gno-record-gcc-switches

What did you do?

Attempted to list a device path (\\.\C:) in Go 1.22 on Windows.

Code, must be ran under admin:

package main

import (
	"log"
	"os"
)

func main() {
	device_path := `\\.\C:`
	f, err := os.Open(device_path)
	if err != nil {
		log.Fatalf("error opening path: %s", err)
	}

	entries, err := f.ReadDir(100)
	if err != nil {
		log.Fatalf("error reading directory: %s", err)
	}

	log.Printf("found %d entries in directory", len(entries))
}

Note this also happens for other device paths, not just \\.\C:. Other common device paths are things like \\?\Volume{...}, \\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy..., etc. This issue was encountered at kopia/kopia#3842 when dealing with Windows Shadow Copy Volumes in 1.22.

What did you see happen?

# go version
go version go1.22.4 windows/amd64

# go run main.go
2024/06/04 18:18:10 error reading directory: GetFileInformationByHandleEx \\.\C:: The parameter is incorrect.

What did you expect to see?

This code works as expected in 1.21.10, thus it seems to be a regression.

# go version
go version go1.21.10 windows/amd64

# go run main.go
2024/06/04 18:22:09 found 22 entries in directory

In 1.22, you can restore the previous behavior by appending a backslash \ to the path:

...
device_path := `\\.\C:\` // <- Trailing backslash
...

# go version
go version go1.22.4 windows/amd64

# go run main.go 
2024/06/04 18:49:36 found 22 entries in directory

but these kinds of paths with trailing backslashes are mangled by functions like filepath.Clean and have the backslash stripped. filepath.Clean has exceptions for root windows paths like C:\, but these exceptions aren't applied to device paths.

[seankhliao]

cc @golang/windows

[qmuntal]

I can't reproduce this issue. os.Open("\\.\C:") fails with access denied, so f.ReadDir is not even called. Could you elaborate on the reproducer a bit more?

[Hakkin]

I can't reproduce this issue. os.Open("\\.\C:") fails with access denied, so f.ReadDir is not even called. Could you elaborate on the reproducer a bit more?

As noted, the code must be run under admin or you will get access denied trying to access the root device path.

[qmuntal]

As noted, the code must be run under admin or you will get access denied trying to access the root device path.

Ouch, thanks. I can reproduce it now.

I'm not sure if this is a valid regression. Before go1.22, we were reading directories using the syscall.Find[First|Next]File windows APIs and were always appending a backslash in case it wasn't there: https://github.com/golang/go/blob/release-branch.go1.21/src/os/file_windows.go#L119. Then CL 452995 switched to a more performant Windows APIs which didn't need the path to always end with a backslash, so it was removed.

The side effect is that invalid device paths, like \\.\C:, can't be read anymore. Please see this Microsoft docs:

All volume and mounted folder functions that take a volume GUID path as an input parameter require the trailing backslash.
https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-volume.

Although it only mention GUID paths (\\?\Volume{X}\), it also applies to device paths, as either the \\?\ and the \\.\ instructs the Windows API to ingest the file path as-is, without any normalization, so a trailing backslash can't be automatically added. It's weird that os.Open succeed, but the same docs also mention that syscall.CreateFile (used on os.Open) do special case this invalid form.

Note that running the CMD command dir \\.\C: also fails.

[Hakkin]

I'm not sure if this is a valid regression.

I agree that that the previous behavior might have technically been "incorrect", but since it did work without any special handling by the program, it's also likely true that a lot of Go programs that previously handled root device paths fine in 1.21 will now fail in 1.22 (like the Kopia issue linked in the OP, or just any program that accepts user directory paths). The fact that these paths aren't properly handled by standard library functions like filepath.Clean() means that any file handling code that passed through these functions (which I would assume is quite a lot) will be broken with these kinds of device paths after 1.22, unless explicitly handled by each program.

[qmuntal]

I agree that this issue should be fixed and backported. Unfortunately I don't have the bandwith to submit a fix now. Maybe @neild or @ianlancetaylor can help here.

[Hakkin]

The fact that these paths aren't properly handled by standard library functions like filepath.Clean()

Sorry for the double post, but to clarify this a little more, filepath.Clean does actually have special handling for these paths, but only in specific circumstances. This mostly seems to be handled in volumeNameLen here:

go/src/internal/filepathlite/path_windows.go

Lines 228 to 243 in 45967bb

	case pathHasPrefixFold(path, `\\.`) ||
	pathHasPrefixFold(path, `\\?`) || pathHasPrefixFold(path, `\??`):
	// Path starts with \\.\, and is a Local Device path; or
	// path starts with \\?\ or \??\ and is a Root Local Device path.
	//
	// We treat the next component after the \\.\ prefix as
	// part of the volume name, which means Clean(`\\?\c:\`)
	// won't remove the trailing \. (See #64028.)
	if len(path) == 3 {
	return 3 // exactly \\.
	}
	_, rest, ok := cutPath(path[4:])
	if !ok {
	return len(path)
	}
	return len(path) - len(rest) - 1

then Clean uses this here:

go/src/internal/filepathlite/path.go

Lines 66 to 75 in 45967bb

	originalPath := path
	volLen := volumeNameLen(path)
	path = path[volLen:]
	if path == "" {
	if volLen > 1 && IsPathSeparator(originalPath[0]) && IsPathSeparator(originalPath[1]) {
	// should be UNC
	return FromSlash(originalPath)
	}
	return originalPath + "."
	}

So filepath.Clean does maintain the trailing slash for paths like \\.\C:\, but doesn't for paths like \\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1\, since it treats GLOBALROOT as the volume name instead of the full device path.

Also note volumeNameLen references #64028

[qmuntal]

So filepath.Clean does maintain the trailing slash for paths like \.\C:, but doesn't for paths like \?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1, since it treats GLOBALROOT as the volume name instead of the full device path.

Please fill another issue for this bug. Thanks!