Open Hakkin opened 5 months ago
cc @golang/windows
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?
I can't reproduce this issue.
os.Open("\\.\C:")
fails with access denied, sof.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.
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.
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.
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.
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:
https://github.com/golang/go/blob/45967bb18e04fa6dc62c2786c87ce120443c64f6/src/internal/filepathlite/path_windows.go#L228-L243
then Clean
uses this here:
https://github.com/golang/go/blob/45967bb18e04fa6dc62c2786c87ce120443c64f6/src/internal/filepathlite/path.go#L66-L75
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 https://github.com/golang/go/issues/64028
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!
Go version
1.22.4 windows/amd64
Output of
go env
in your module/workspace:What did you do?
Attempted to list a device path (
\\.\C:
) in Go 1.22 on Windows.Code, must be ran under admin:
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 https://github.com/kopia/kopia/issues/3842 when dealing with Windows Shadow Copy Volumes in 1.22.What did you see happen?
What did you expect to see?
This code works as expected in 1.21.10, thus it seems to be a regression.
In 1.22, you can restore the previous behavior by appending a backslash
\
to the path: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 likeC:\
, but these exceptions aren't applied to device paths.