caddy-webdav icon indicating copy to clipboard operation
caddy-webdav copied to clipboard

Permissions error with one file prevents listing the whole directory

Open mholt opened this issue 3 years ago • 19 comments

I think this is a bug or limitation in the upstream webdav library, but I noticed that when listing the contents of a directory, the whole request returns an error if just one of the files has a permissions error. Instead, that file should not be in the list, and the request should still succeed, listing the rest of the files in the directory.

mholt avatar Nov 23 '20 23:11 mholt

I encountered the same problem under Windows 10. My webdav using caddy works perfectly fine until it try to gain permission on a file/dir that has a high permission requirement.

For exemple, I cannot point root to my second internal hard drive D:\ because it does not have permission to read D:\System Volume Information which is a protected OS file present at the root of every mounted drive.

ERROR http.handlers.webdav internal handler error {"error": "open D:\\System Volume Information: Access is denied.", ...}}}

But if I point root to D:\Sharing everything works fine because there are no files that require special permissions.

occlu avatar May 05 '21 14:05 occlu

Same issue with Windows 11: the "System Volume Information" folder prevents the directory listing when pointing to a drive. I think the default behavior should be as in Windows explorer: the upper folder listing should work, but it should return an error only if you try to open the system folder for which you don't have the right. It seems the Webdav module tries to aaccess the sub-folder content or some other property for the upper folder listing when all it needs is the name, or if name is not accessible, just ignore it. What I get for the moment, is a partial listing of the folder, ending at the previous sub-folder (sorted alphabetically).

sillikk avatar Jul 28 '22 09:07 sillikk

I just upgraded the webdav package and our other dependencies for the first time in a couple years; someone want to try and see if things have been fixed?

mholt avatar Jul 28 '22 17:07 mholt

I tried downloading Caddy with the webdav plugin again from the Caddy website and it still doesn't seem to work. How can I see which version I'm using? I'm not sure this is the proper way to get the latest update?

sillikk avatar Jul 29 '22 11:07 sillikk

@sillikk If the version is implicit (not specified in the URL), then the build is probably cached on an old commit. (That's a known issue that I'm not quite sure how to solve at the moment.)

You can either specify the latest version (branch name like master or a commit SHA), or use xcaddy to build from source yourself.

Edit: I just cleared the build cache anyway, so you could just try the download again.

mholt avatar Jul 29 '22 19:07 mholt

I tried entering "master" or the latest build SHA into the version field on the caddy download page, but the download file is exactly the same. So it seems it doesn't change anything. I'll look into xcaddy when I find the time but that seems a bit more complicated...

sillikk avatar Jul 29 '22 20:07 sillikk

@sillikk Odd, that should be impossible. It worked for me. Make sure you're running the newly downloaded binary.

If you run ./caddy list-modules --versions you should see in the output:

http.handlers.webdav v0.0.0-20220728174539-14bd56b1afc9

  Non-standard modules: 1

Note the version string is a commit from yesterday.

mholt avatar Jul 29 '22 20:07 mholt

I have the same: http.handlers.webdav v0.0.0-20220728174539-14bd56b1afc9

So it seems the latest version doesn't fix the issue, I'm afraid.

sillikk avatar Jul 29 '22 20:07 sillikk

@sillikk The SHA of the binary should definitely be different. Can you confirm the two binaries have different checksums, and then verify you are performing the test using the binary with the new checksum?

(Again, it's quite possible the upstream hasn't fixed this, but there seems to be something non-sequitur here that I want to make sure we are clear on first, so we can be precise.)

mholt avatar Jul 29 '22 20:07 mholt

I'm not sure how to get the checksum of the binary. But I'm definitely using the latest version I just downloaded, with the version I just showed above.

sillikk avatar Jul 29 '22 20:07 sillikk

I'm confused, you said:

the download file is exactly the same

how do you know that without hashing it?

mholt avatar Jul 29 '22 21:07 mholt

Sorry, it is exactly the same size but I didn't hash it.

sillikk avatar Jul 29 '22 22:07 sillikk

Ok. In that case I am not sure then. Still seems like this bug should be reported upstream.

mholt avatar Jul 30 '22 21:07 mholt

Where is upstream? On the Caddy repository? Will you report the bug or should I?

sillikk avatar Jul 31 '22 17:07 sillikk

@sillikk With Go, since I believe the bug is in https://pkg.go.dev/golang.org/x/net/webdav.

I only scanned the code very quickly, but I believe the error would be somewhere around here: https://cs.opensource.google/go/x/net/+/c7608f3a:webdav/file.go;l=791;drc=c7608f3a8462fd23f95e21be49b40c7b9c20c74a;bpv=0;bpt=1 -- or really, any of the Open/Read/Stat/Readdir operations (etc) that might return a permissions error are possible causes of this behavior.

I haven't had a chance to file a bug myself, so it might be faster if you do it :sweat_smile:

mholt avatar Jul 31 '22 22:07 mholt

Well I couldn't find a way to file a bug on the two links you mentioned. Besides, I think you will probably be better informed than me to put a correct description I guess. All I could do is point to this issue on Github. Could you file the bug when you have time? Thank you.

sillikk avatar Aug 02 '22 08:08 sillikk

Sure, but I won't have time for a while. Bug report link is here: golang/go/issues/new/choose

mholt avatar Aug 02 '22 15:08 mholt

There is a PR https://github.com/golang/net/pull/91 but still under review, see also https://go-review.googlesource.com/c/net/+/285752/

I cherry-pick that commit to my fork. Add one line replace golang.org/x/net => github.com/heimoshuiyu/net v0.0.0-20220813162333-9dad79f931f2 to the end of caddy's (not caddy-webdav) go.mod can temporarily solve this problem

heimoshuiyu avatar Aug 13 '22 17:08 heimoshuiyu

Ah that's great news!! Thanks for the update!

mholt avatar Aug 13 '22 17:08 mholt

@heimoshuiyu Well, while we wait for Go to review and merge the CL, I've gone ahead and pushed the replacement to our go.mod in the meantime.

mholt avatar Sep 17 '22 04:09 mholt