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

Set last-modified time support? #19

Closed
jonasbardino opened this issue Jul 13, 2015 · 3 comments
Closed

Set last-modified time support? #19

jonasbardino opened this issue Jul 13, 2015 · 3 comments

Comments

@jonasbardino
Copy link
Contributor

Hi Martin

First of all sorry if this should not go under issues, as it may be more a question or feature request than a bug/issue.

One of our users tried out rsync -a localdir mntdir against an OSX Finder-mounted wsgidav volume and it actually mostly works, except the file time stamps are not transferred. Thus repeating the rsync call does not skip the already transferred files as intended. I repeated the test with fusedav from Linux and got similar results. Calling a simple touch on a file in the mount also gets rejected with "Operation not supported".
As far as I can tell from my tracing this is due to wsgidav refusing PROPPATCH calls with the {DAV:}getlastmodified argument. The relevant doc string in setPropertyValue of dav_provider correctly states that any {DAV:} property modification requests result in a HTTP_FORBIDDEN reply and that resource providers may override the method to "update supported custom live properties".

I did a quick test and added such set {DAV:}getlastmodified support directly in (fs_)dav_handler and then touch, rsync -t and rsync -a all work as expected. However, I'm not really sure if it belongs there or if it is even compliant with the WebDAV spec:
http://tools.ietf.org/html/rfc4918#section-15.7

It would be quite cool with rsync support for our purpose, but maybe I'm overlooking some problems it might introduce? Would it make sense to properly add such general set modification time support and maybe only enable it with a conf setting? I think I can give it a shot and send a pull request if so.
Or is that something you consider in the custom live properties category and better left for a derived fs resource provider?

Cheers, Jonas

@mar10
Copy link
Owner

mar10 commented Jul 13, 2015

Hi Jonas,

(found a related link here http://stackoverflow.com/a/12643147/19166)

Writing a property that is named 'getlastmodified' feels wrong.
But you observed that touch and rsync -t send a PROPPATCH {DAV:}getlastmodified?
In this case it would not help to support PROPPATCH {DAV:}lastmodified` (without 'get') or another custom approach.

I have seen objections that this may confuse clients that rely on If-Modified-Since headers, and the spec says it SHOULD be protected, but I am open to a configuration setting...

@jonasbardino
Copy link
Contributor Author

Thanks for the informative input and link, Martin.

Yes, I also initially went searching for a corresponding **set**lastmodified or just lastmodified in the RFC and code, but without any luck. In fact the commands both result in PROPPATCH **get**lastmodified requests.
I uploaded a trace of touch from the original at:
http://www.erda.dk/wsgidav-touch-trace.txt
and one from the version with my added support for changing it at:
http://www.erda.dk/wsgidav-touch-enabled-trace.txt

So apparently one must handle getlastmodified for it to work, but it could of course easily be extended to cover plain lastmodified requests as well.

Doing another touch after the first one resulted in first a HEAD request that received a matching
Last-Modified: 'Tue, 14 Jul 2015 06:15:02 GMT' reply, and then the actual next PROPPATCH request. I don't know if that would confuse other clients, but in our case I'd say it's the intended result.
I guess the same clients would then also be confused if a user would mimic a touch by moving the original file out of the way and then moving another copy with just a different time stamp there, because then that different time stamp is transferred.

I'm going to be mostly away for the next week or two, but will take a look at making a configurable implementation when time permits. I'm thinking something along the lines of adding support for a
mutableliveprops = ["{DAV:}getlastmodified"]
conf option and forwarding any matching live PROPPATCH requests in dav_provider.setPropertyValue if the actual provider has a corresponding setX method.
Comments and suggestions are welcome...

mar10 added a commit that referenced this issue Jul 23, 2015
Add configurable support for setting last modified file/directory timestamps (issue #19)
@jonasbardino
Copy link
Contributor Author

Hi Martin
Thanks for merging the patch!
I've switched our production to your updated version and verified that fusedav mounted touch and rsync -a works. Therefore closing the issue here.
Cheers, Jonas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants