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

proposal: add a context argument to webdav.FileSystem methods #17658

Closed
kardianos opened this issue Oct 29, 2016 · 5 comments
Closed

proposal: add a context argument to webdav.FileSystem methods #17658

kardianos opened this issue Oct 29, 2016 · 5 comments

Comments

@kardianos
Copy link
Contributor

Proposed Change

In the package golang.org/x/net/webdav there is currently no way to pass request specific parameters from the webdav request to the FileSystem interface. If the FileSystem methods took a context parameter such parameters and request scoped parameters could be put in the context. The context value would come from the http.Request.Context() method when available.

So this:

type FileSystem interface {
    Mkdir(name string, perm os.FileMode) error
    OpenFile(name string, flag int, perm os.FileMode) (File, error)
    RemoveAll(name string) error
    Rename(oldName, newName string) error
    Stat(name string) (os.FileInfo, error)
}

would turn into this:

type FileSystem interface {
    Mkdir(ctx context.Context, name string, perm os.FileMode) error
    OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (File, error)
    RemoveAll(ctx context.Context, name string) error
    Rename(ctx context.Context, oldName, newName string) error
    Stat(ctx context.Context, name string) (os.FileInfo, error)
}

Examples of where this is useful

  • Authenticate the username and password from a webdav request and provide file level authentication or remote authentication to the FileSystem.
  • If a webdav request is initiated but cancelled or the connection is dropped, any subsequent operations, esp remote operations or copy operations on large files, could be also cancelled.

Other Options

I'm not aware of any viable workarounds that would work today that could pass request specific information to the FileSystem operations. The FileSystem interface is provided to the webdav.Handler so there is no way to create some type of closure that returns a request scoped properties. Nor would you want to create an ad-hoc webdav.Handler as that would defeat the lock system.

Compatibility

This would break any custom FileSystem implementations. However updating to be compatible is trivial, just adding a context argument to file methods.

On go1.7+ the context argument would be supplied by http.Request.Context() and on eailier versions the context method would come from golang.org/x/net/context.Background(). When go1.6 support is dropped from the builders the internal xml package and the background context shim can be dropped.

/cc @nigeltao @adg

@nigeltao
Copy link
Contributor

Looks good to me, assuming that the backwards compat issue is indeed manageable.

@rsto for x/net/webdav expertise.

@rsto
Copy link
Contributor

rsto commented Oct 30, 2016

thanks for this. lgtm.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/32421 mentions this issue.

@ribrdb
Copy link
Contributor

ribrdb commented Dec 13, 2016

Doesn't the LockSystem need the same context?

@kardianos
Copy link
Contributor Author

@ribrdb I think that any lock system would use an internal context when a lock request was made based on the characteristics of your locking backend. It wouldn't need to propagate any user information.

If you think it does need the context, open a new issue explaining why.

@golang golang locked and limited conversation to collaborators Dec 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants