Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io/ioutil: ReadDir if long-path ends in backslash on Windows #17500

Closed
ncw opened this issue Oct 18, 2016 · 13 comments
Closed

io/ioutil: ReadDir if long-path ends in backslash on Windows #17500

ncw opened this issue Oct 18, 2016 · 13 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@ncw
Copy link
Contributor

ncw commented Oct 18, 2016

What version of Go are you using (go version)?

X:>go version
go version go1.7.1 windows/386

What operating system and processor architecture are you using (go env)?

I am testing this on Windows 7 Pro 386 SP1 running under VirtualBox.

X:\>go env
set GOARCH=386
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=386
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_386
set CC=gcc
set GOGCCFLAGS=-m32 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1

What did you do?

Build this program http://play.golang.org/p/rBGf11wDo7 (note that this is from #4601) and run it giving UNC paths to the root directory.

X:\Go>go build readdir.go

X:\Go>dir \\?\c:\
 Volume in drive \\?\c: has no label.
 Volume Serial Number is 5CD1-D250

 Directory of \\?\c:

11/06/2009  02:12                24 autoexec.bat
11/06/2009  02:12                10 config.sys
18/10/2016  13:46    <DIR>          Go
[snip]
14/07/2009  07:07    <DIR>          PerfLogs
04/05/2011  14:38    <DIR>          Program Files
04/05/2011  14:30    <DIR>          Users
19/08/2016  02:25    <DIR>          Windows
               2 File(s)             34 bytes
              10 Dir(s)               0 bytes free

X:\Go>readdir.exe \\?\c:\
entries []
err open \\?\c:\: The system cannot find the path specified.

X:\Go>readdir.exe \\?\c:
entries [0x1166e0f0 0x1166e230 0x1166e280 0x1166e320 0x1166e370 0x1166e3c0 0x116
6e410 0x1166e460 0x1166e4b0 0x1166e500 0x1166e5a0 0x1166e5f0 0x1166e640 0x1166e6
90 0x1166e140 0x1166e190 0x1166e1e0 0x1166e2d0 0x1166e6e0]
err <nil>

X:\Go>readdir.exe c:\
entries [0x11548000 0x11548140 0x11548190 0x11548230 0x11548280 0x115482d0 0x115
48320 0x11548370 0x115483c0 0x11548410 0x115484b0 0x11548500 0x11548550 0x115485
a0 0x11548050 0x115480a0 0x115480f0 0x115481e0 0x115485f0]
err <nil>

X:\Go>readdir.exe \\?\c:\Go
entries [0x11728050 0x11728140 0x11728190 0x117282d0 0x11728370 0x11728410 0x117
28550 0x11728000 0x117280a0 0x117280f0 0x117281e0 0x11728230 0x11728280 0x117283
20 0x117283c0 0x11728460 0x117284b0 0x11728500]
err <nil>

What did you expect to see?

I expected readdir.exe \\?\c:\ to produce an output with some directory entries, not an error. It should have produced the same output as provided by readdir.exe \\?\c:

What did you see instead?

The error err open \\?\c:\: The system cannot find the path specified.

Note that this is possibly related to #4601

@mattn
Copy link
Member

mattn commented Oct 18, 2016

Try to display the filename

package main

import (
    "fmt"
    "io/ioutil"
    "os"
)

func main() {
    if len(os.Args) < 2 {
        fmt.Println("Need directory as parameter")
        return
    }
    dir := os.Args[1]
    entries, err := ioutil.ReadDir(dir)
    for _, f := range entries {
        fmt.Println(f.Name())
    }
    fmt.Println("err", err)
}

@hirochachacha
Copy link
Contributor

FWIW, "\?" is not a UNC path, but the long path prefix. Dup of #3358?

@quentinmit quentinmit changed the title io/ioutil: ReadDir fails on UNC format \\?\c:\ root directories on Windows io/ioutil: ReadDir if long-path ends in backslash on Windows Oct 20, 2016
@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 20, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Oct 20, 2016
@quentinmit
Copy link
Contributor

I'm inclined to say this is WAI. The Windows documentation is pretty clear that ?\ turns off a bunch of path handling code; it looks like one of the things it turns off is stripping of the trailing slash character. I don't think Go should do anything above and beyond what the underlying system calls do to handle malformed paths.

/cc @rsc @alexbrainman

@mattn
Copy link
Member

mattn commented Oct 20, 2016

Well, I put the code since I don't reproduce.

Windows7 64bit

@alexbrainman
Copy link
Member

@ncw I cannot reproduce it here too. It displays files and returns no error on my Windows XP (386) and Windows 7 (amd64). I am using current tip, but I doubt it matters. Perhaps you could try and debug this. Thank you.

Alex

@ncw
Copy link
Contributor Author

ncw commented Oct 21, 2016

I've done a bit more testing. I can reproduce the problem on my other Windows 7 Pro 386 SP1 VM.

On windows XP I seem to get the opposite results where \\?\c: is not found but \\?\c:\ is.

However as @quentinmit suggested it does seem to be just whether you supply a trailing \ or not, it isn't a specific feature of the root directory. So if I supply a trailing \ the listing fails and if I don't it doesn't.

I've been unable to find a definitive answer as to whether trailing \ are allowed for \\?\ directory paths, or not. From the testing above it seems to depend on Windows version.

Maybe that is as it should be and Go shouldn't try and fixup Windows syscalls.

@alexbrainman
Copy link
Member

On windows XP I seem to get the opposite results where ?\c: is not found but ?\c:\ is.

I can reproduce that, thank you very much.

The problem appears to be with the way we open file or directory. Windows does not have generic API that opens both. Instead we always try to open path as file first (see os.openFile function), and, if that fails, we assume we have directory here (os.openDir). Our logic assumes that directory cannot be opened with "file opening" API CreateFile, and that seems worked so far. But not in the issue above.

We could, probably, "fix" this by adding some extra checks after CreateFile returns (we could try and call GetFileInformationByHandle, even check returned parameter FileAttributes field if succeeds). But, I think, we should avoid using long paths (#10577 (comment)) in general, so adding extra code for every file open does not sounds right to me.

Alex

@minux
Copy link
Member

minux commented Oct 25, 2016 via email

@rsc
Copy link
Contributor

rsc commented Nov 2, 2016

I think this is working as intended, but to the extent that it informs what @quentinmit's patch for long file names can do, it's relevant. At the least we should have tests for ReadDir of c:\windows\ and c:\windows and make sure both get the same result. And user-specified \\?\ paths should be completely unaltered.

@quentinmit
Copy link
Contributor

After spending days banging my head on this, I think I understand what's happening here. \\?\c:\ requires the trailing slash to identify the root directory. Any other path, such as \\?\c:\windows requires omitting the trailing slash to name the "windows" directory.

This is WAI; if you use a \\?\ path, you are completely at the mercy of Windows's behavior. I do not believe Go behaves any differently than a C program using the same underlying APIs.

@quentinmit
Copy link
Contributor

Ack, reading more closely, I see that a couple of you have reported that you need to omit the trailing slash even for the root directory. I'll keep this issue open until I've figured out under what conditions that is true.

@quentinmit quentinmit reopened this Nov 2, 2016
@quentinmit
Copy link
Contributor

Okay, I think I have an explanation for the behavior you observed. openDir in file_windows.go calls FindNextFile with a match of \\?\C:\\*. Some versions of windows are okay with the doubled slash, and some are not. I have a CL out to never add a double slash, which should make it always work.

Note that you do need a trailing slash on the root directory or Stat will fail.

@gopherbot
Copy link

CL https://golang.org/cl/32451 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

8 participants