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

When using software written in the Go language, directories reportedly fail to open #406

Closed
4 tasks done
jetwhiz opened this issue Nov 28, 2016 · 26 comments
Closed
4 tasks done
Assignees

Comments

@jetwhiz
Copy link

jetwhiz commented Nov 28, 2016

Environment

  • Windows version: Windows 7
  • Processor architecture: x86 and x64
  • Dokany version: 1.0.1.1000
  • Library type (Dokany/FUSE): FUSE

Check List

  • I checked my issue doesn't exist yet
  • My issue is valid with mirror default sample and not specific to my user-mode driver implementation (fuse mirror)
  • I can always reproduce the issue with the provided description below.
  • I have updated Dokany to the latest version and have reboot my computer after.
  • [?] I tested one of the last snapshot from appveyor CI

Description

This is based on various bug reports that involve encfs4win:

From what I can tell, the Go programming language will attempt to open everything as a file first and then (only if that fails) does it try opening it as a directory (see https://github.com/golang/go/blob/master/src/os/file_windows.go#L150).

This is apparently causing an issue with Dokany FUSE, since it looks like Dokany does not fail when attempting to open a directory as a file.

For instance, the rclone program (written in Go), fails with the following error when trying to copy files out of the Dokany fuse mirror:

With UNC paths:

Local file system at \\?\X:: Failed to open directory: \\?\X:: Readdir \\?\X:: The system cannot find the path specified.

Without UNC paths:

Local file system at X:: failed to read directory "X:\": Readdir X:: The system cannot find the path specified.

Logs

@qnorsten

This example was pulled off of my bug report: http://pastebin.com/QjeLcXY9

@jetwhiz
Copy link
Author

jetwhiz commented Nov 28, 2016

To fix the FUSE mirror, it might be enough to just do an lstat and S_ISDIR before calling open:

https://github.com/dokan-dev/dokany/blob/master/samples/fuse_mirror/fusexmp.c#L246

Since open() will not complain if a directory is passed in (unless write permissions are requested):

https://linux.die.net/man/3/open

But I'm not sure if the problem goes deeper in Dokany or not.

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

The more I think about this, the less certain I am that the fix should be on our end (encfs/dokany).

Under Linux, when someone calls open() on a directory, an error is not given unless write permissions are requested (EISDIR error).

Similarly, on Windows, CreateFile will not return an error as long as OPEN_EXISTING is used (directory already exists) or the FILE_FLAG_BACKUP_SEMANTICS flag is specified.

It seems inapproproate for Go to be expecting an error when requesting to open an existing directory in read-only mode (since both Windows and Linux clearly do not see this as an error situation).

@Liryna -- what do you think?

@taruti
Copy link
Contributor

taruti commented Nov 29, 2016

btw We use Go successfully to access a filesystem using dokan1.sys.

@jetwhiz, to make Go work you need to check flags carefully in CreateFile and return an error if it is a directory and the request does not have the necessary flags.

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

Hi @taruti -- this is mainly regarding the fuse_mirror sample, as it does not perform the check you describe (and so software using Go will fail awkwardly on fuse_mirror). see https://github.com/dokan-dev/dokany/blob/master/samples/fuse_mirror/fusexmp.c#L246

I guess my main concern is that opening a directory as a file is not considered an error (in Linux and Windows) unless you request write permissions. Adding a check to fix the issue with Go's implementation seems to go against expected behavior for a filesystem, however.

IMO, Go should be the one to make the appropriate request (there is an important distinction between opening a directory as a file or as a directory, both of which are valid and do not return errors). If Go wants to open a directory as a directory, then it should explicitly make that request (rather than relying on non-standard behavior).

@taruti
Copy link
Contributor

taruti commented Nov 29, 2016

@jetwhiz iirc the filesystem should be checking for FILE_DIRECTORY_FILE and FILE_NON_DIRECTORY_FILE from ZwCreateFile but the fuse mapping layer makes that kind of hard...

Perhaps the dokan fuse mapping layer should handle these transparently? Since here Windows and Linux semantics are different.

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

Ah I understand, so Go is sending the FILE_NON_DIRECTORY_FILE flag on Windows when it attempts to open a file (to force a fail for directories)?

It might be better if the Dokany FUSE API handles this then since that flag is not currently passed through the API from what I can see?

Alternatively, the newest POSIX version has the O_DIRECTORY creation flag (it seems O_DIRECTORY is the opposite of FILE_NON_DIRECTORY_FILE, though, and only tangentially-related to FILE_DIRECTORY_FILE).

@Liryna
Copy link
Member

Liryna commented Nov 29, 2016

Hi @jetwhiz ,

Thank you for the report !

dokan fuse call mkdir/opendir if CreateOptions has the flag FILE_DIRECTORY_FILE.

if ((CreateOptions & FILE_DIRECTORY_FILE) == FILE_DIRECTORY_FILE) {

Normally when FILE_NON_DIRECTORY_FILE is set, it means that FILE_DIRECTORY_FILE is not set. So here will be called open .

After here, we don't check if FILE_FLAG_BACKUP_SEMANTICS is set (it is a flag the give the possibility to open a folder as a flag. It would be interesting to know if Go is using this to open folder.
I looked at your logs but they are pretty long 😄 Could it be possible to have a simple log with a go application that open a directory ?

@qnorsten
Copy link

qnorsten commented Nov 29, 2016

Hi @Liryna
I am the one who initially posted the issue over at encfs4win (but not the issue at rclone).
Here is a log when running the fuse_mirror sample. with mirror.exe -d X: followed by trying to copy the content of X: to the cloud with rcloud (go application that try to read the file)

Log file: http://pastebin.com/5e5DgKhG

Hopefully this is a bit shorter then before everything until Close 0016 or Close 0017 was before the go application tried to read the files.

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

@Liryna @taruti -- I'm not very familiar with Go, so I can't say one way or another what is happening behind the scenes there. From what I found here: https://github.com/golang/go/blob/master/src/syscall/syscall_windows.go#L248

func Open(path string, mode int, perm uint32) (fd Handle, err error) {
	if len(path) == 0 {
		return InvalidHandle, ERROR_FILE_NOT_FOUND
	}
	pathp, err := UTF16PtrFromString(path)
	if err != nil {
		return InvalidHandle, err
	}
	var access uint32
	switch mode & (O_RDONLY | O_WRONLY | O_RDWR) {
	case O_RDONLY:
		access = GENERIC_READ
	case O_WRONLY:
		access = GENERIC_WRITE
	case O_RDWR:
		access = GENERIC_READ | GENERIC_WRITE
	}
	if mode&O_CREAT != 0 {
		access |= GENERIC_WRITE
	}
	if mode&O_APPEND != 0 {
		access &^= GENERIC_WRITE
		access |= FILE_APPEND_DATA
	}
	sharemode := uint32(FILE_SHARE_READ | FILE_SHARE_WRITE)
	var sa *SecurityAttributes
	if mode&O_CLOEXEC == 0 {
		sa = makeInheritSa()
	}
	var createmode uint32
	switch {
	case mode&(O_CREAT|O_EXCL) == (O_CREAT | O_EXCL):
		createmode = CREATE_NEW
	case mode&(O_CREAT|O_TRUNC) == (O_CREAT | O_TRUNC):
		createmode = CREATE_ALWAYS
	case mode&O_CREAT == O_CREAT:
		createmode = OPEN_ALWAYS
	case mode&O_TRUNC == O_TRUNC:
		createmode = TRUNCATE_EXISTING
	default:
		createmode = OPEN_EXISTING
	}
	h, e := CreateFile(pathp, access, sharemode, sa, createmode, FILE_ATTRIBUTE_NORMAL, 0)
	return h, e
}

It looks like they're using "OPEN_EXISTING" by default, but I don't see the FILE_FLAG_BACKUP_SEMANTICS flag being set.

@Liryna
Copy link
Member

Liryna commented Nov 29, 2016

Thank you @qnorsten
Looks like all Creatfile seems to return success 😮 and you got an error during the copy ?
If you create a hello world app that open a folder, you get an error ?

@jetwhiz ty I found nothing in the file how they open a folder.

@taruti
Copy link
Contributor

taruti commented Nov 29, 2016

In Go syscall.Open is called from https://github.com/golang/go/blob/master/src/os/file_windows.go#L150 for normal file opens.

@qnorsten
Copy link

qnorsten commented Nov 29, 2016

@Liryna yes I get the error described above
Local file system at \?\X:: Failed to open directory: \?\X:: Readdir \?\X:: The system cannot find the path specified.
No files could be read or copied at all, with go. If I try and copy one specific file like X:/Cygwin.bat it works, but not if I try and copy a dir or the root of the mounted drive.

I have never programmed anything in Go, but I might be able to try and write something up tomorrow, if no one else is willing to install go and try and write a basic hello world load dir application.

I also found this test program (https://gist.github.com/klauspost/5f87caf402a8abf369d5#file-test-go-L42) from rclone/rclone#261 (comment) that might be useful as it is or modified.

Update: The output of the above test program

 go run c:\go\bin\testIO.go X:
Current dir: "X:"
Main dir Stat:
Name "."
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx
2016/11/29 23:02:55 ioutil.ReadDirReaddir X:: Det går inte att hitta sökvägen. 
exit status 1

"Det går inte att hitta sökvägen." is Swedish for roughly "The Path can not be found"

Here is the dokan log:

mirror X: -d
Dokan: debug mode on
Dokan: use stderr
AllocationUnitSize: 512 SectorSize: 512
device opened
###QueryVolumeInfo -001
GetVolumeInformation
###Create 0000
CreateFile: \
        DesiredAccess: SYNCHRONIZE
        ShareAccess: 0x0
        Disposition: FILE_OPEN (1)
        Attributes: 0 (0x0)
        Options: 33 (0x21)
CreateFile status = 0
###GetFileInfo 0000
GetFileInfo: : \
        result =  0
  unknown type:54
        DispatchQueryInformation result =  c000000d
###GetFileInfo 0000
GetFileInfo: : \
        result =  0
        FileStandardInformation
        DispatchQueryInformation result =  0
###QueryVolumeInfo 0000
GetVolumeInformation
###QueryVolumeInfo 0000
GetVolumeInformation
###Cleanup 0000
Cleanup: \

###Close 0000
Close: \

mounted: X: -> \Volume{d6cc17c5-1714-4085-bce7-964f1e9f5de9}
Mounted
###Create 0001
CreateFile: \
        DesiredAccess: SYNCHRONIZE
        ShareAccess: FILE_SHARE_WRITE|FILE_SHARE_READ
        Disposition: FILE_OPEN (1)
        Attributes: 0 (0x0)
        Options: 33 (0x21)
CreateFile status = 0
###GetFileInfo 0001
GetFileInfo: : \
        result =  0
        FileStandardInformation
        DispatchQueryInformation result =  0
###Cleanup 0001
Cleanup: \

###Close 0001
Close: ###Create 0002
\CreateFile:
\
        DesiredAccess: FILE_GENERIC_READ
        ShareAccess: FILE_SHARE_WRITE|FILE_SHARE_READ
        Disposition: FILE_OPEN (1)
        Attributes: 128 (0x80)
        Options: 96 (0x60)
CreateFile status = 0
###GetFileInfo 0002
GetFileInfo: : \
        result =  0
        FileStandardInformation
        DispatchQueryInformation result =  0
###QueryVolumeInfo 0002
GetVolumeInformation
###GetFileInfo 0002
GetFileInfo: : \
        result =  0
        FileAllInformation
        DispatchQueryInformation result =  80000005
###Create 0003
CreateFile: \
        DesiredAccess: SYNCHRONIZE
        ShareAccess: FILE_SHARE_WRITE|FILE_SHARE_READ
        Disposition: FILE_OPEN (1)
        Attributes: 0 (0x0)
        Options: 33 (0x21)
CreateFile status = 0
###Cleanup 0003
Cleanup: \

###Close 0003
Close: ###Create 0004
\CreateFile:
\
        DesiredAccess: FILE_GENERIC_READ
        ShareAccess: FILE_SHARE_WRITE|FILE_SHARE_READ
        Disposition: FILE_OPEN (1)
        Attributes: 128 (0x80)
        Options: 96 (0x60)
CreateFile status = 0
###Cleanup 0004
Cleanup: \

###Close 0004
Close: \

###Cleanup 0002
Cleanup: \

###Close 0002
Close: \

If I ran the above test program on a folder c:\test that contains a file test.txt and a folder named test folder I get the following output:

$ go run c:\go\bin\testIO.go C:\test
Current dir: "C:\test"
Main dir Stat:
Name "test"
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx

Readdir results:
---
Name "test.txt"
Size 0
IsDir false
Mode.IsDir false
Mode.IsRegular true
Mode.String -rw-rw-rw-
Mode.Perm: -rw-rw-rw-
---
Name "testfolder"
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx
Success

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

@qnorsten -- can you replace this line:

f, err := os.Open(dir)

with:

f, err := os.OpenFile(dir, os.O_RDWR | os.O_CREATE, 0666)

And see what happens?

@qnorsten
Copy link

qnorsten commented Nov 29, 2016

@jetwhiz still get the same error message and the Dokan log looks the same to me

Update: Seems like the part that gives an error is not

f, err := os.Open(dir)

but instead

info, err := ioutil.ReadDir(dir)

https://github.com/golang/go/blob/master/src/io/ioutil/ioutil.go#L93

@jetwhiz
Copy link
Author

jetwhiz commented Nov 29, 2016

I don't think that shouldn't be happening -- fuse_mirror should be failing with an EISDIR errno (https://github.com/dokan-dev/dokany/blob/master/samples/fuse_mirror/fusexmp.c#L246) in that situation if I understand this correctly. https://www.gnu.org/software/libc/manual/html_node/Opening-and-Closing-Files.html

Unless building with Cygwin leads to a different (non-POSIX) open() syscall behavior.

@Liryna
Copy link
Member

Liryna commented Dec 6, 2016

@jetwhiz Just an update on this, do you think there is an issue on dokan for now ? or it is open() that seems to not return the correct error ?

@Rondom
Copy link
Contributor

Rondom commented Dec 17, 2016

Is there some .exe I could try to reproduce this without setting up a dev-env for a language I have never used?

@qnorsten
Copy link

@Rondom perhaps you can use the test program mentioned earlier from rclone/rclone#261 (comment) https://github.com/ncw/rclone/files/79771/test.zip it comes as an exe

@alexbrainman
Copy link

I have created small Go program https://play.golang.org/p/DrtghDt-48 to demonstrate what Go does to open directory and file. You would have to create c:\dir directory and c:\dir\file.txt file before running the program.

Running it on my computer, the program prints:

c:\dir: Access is denied.
c:\dir\file.txt: <nil>

So calling Windows CreateFile for my directory returns ERROR_ACCESS_DENIED. I think this is expected. According to https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx "You must set this flag to obtain a handle to a directory." (FILE_FLAG_BACKUP_SEMANTICS). And Go does not pass FILE_FLAG_BACKUP_SEMANTICS to Windows CreateFile.

I hope my Go program is simple enough, so you can translate it into C or whatever. Alternatively, you can download Go and build it yourselves. It is quite easy.

Let me know if I can help in any way.

Alex

@Liryna Liryna self-assigned this Feb 4, 2017
@Liryna
Copy link
Member

Liryna commented Feb 5, 2017

Hi @qnorsten @alexbrainman ,

Sorry for the delay, I had the time to look on this.
I tested @qnorsten test.exe and @alexbrainman sample and found no issue (maybe something that I didnt get).

M:\>test.exe
Current dir: "M:\"
Main dir Stat:
Name "\"
Size 8192
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx

Readdir results:
---
Name "Go"
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx
---
Name "Users"
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String dr-xr-xr-x
Mode.Perm: -r-xr-xr-x
---
Name "Windows"
Size 0
IsDir true
Mode.IsDir true
Mode.IsRegular false
Mode.String drwxrwxrwx
Mode.Perm: -rwxrwxrwx
---
Name "pagefile.sys"
Size 1476395008
IsDir false
Mode.IsDir false
Mode.IsRegular true
Mode.String -rw-rw-rw-
Mode.Perm: -rw-rw-rw-
---
Name "test.exe"
Size 2677760
IsDir false
Mode.IsDir false
Mode.IsRegular true
Mode.String -rw-rw-rw-
Mode.Perm: -rw-rw-rw-
Success
C:\Users\liryna\Desktop>go run main.go
c:\dir: Access is denied.
c:\dir\file.txt: The system cannot find the file specified.

C:\Users\liryna\Desktop>go run main.go
m:\dir: Access is denied.
m:\dir\file.txt: The system cannot find the file specified.

Maybe I come too late 😢 or maybe go has changed something ? (I use go 1.7.5)

@qnorsten
Copy link

qnorsten commented Feb 5, 2017

That is strange @Liryna. Did you try it with the dokany fuse mirror sample? Cause it did not work for me last I tried.

@Liryna
Copy link
Member

Liryna commented Feb 5, 2017

ohhh sorry >< so it is with FUSE that the issue only happen

@qnorsten
Copy link

qnorsten commented Feb 5, 2017

Yes it only a problem with fuse both the fuse sample and encfs4win at least

@Liryna
Copy link
Member

Liryna commented Feb 5, 2017

Fixed with 4860b95
The directory was open as a file but FILE_NON_DIRECTORY_FILE was not checked.

Thank all of you for the report and sorry again. This was quick to find but had no time to do it 😢

@qnorsten
Copy link

qnorsten commented Feb 5, 2017

Just tried the lastest snapshot from appveyor CI. Now everything works perfectly. Thanks for the fix Liryna :)

@jetwhiz
Copy link
Author

jetwhiz commented Mar 2, 2017

Thanks for fixing this @Liryna ! And sorry for the long hiatus. I moved at the beginning of this year to another country and have been swamped. I'll get this fix pulled into encfs4win as soon as I can, though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants