Skip to content

os.Lstat & os.OpenFile fail at handling long paths on Windows #15208

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

Closed
xStrom opened this issue Apr 9, 2016 · 1 comment
Closed

os.Lstat & os.OpenFile fail at handling long paths on Windows #15208

xStrom opened this issue Apr 9, 2016 · 1 comment

Comments

@xStrom
Copy link

xStrom commented Apr 9, 2016

os.Lstat & os.OpenFile fail at handling long paths with go1.6 windows/amd64.

package main

import (
    "fmt"
    "os"
)

func main() {
    // The LICENSE file actually exists
    name := `C:\Gopath\src\bitbucket.org\xStrom\lopya\node_modules\gulp\node_modules\gulp-util\node_modules\dateformat\node_modules\meow\node_modules\normalize-package-data\node_modules\validate-npm-package-license\node_modules\spdx-correct\node_modules\spdx-license-ids\LICENSE`
    _, err := os.Lstat(name)
    if err != nil {
        fmt.Printf("Got error, but shouldn't: %v\n", err)
    }
    _, err = os.OpenFile(name, os.O_RDONLY, 0)
    if err != nil {
        fmt.Printf("Got error, but shouldn't: %v\n", err)
    }
}

Both of these functions fail due to improper usage of syscall.GetFileAttributesEx. [1] [2] Specifically, they just pass the user provided name to GetFileAttributesEx.

The GetFileAttributesEx documentation states: In the ANSI version of this function, the name is limited to MAX_PATH [== 260] characters. To extend this limit to 32,767 wide characters, call the Unicode version of the function and prepend "\?" to the path.

It seems to me that Go always uses the Unicode version, which should make resolving this easier.

As a developer, I could prepend the "\?" to the path myself before calling os.Lstat or os.OpenFile, and indeed this seems to solve the problem according to some of my quick tests. However other people's code would still suffer from this bug, and indeed I found this bug when using a 3rd party program. Also the os package claims to provide "a platform-independent interface to operating system functionality". So I think it would be more fitting if it actually did that and I didn't have to prepend "\?" after an if windows check.

At first it seems like a simple fix to just start prepending "\?" to the path before calling syscall.GetFileAttributesEx. However it turns out that GetFileAttributesEx does some magic if the MAX_PATH code path executes, e.g. File I/O functions in the Windows API convert "/" to "" as part of converting the name to an NT-style name, except when using the "\?" prefix. So if we just start prepending "\?" then people's existing code will start breaking, if they're using C:/foo/bar.txt or something similar as input.

I think a reasonable solution would be to do a length check on the path name before calling syscall.GetFileAttributesEx. If the name fits MAX_PATH, then use it as is, and enjoy the extra magic. If the name exceeds MAX_PATH, then we can only benefit from prepending "\?". This would also solve the problem I was having.

I can submit a code change if this seems like an acceptable solution.

@bradfitz
Copy link
Contributor

bradfitz commented Apr 9, 2016

I think this is a dup of #3358

I agree this should be fixed, but let's consolidate planning in #3358

@bradfitz bradfitz closed this as completed Apr 9, 2016
@golang golang locked and limited conversation to collaborators Apr 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants