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

Permissions error with one file prevents listing the whole directory #16

Closed
mholt opened this issue Nov 23, 2020 · 20 comments · Fixed by #33
Closed

Permissions error with one file prevents listing the whole directory #16

mholt opened this issue Nov 23, 2020 · 20 comments · Fixed by #33
Labels
help wanted Extra attention is needed

Comments

@mholt
Copy link
Owner

mholt commented Nov 23, 2020

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.

@occlu
Copy link

occlu commented May 5, 2021

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.

@sillikk
Copy link

sillikk commented Jul 28, 2022

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).

@mholt mholt added the help wanted Extra attention is needed label Jul 28, 2022
@mholt
Copy link
Owner Author

mholt commented Jul 28, 2022

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?

@sillikk
Copy link

sillikk commented Jul 29, 2022

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?

@mholt
Copy link
Owner Author

mholt commented Jul 29, 2022

@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.

@sillikk
Copy link

sillikk commented Jul 29, 2022

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...

@mholt
Copy link
Owner Author

mholt commented Jul 29, 2022

@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.

@sillikk
Copy link

sillikk commented Jul 29, 2022

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.

@mholt
Copy link
Owner Author

mholt commented Jul 29, 2022

@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.)

@sillikk
Copy link

sillikk commented Jul 29, 2022

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.

@mholt
Copy link
Owner Author

mholt commented Jul 29, 2022

I'm confused, you said:

the download file is exactly the same

how do you know that without hashing it?

@sillikk
Copy link

sillikk commented Jul 29, 2022

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

@mholt
Copy link
Owner Author

mholt commented Jul 30, 2022

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

@sillikk
Copy link

sillikk commented Jul 31, 2022

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

@mholt
Copy link
Owner Author

mholt commented Jul 31, 2022

@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 😅

@sillikk
Copy link

sillikk commented Aug 2, 2022

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.

@mholt
Copy link
Owner Author

mholt commented Aug 2, 2022

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

@heimoshuiyu
Copy link
Contributor

There is a PR golang/net#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

@mholt
Copy link
Owner Author

mholt commented Aug 13, 2022

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

@mholt
Copy link
Owner Author

mholt commented Sep 17, 2022

@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.

heimoshuiyu added a commit to heimoshuiyu/caddy-webdav that referenced this issue Dec 6, 2022
heimoshuiyu added a commit to heimoshuiyu/caddy-webdav that referenced this issue Dec 6, 2022
Upstream bug fixed and merged in golang.org/x/net

<https://go-review.googlesource.com/c/net/+/285752>
@mholt mholt linked a pull request Dec 6, 2022 that will close this issue
@mholt mholt closed this as completed in #33 Dec 6, 2022
mholt pushed a commit that referenced this issue Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants