net
net copied to clipboard
golang
webdav: ignore path and perm errors in PROPFIND
PROPFIND can walk through directories, retrieving information about each file. Unfortunately, the filesystem may deny access to the WebDAV server for various reasons, such as the file truly not being readable (e.g. a broken symlink), or because the server does not have permission to read the file. PROPFIND should ignore these.
The current behaviour of the WebDAV server when encountering such issues is to immediately stop its walk, and output an http 500. This leads to poor behaviour with the builtin golang server, since the walk has likely already written out its status header as part of serving the previously walked files' properties. The server closes the response, also emitting an error log.
While the error log is noisy, the closed response is truly an issue: the xml returned to the client is invalid, which means that the response is useless. It is not unreasonable to expect that a directory shared using WebDAV has files which cannot be read for the reasons given above. The shared directory becomes useless with the current behavior.
Rather than making directories with unreadable files useless, skip over anything that is bad. A more nuanced solution to this problem could likely involve indicating that the requested properties have problems, or buffering the response prior to returning the failure. However, this change is simple and a move in the right direction.
Fixes golang/go#16195 Fixes golang/go#43782
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
![google-cla[bot] avatar](https://avatars.githubusercontent.com/in/42202?v=4 "Published by a pub.dev verified publisher"){kind=small}
re the CLA failing, we just signed the corporate CLA. I'm in the group, so it probably just needs to be accepted (at least according to the troubleshooting docs).
What corporation? That is, what name was used to sign the corporate CLA?
What corporation? That is, what name was used to sign the corporate CLA?
It would have been Agilicus (or some variation thereof. My boss filled out the form).
This PR (HEAD: d56917e02885fb4151c0d6d8303be3e70dd4aa7a) has been imported to Gerrit for code review.
Please visit https://go-review.googlesource.com/c/net/+/285752 to see it.
Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info
@ianlancetaylor I don't like poking people like this, but I have a series of webdav reviews that have been open for ~5 months with no feedback at all. Is there anything I can do to expedite them? I'd rather close them off while the issues are still somewhat fresh in my mind, and the codebase hasn't wandered away too much.
Poking people is fine. Poking on the Gerritt CL is better (https://golang.org/cl/285752). I know nothing about the webdav code, I'll see if I can find someone who does.
Message from Ian Lance Taylor:
Patch Set 1:
(1 comment)
Please don’t reply on this GitHub thread. Visit golang.org/cl/285752. After addressing review feedback, remember to publish your drafts!
Message from Ian Lance Taylor:
Patch Set 1:
(1 comment)
Please don’t reply on this GitHub thread. Visit golang.org/cl/285752. After addressing review feedback, remember to publish your drafts!
@ianlancetaylor I don't like poking people like this, but I have a series of webdav reviews that have been open for ~5 months with no feedback at all. Is there anything I can do to expedite them? I'd rather close them off while the issues are still somewhat fresh in my mind, and the codebase hasn't wandered away too much.
It seems really difficult to get a patch accepted in the net and crypto repositories. I stopped trying, if anyone is interested I am maintaining a fork of net and crypto with the necessary patch for SFTPGo. For webdav I included some patches here and added lock discovery support
https://github.com/drakkan/net/commits/sftpgo https://github.com/drakkan/crypto/commits/sftpgo
I periodically rebase my branches against master
Any updates for this patch?
I haven't been pushing it from my side. I'd like for it to be integrated to the x/net repo, but I don't really have time to wrangle the reviewers, so unfortunately, I've just left it hanging.
I'd like to say, that this is definitely a step in the right direction and is a bit sad to see this trivial and obvious problem existing for around 6 years in the golang standard library now.
Even more so, when someone took the time to research it, fix it, test it, upload it, legalise it and publish it.
Is there something that can be done to speed this up?
