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

OC ext storage PROPFIND recreates etag for every call #15125

Closed
PVince81 opened this issue Mar 23, 2015 · 10 comments · Fixed by #15895
Closed

OC ext storage PROPFIND recreates etag for every call #15125

PVince81 opened this issue Mar 23, 2015 · 10 comments · Fixed by #15895

Comments

@PVince81
Copy link
Contributor

Steps to reproduce

  1. Setup two 8.0.2 instances F (Frontend) and B (Backend)
  2. On F, enable external storage app
  3. Mount B using the "ownCloud" external storage using the mount point "mount"
  4. Setup the sync client for F
  5. Create a local file "mount/test.txt" and wait for sync-upload
  6. Modify the local file "mount/test.txt" and wait for sync-upload

Expected result

File is overwritten properly

Actual result

Sync client says there is a conflict and creates a local conflict file.

Versions

Both ownCloud 8.0.2

Logs

Frontend OC log:

{"reqId":"87e555ad2eed1ae36ff414908dbe4406","remoteAddr":"127.0.0.1","app":"webdav","message":"Exception: {\"Message\":\"An If-Match header was specified, but none of the specified the ETags matched.\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/connector\\\/sabre\\\/server.php(70): Sabre\\\\DAV\\\\Server->checkPreconditions(false)\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/Sabre\\\/DAV\\\/Server.php(878): OC_Connector_Sabre_Server->checkPreconditions()\\n#2 [internal function]: Sabre\\\\DAV\\\\Server->httpPut('folder\\\/o.txt')\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/Sabre\\\/DAV\\\/Server.php(474): call_user_func(Array, 'folder\\\/o.txt')\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/Sabre\\\/DAV\\\/Server.php(214): Sabre\\\\DAV\\\\Server->invokeMethod('PUT', 'folder\\\/o.txt')\\n#5 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/files\\\/appinfo\\\/remote.php(61): Sabre\\\\DAV\\\\Server->exec()\\n#6 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(54): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#7 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/Sabre\\\/DAV\\\/Server.php\",\"Line\":1962}","level":0,"time":"2015-03-23T16:30:06+00:00","method":"PUT","url":"\/owncloud\/remote.php\/webdav\/folder\/o.txt"}

Backend OC: nothing

Note: mount points that use server to server sharing work fine with overwrite, this seems to be specific to external storages.

@PVince81
Copy link
Contributor Author

Hmm, it looks like every "hasUpdated" call causes the file's etag to change. So basically in checkPreconditions of the PUT operation, it sends the correct etag. And while we try and verify it, it does a hasUpdated call which itself changes the etag, and then it doesn't match => precondition failed.

@PVince81
Copy link
Contributor Author

This can be observed by running:

curl -X PROPFIND http://root:admin@localhost/owncloud/remote.php/webdav/mount/test.txt | xmllint --format -

and seeing that for every call, the etag of the file changes!

@PVince81
Copy link
Contributor Author

Ok I think I got it:
It works properly with federation because the etag is read through the shareinfo.php call, and then set directly into the local cache.
It does NOT work when mounting OC directly because there is no logic to store the remote etag locally, so the local etag will ALWAYS be different than the remote one.

There are two possible fixes for this, either:

  1. Disable the etag comparison during update detection in non-federation case:
    or
  2. Implement ownCloud::getEtag() to retrieve the etag from the remote server (but this might fail for generic WebDAV storages, so put it on the OwnCloud ext storage backend). But here we still need a fallback in case no remote etag was returned, and make it detectable through the logic from 1).

Another problem: we currently do not use the OwnCloud ext storage impl for federation, so we cannot simply move logic there...

CC @icewind1991 in case you got a better idea

@PVince81 PVince81 changed the title OC ext storage overwrite produces conflict files OC ext storage PROPFIND recreates etag for every call Apr 27, 2015
@PVince81
Copy link
Contributor Author

This PR fixes the OC ext storage resync/conflict, but will break DAV servers: #15895
Needs further research

@PVince81
Copy link
Contributor Author

Also just observed on stable7.

Basically OC ext storage backend is broken with sync since the introduction of s2s/federation.

@karlitschek
Copy link
Contributor

@icewind1991

@nickvergessen
Copy link
Contributor

This also causes server2server shares to be synced every minute

@PVince81
Copy link
Contributor Author

Maybe some code path is bypassing the shareinfo.php logic

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 2, 2015

Looks like with s2s it is also redownloading the same file (single file share through s2s) over and over again. It doesn't seem to go through shareinfo.php, so it is likely that it is re-generating a new etag for every access too.

@PVince81
Copy link
Contributor Author

PVince81 commented Jun 2, 2015

This PR fixes it: #15895

But not sure about the consequences with regular WebDAV servers that do not return etags. It would just fallback to re-generating a new etag as it did before. On the other hand, "hasUpdated()" will not rely on remote etags in such cases but only on mtimes. So might be fine.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants