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

Client uses old chunking protocol with new DAV endpoint "Precondition failed" "An If-Match header was specified and the resource did not exist" #5855

Closed
PVince81 opened this issue Jun 23, 2017 · 19 comments
Assignees
Labels
p2-high Escalation, on top of current planning, release blocker type:bug

Comments

@PVince81
Copy link
Contributor

Steps

Follow the steps from owncloud/core#28197 and observe access log.

The linked ticket is about a received shared file (not folder, not file in shared folder).
When upload+overwriting this special file (which is also a shared mount / mount point), the client somehow decides to use the old chunking approach but with the new endpoint:

{"reqId":"sFrpwigItQogTHW5WblS","level":0,"time":"2017-06-23T13:28:48+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud\/remote.php\/dav\/files\/admin\/data1.dat-chunking-3886638851-13-1","message":"Exception: {\"Message\":\"HTTP\\\/1.1 412 An If-Match header was specified and the resource did not exist\",\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\PreconditionFailed\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(474): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(229): Sabre\\\\DAV\\\\Server->exec()\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(165): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#5 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php\",\"Line\":1309,\"User\":\"admin\"}"}
{"reqId":"gvl4Qhlae2wIh7VHzOHt","level":0,"time":"2017-06-23T13:28:48+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud\/remote.php\/dav\/files\/admin\/data1.dat-chunking-3886638851-13-2","message":"Exception: {\"Message\":\"HTTP\\\/1.1 412 An If-Match header was specified and the resource did not exist\",\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\PreconditionFailed\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(474): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(229): Sabre\\\\DAV\\\\Server->exec()\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(165): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#5 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php\",\"Line\":1309,\"User\":\"admin\"}"}
{"reqId":"8zYlEnfzBMb1ChuIrY9M","level":0,"time":"2017-06-23T13:28:48+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud\/remote.php\/dav\/files\/admin\/data1.dat-chunking-3886638851-13-0","message":"Exception: {\"Message\":\"HTTP\\\/1.1 412 An If-Match header was specified and the resource did not exist\",\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\PreconditionFailed\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(474): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(229): Sabre\\\\DAV\\\\Server->exec()\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(165): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#5 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php\",\"Line\":1309,\"User\":\"admin\"}"}
{"reqId":"MKPYz2yrKVCVcwd7rkzS","level":0,"time":"2017-06-23T13:28:51+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud\/remote.php\/dav\/files\/admin\/data1.dat-chunking-3246638472-13-2","message":"Exception: {\"Message\":\"HTTP\\\/1.1 412 An If-Match header was specified and the resource did not exist\",\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\PreconditionFailed\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(474): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(229): Sabre\\\\DAV\\\\Server->exec()\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(165): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#5 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php\",\"Line\":1309,\"User\":\"admin\"}"}
{"reqId":"ETCSfrMC8JaUaMztJM7w","level":0,"time":"2017-06-23T13:28:51+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"webdav","method":"PUT","url":"\/owncloud\/remote.php\/dav\/files\/admin\/data1.dat-chunking-3246638472-13-1","message":"Exception: {\"Message\":\"HTTP\\\/1.1 412 An If-Match header was specified and the resource did not exist\",\"Exception\":\"Sabre\\\\DAV\\\\Exception\\\\PreconditionFailed\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(474): Sabre\\\\DAV\\\\Server->checkPreconditions(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/lib\\\/Server.php(229): Sabre\\\\DAV\\\\Server->exec()\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#4 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(165): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#5 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php\",\"Line\":1309,\"User\":\"admin\"}"}

If you do the same test with a regular file that you overwrite, it will use the new chunking approach with the "uploads" folder:

127.0.0.1 - admin [23/Jun/2017:15:53:36 +0200] "MKCOL /owncloud/remote.php/dav/uploads/admin/1226433652 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"
127.0.0.1 - admin [23/Jun/2017:15:53:36 +0200] "PUT /owncloud/remote.php/dav/uploads/admin/1226433652/00000000 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"
127.0.0.1 - admin [23/Jun/2017:15:53:37 +0200] "PUT /owncloud/remote.php/dav/uploads/admin/1226433652/00000001 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"

Is there a special logic in place that makes it fall back to old chunking and keeps the new dav endpoint ?

I'd expect it to always use the "uploads" folder for any uploads when using the new dav endpoint.

The error that happens on the server is not related, to be investigated in owncloud/core#28197.

In general I except all three approaches to be valid:

  1. old chunking on old endpoint
  2. old chunking on new endpoint
  3. new chunking on new endpoint ("uploads")

@guruz

@PVince81
Copy link
Contributor Author

Desktop client version owncloud-client-2.3.2-1.5.x86_64 on openSUSE Tumbleweed.

@PVince81
Copy link
Contributor Author

Going to update the report, there is something about file sizes

@guruz
Copy link
Contributor

guruz commented Jun 23, 2017

@jturcotte @ogoffart I think this is about https://github.com/owncloud/client/blob/v2.3.2/src/libsync/syncengine.cpp#L520 blindly overwriting size in the second treewalk (=remote treewalk).
The remote file is the smaller one.
So it then later decides it will use v1 (because below chunk size).
Even though the actual to be uploaded real file size is bigger.

(I wonder if this has other implications, e.g. progress reporting? maybe not)

@PVince81
Copy link
Contributor Author

Steps:

  1. Setup OC 10.0.3
  2. Upload a small file "small.dat"
  3. Upload a big file "big.dat" (100+ MB)
  4. Setup Desktop client 2.3.2
  5. Overwrite "big.dat" with another big file, keeping the target name
  6. Overwrite "small.dat" with another big file, but keeping the target name
  7. Observe access_log during the upload

Expected

Both overwrite operations use the "uploads" chunking approach.

Actual

When overwriting the small file, it will use the old chunking approach (ugly file names) on the new endpoint.

For big:

127.0.0.1 - admin [23/Jun/2017:16:29:30 +0200] "MKCOL /owncloud/remote.php/dav/uploads/admin/4177904523 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"
127.0.0.1 - admin [23/Jun/2017:16:29:31 +0200] "PUT /owncloud/remote.php/dav/uploads/admin/4177904523/00000000 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"
127.0.0.1 - admin [23/Jun/2017:16:29:31 +0200] "PUT /owncloud/remote.php/dav/uploads/admin/4177904523/00000001 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/2.3.2"

For overwriting small.dat:

127.0.0.1 - admin [23/Jun/2017:16:30:10 +0200] "PUT /owncloud/remote.php/dav/files/admin/small.dat-chunking-3428624913-13-2 HTTP/1.1" 412 297 "-" "Mozilla/5.0 (Linux) mirall/2.3.2"
127.0.0.1 - admin [23/Jun/2017:16:30:10 +0200] "PUT /owncloud/remote.php/dav/files/admin/small.dat-chunking-3428624913-13-1 HTTP/1.1" 412 297 "-" "Mozilla/5.0 (Linux) mirall/2.3.2"

@guruz

@guruz guruz changed the title Client uses old chunk protocol with new endpoint with received shared files Client uses old chunking protocol with new DAV endpoint Jun 23, 2017
@guruz guruz added type:bug p2-high Escalation, on top of current planning, release blocker labels Jun 23, 2017
@guruz
Copy link
Contributor

guruz commented Jun 23, 2017

@ogoffart @ckamm Could you review the above commit?
@ogoffart could you run tx.pl on it?

@guruz
Copy link
Contributor

guruz commented Jun 23, 2017

@mrow4a Could you add a smashbox test for overwriting/uploading a small remote file with a big one?

@PVince81
Copy link
Contributor Author

I've tested #5855 locally and it works correctly:

  1. Overwrite small file with big: uses "uploads" endpoint
  2. Overwrite big file with big: uses "uploads"
  3. Overwrite big file with small: uses simple PUT

No errors, looks good.

@ogoffart
Copy link
Contributor

I think that commit is pentotially bery dangerous.

I believe the issue should be fixed with: #5852

Altough it may still be a problem if the file changes on disk between discovery and propagation or such things. Maybe then we need to make sure that the legacy job does not do chunking at all.

@ogoffart
Copy link
Contributor

But that's right that we should not overwrite the size from an INSTRUCTION_NONE, that may be the bug.

@mrow4a
Copy link
Contributor

mrow4a commented Jun 27, 2017

@guruz @PVince81 I will start with writing a smashbox test before fixing this.

@mrow4a mrow4a self-assigned this Jun 27, 2017
@mrow4a
Copy link
Contributor

mrow4a commented Jun 27, 2017

Ok, this became interesting on current OC10 and current client master.. @guruz @ogoffart
selection_046

Both ways it become strange. I replaced big file of 50MB with small file of 12KB. It started chunking 12kB ;dd. When I had small and replaced with big, boom.

@mrow4a
Copy link
Contributor

mrow4a commented Jun 27, 2017

@ogoffart Issue not fixed with #5852

ckamm added a commit that referenced this issue Jun 28, 2017
Checks instruction, direction, size, modtime for three common cases.
@ckamm
Copy link
Contributor

ckamm commented Jun 28, 2017

@mrow4a @guruz I made a unittest to guard against similar problems in the future. #5868

@guruz guruz changed the title Client uses old chunking protocol with new DAV endpoint Client uses old chunking protocol with new DAV endpoint "An If-Match header was specified and the resource did not exist" Jun 28, 2017
@guruz guruz changed the title Client uses old chunking protocol with new DAV endpoint "An If-Match header was specified and the resource did not exist" Client uses old chunking protocol with new DAV endpoint "Precondition failed" "An If-Match header was specified and the resource did not exist" Jun 28, 2017
@mrow4a
Copy link
Contributor

mrow4a commented Jun 28, 2017

owncloud/smashbox#163 integration for server/client to protect that, will also add zero sized

guruz pushed a commit that referenced this issue Jul 3, 2017
Checks instruction, direction, size, modtime for three common cases.
@guruz
Copy link
Contributor

guruz commented Jul 11, 2017

@mrow4a has confirmed in the PR that this is fixed now :)

@Ricardosgeral
Copy link

I'm having sometimes the error syncing nextcloud
"An If-Match header was specified and the resource did not exist"
anyone knows what is happening?

@mrow4a
Copy link
Contributor

mrow4a commented Oct 3, 2017

I think you need to contact NextCloud for that, this might be related to the server also. I am not sure if they follow our protocol. We developed new chunking algorithm, you can read about it here #4019

@Ricardosgeral
Copy link

yep. sorry didn't notice this is for the owncloud and not the nextcloud

@mrow4a
Copy link
Contributor

mrow4a commented Oct 3, 2017

As far as I remember, they are using ownCloud's sync client, which is just branded. So you are in proper place. What is your sync client version? Maybe you just need to update your client. Otherwise, you need to post an issue in the Nextcloud server repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-high Escalation, on top of current planning, release blocker type:bug
Projects
None yet
Development

No branches or pull requests

6 participants