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

syscall, os: Chmod doesn't support long paths on Windows #20829

Closed
ibrasho opened this issue Jun 28, 2017 · 6 comments
Closed

syscall, os: Chmod doesn't support long paths on Windows #20829

ibrasho opened this issue Jun 28, 2017 · 6 comments

Comments

@ibrasho
Copy link
Contributor

@ibrasho ibrasho commented Jun 28, 2017

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

go version go1.8.3 windows/amd64

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

GOOS=windows
GOARCH=amd64

What did you do?

os.Chmod returns an unexpected error when passed a path that is longer than MAX_PATH (260) characters.

What did you expect to see?

Similar behavior to other file functions in os. Calling os.fixLongPath on the path before passing the pointer to GetFileAttributesW and SetFileAttributesW.

231aa9d fixed the issue with long paths (>=260 chars) by introducing a new function, os.fixLongPath, which converts the path to an extended-length path on Windows.

What did you see instead?

chmod C:\Users\ibrahim\go\src\github.com\ibrasho\deptest\vendor\github.com\prometheus\procfs\sysfs\fixtures.src\devices\pci0000_@colon@_00\0000_@colon@_00_@colon@_0d.0\ata4\host3\target3_@colon@_0_@colon@_0\3_@colon@_0_@colon@_0_@colon@_0\block\sdb\bcache\dirty_data: The system cannot find the path specified.

Applying the same logic from os.fixLongPath before passing the path to os.Chmod resolves this issue.

@ibrasho
Copy link
Contributor Author

@ibrasho ibrasho commented Jun 28, 2017

I've found this issue while investigating golang/dep#774.

I've found multiple other file related functions implemented outside os (in syscall) that are also affected. According to Naming Files, Paths and Namespaces on MSDN:

These are the directory management functions that no longer have MAX_PATH restrictions if you opt-in to long path behavior: CreateDirectoryW, CreateDirectoryExW GetCurrentDirectoryW RemoveDirectoryW SetCurrentDirectoryW.
These are the file management functions that no longer have MAX_PATH restrictions if you opt-in to long path behavior: CopyFileW, CopyFile2, CopyFileExW, CreateFileW, CreateFile2, CreateHardLinkW, CreateSymbolicLinkW, DeleteFileW, FindFirstFileW, FindFirstFileExW, FindNextFileW, GetFileAttributesW, GetFileAttributesExW, SetFileAttributesW, GetFullPathNameW, GetLongPathNameW, MoveFileW, MoveFileExW, MoveFileWithProgressW, ReplaceFileW, SearchPathW, FindFirstFileNameW, FindNextFileNameW, FindFirstStreamW, FindNextStreamW, GetCompressedFileSizeW, GetFinalPathNameByHandleW.

Here is a list of possible occurrences of this issue (where these functions were called without running the path through os.fixLongPath first:

It could possibly occur in os.SameFile since it ends up calling os.*fileStat.loadFileId which attempts CreateFileW without using os.fixLongPath.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 28, 2017

I think we previously decided that we aren't adding magic UNC path mangling to the low-level syscall package. If users are using syscall, they can read MSDN docs.

Let's keep this bug specifically about Chmod.

@ibrasho
Copy link
Contributor Author

@ibrasho ibrasho commented Jun 28, 2017

I'm can work on a fix. But I've some questions:

  • Since these functions are implemented in syscall, how are they exposed in os? Can we just create a wrapper around them in os that fixes this bug? only 2 of them need fixing.
  • syscall.Rename: is duplicated in internal/syscall/windows.Rename. Is there a reason?

edit: Actually, there is a difference. internal/syscall/windows.Rename replaces the destination if it exists, while syscall.Rename fails in that case.

@ibrasho
Copy link
Contributor Author

@ibrasho ibrasho commented Jun 28, 2017

@bradfitz In that case, even os.Remove needs the same fix. I'll submit a CL in a couple of minutes.

Edit: my mistake. I was reading the wrong file. Only os.Chmod is broken.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 28, 2017

Remove was already done in 231aa9d. I see:

// Remove removes the named file or directory.                                                                                                                                                                  
// If there is an error, it will be of type *PathError.                                                                                                                                                         
func Remove(name string) error {                                                                       
        p, e := syscall.UTF16PtrFromString(fixLongPath(name))                                          
        if e != nil {                                                                                  
                return &PathError{"remove", name, e}                                                   
        }                                                                                              
...

And Chmod should be just:

diff --git a/src/os/file_posix.go b/src/os/file_posix.go
index 6ee7eeb..5ac0acd 100644
--- a/src/os/file_posix.go
+++ b/src/os/file_posix.go
@@ -48,7 +48,7 @@ func syscallMode(i FileMode) (o uint32) {
 // If the file is a symbolic link, it changes the mode of the link's target.
 // If there is an error, it will be of type *PathError.
 func Chmod(name string, mode FileMode) error {
-       if e := syscall.Chmod(name, syscallMode(mode)); e != nil {
+       if e := syscall.Chmod(fixLongPath(name), syscallMode(mode)); e != nil {
                return &PathError{"chmod", name, e}
        }
        return nil
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 28, 2017

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

@gopherbot gopherbot closed this in cfb8404 Jun 28, 2017
@golang golang locked and limited conversation to collaborators Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.