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.

Sign up for GitHub

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

Jump to bottom

x/net/webdav: microsoft miniredirector fails setting properties of directories #43929

Open
klarose opened this issue Jan 26, 2021 · 4 comments · May be fixed by golang/net#94
Open

x/net/webdav: microsoft miniredirector fails setting properties of directories #43929

klarose opened this issue Jan 26, 2021 · 4 comments · May be fixed by golang/net#94
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unreleased

Comments

@klarose
Copy link

klarose commented Jan 26, 2021

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

$ go version
go version go1.15 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/kyle/.cache/go-build"
GOENV="/home/kyle/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/kyle/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/kyle/"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build009155035=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Ran a simple wrapper around x/net/webdav. Pointed the windows mini redirector (from windows 10) at it, then tried to clone golang/net: git clone https://github.com/golang/net.git

What did you expect to see?

The git clone succeed.

What did you see instead?

Z:\>git clone https://github.com/golang/net.git
Cloning into 'net'...
//files.kyle-shares.agilicus.cloud@SSL/DavWWWRoot/agent/net/.git: Input/output error

Digging in, it's clear that the issue is due to windows trying to set properties directories such as ./.git.

I added some logging to the proppatch handler and found it doing these operations:

2021/01/26 12:16:21 Patching /net {urn:schemas-microsoft-com: Win32CreationTime}
2021/01/26 12:16:21 Patching /net {urn:schemas-microsoft-com: Win32LastAccessTime}
2021/01/26 12:16:21 Patching /net {urn:schemas-microsoft-com: Win32LastModifiedTime}
2021/01/26 12:16:21 Patching /net {urn:schemas-microsoft-com: Win32FileAttributes}
2021/01/26 12:16:21 Fail on patch: open /home/kyle/shared/net: is a directory
@gopherbot gopherbot added this to the Unreleased milestone Jan 26, 2021
@klarose
Copy link
Author

klarose commented Jan 26, 2021

I was able to address this in testing by first checking if the file was a directory, then returning forbidden for every patch. That seemed to let it move forward at least. We may want to look at a more abstract way of handling this, but I'll submit a patch for now, since we need this to be fixed for our usecase.

klarose added a commit to Agilicus/golang-net that referenced this issue Jan 26, 2021
@klarose
webdav: return forbidden on failed proppatch
ee2d1dd 
Some file systems do not allow opening directories with a write flag.
The PROPPATCH implementation always opens all files it tries to patch
with O_RDWR. When running on Linux with the Dir implementation of
FileSystem, this leads to all PROPPATCH calls to a directory failing
with an Internal Server Error.

Some clients, such as git via the Microsoft Mini Redirector try to patch
directories' properties. A server using a Dir FileSystem fails in this
situation, which makes it unusable for that entire class of clients.

Rather than failing with a Internal Server Error we can instead return
Forbidden, which indicates to the client that it's not allowed to patch
properties on that file. This patch does so if we fail to open the file,
and the error is because of a lack of permission, or because the file is
a directory. We don't unconditionally return Forbidden in this case
because other errors are more likey to be ephemeral: a client retrying
could possibly succeed in a subsequent patch.

Fixes golang/go#43929
@klarose klarose linked a pull request Jan 26, 2021 that will close this issue
Open
klarose added a commit to Agilicus/golang-net that referenced this issue Jan 26, 2021
@klarose
webdav: return forbidden on failed proppatch
5768315 
Some file systems do not allow opening directories with a write flag.
The PROPPATCH implementation always opens all files it tries to patch
with O_RDWR. When running on Linux with the Dir implementation of
FileSystem, this leads to all PROPPATCH calls to a directory failing
with an Internal Server Error.

Some clients, such as git via the Microsoft Mini Redirector try to patch
directories' properties. A server using a Dir FileSystem fails in this
situation, which makes it unusable for that entire class of clients.

Rather than failing with a Internal Server Error we can instead return
Forbidden, which indicates to the client that it's not allowed to patch
properties on that file. This patch does so if we fail to open the file,
and the error is because of a lack of permission, or because the file is
a directory. We don't unconditionally return Forbidden in this case
because other errors are more likey to be ephemeral: a client retrying
could possibly succeed in a subsequent patch.

Fixes golang/go#43929
@gopherbot
Copy link

Change https://golang.org/cl/286992 mentions this issue: webdav: return forbidden on failed proppatch

@ncw
Copy link
Contributor

ncw commented Jan 27, 2021

This has come up with rclone serve webdav very recently - see https://forum.rclone.org/t/writeable-readonly-directories/21919/

Rclone already returns EPERM when you try to open a directory Read/Write and Windows seems to accept that as OK as it doesn't report an error. However it makes a nasty ERROR in the log.

Your patch looks like it will solve our problem too.

Did you try your webdav implementation with the Windows explorer?

@klarose
Copy link
Author

klarose commented Jan 27, 2021

@ncw awesome, I'm glad to hear it'll fix the problem for you!

I tested using both linux davfs2 and Windows Explorer (via map network drive). The issue was only present when using Windows Explorer. With my patch in place, the issue did not occur at all.

@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

webdav: return forbidden on failed proppatch Agilicus/golang-net
4 participants
@toothrot @ncw @gopherbot @klarose