"Server does not support X-OC-MTime" warning, wrongly sending OC-ETag

[PVince81]

Steps to reproduce

1. Create a big file "data.dat" around 120 MB big
2. Setup ownCloud 8 master (breaking commit is 96a9319)
3. Setup the sync client from git master (1.8.x commit owncloud/client@94e61c3)
4. Put "data.dat" in the local folder and wait for the sync client to sync
5. touch data.dat to make it overwrite the remote file

Additional hint: add the debug line:
qDebug() << "OLIVIER" << finished << job->reply()->rawHeader("ETag") << job->reply()->rawHeader("OC-ETag");

here: https://github.com/owncloud/client/blob/94e61c32056c470a68c773163d714b0003fddbe6/src/libsync/propagateupload.cpp#L486

Expected result

No error in sync log.

02-05 20:28:08:186 0xd26050 propagateupload.cpp:487 OLIVIER false "" ""
(...)
02-05 20:28:08:269 0xd26050 propagateupload.cpp:487 OLIVIER false "" ""
02-05 20:28:08:304 0xd26050 propagateupload.cpp:487 OLIVIER false "" ""
02-05 20:28:08:878 0xd26050 propagateupload.cpp:487 OLIVIER true "54d3c448cab1b" ""54d3c448cab1b""

(the second field being the OC-Etag)

Actual result

Sync log contains message: propagateupload.cpp:565 Server do not support X-OC-MTime ""
Also the log doesn't contain any debug entries with "false", but only:

02-05 20:35:29:815 0x13a9050 propagateupload.cpp:487 OLIVIER true "" ""54d3c5bd70757""

The Etag field is missing, only OC-Etag is returned.

This is a regression introduced by #10725

@guruz as discussed on IRC. What is the consequence of this regression ? Is it related to parallel uploads ?

@DeepDiver1975 @icewind1991 @guruz

[PVince81]

Setting to 8.0 to evaluate severity.

CC @karlitschek

[guruz]

The Etag field is missing, only OC-Etag is returned.

The other way... The fields need to be both empty because the upload is not actually done (but the server claims it is)

[PVince81]

I see, the access log shows the first three chunks:

127.0.0.1 - root [05/Feb/2015:20:39:42 +0100] "PUT /owncloud/remote.php/webdav/sub/data2.dat-chunking-4122497682-25-1 HTTP/1.1" 201 - "-" "Mozilla/5.0 (Linux) mirall/1.8.0"
127.0.0.1 - root [05/Feb/2015:20:39:42 +0100] "PUT /owncloud/remote.php/webdav/sub/data2.dat-chunking-4122497682-25-0 HTTP/1.1" 400 281 "-" "Mozilla/5.0 (Linux) mirall/1.8.0"
127.0.0.1 - root [05/Feb/2015:20:39:42 +0100] "PUT /owncloud/remote.php/webdav/sub/data2.dat-chunking-4122497682-25-2 HTTP/1.1" 400 281 "-" "Mozilla/5.0 (Linux) mirall/1.8.0"

The first one is supposedly the one that returns the etag prematurely (because it's an existing file it already has one), and that makes subsequent calls fail.

[PVince81]

@karlitschek @DeepDiver1975 I suggest to revert at this stage and find a proper fix in 8.0.1.

[PVince81]

I'll submit a revert PR and we can discuss this there.

[PVince81]

Revert PR is here: #13932

[PVince81]

CC @jnfrmarks to add a test case for this if there isn't one already.

[jnfrmarks]

@PVince81

I can try to get this test case written by EOD tomorrow. I think I have enough of the bits in place

[DeepDiver1975]

You guys are truly kidding me late night - right?

W(hy)TF am I implementing a feature for the client - which is tested and okay-ed HALF a years ago and ONE fuxxing day before release you come up with a regression?

There is seriously something wrong in here ...

@dragotin we need to get this sorted out ... cooperation of server an client team has to evolve ... we need proper automated testing of the sync protocol.

THX and Good night!

[karlitschek]

The good news is that it was found today and not after the release.
But seriously. We really need to build our full end to end testing suite.

[DeepDiver1975]

Issue description looks unrelated to the pr.
mtime != etag

this issue claims to be related to server 8 and client 1.8. What about older clients? Same issue?

thx

[DeepDiver1975]

So you are basically expecting the oc-etag only on the last chunk response?

@guruz

[dragotin]

@DeepDiver1975 yes, only on the last chunk. Its the indicator for the client that all chunks have been received. Sending the ETag header earlier makes the client think that the file upload was completed, ie. because there have been chunks from before.

[guruz]

So you are basically expecting the oc-etag only on the last chunk response?

The OC-Etag must be equivalent to the ETag. The reason the OC-Etag was introduced is because some server modules (mod_gzip) or proxies change/remove the ETag.

The sync client uses ETag/OC-ETag to get the info "you now had uploaded the last chunk, file is complete". So yes only on last chunk (not last numerically but last chunk that completes the file)

[DeepDiver1975]

I ll come up with a fix.

[DeepDiver1975]

@dragotin can I ask you to please add a test on this in our sync test t*.pl? We should catch issues like this in the future - THX

[DeepDiver1975]

@guruz the new header is only available for oc8 - I assume you can continue to live not having the header for now - I'm all in now to simply revert the orifingal pr

[PVince81]

Here we go: #13932

[dragotin]

A test was added to t6.pl: owncloud/client@2a88f50

[DeepDiver1975]

thx @dragotin

[MorrisJobke]

The PR is merged: Close?

[PVince81]

Closing. I reopened the original issue: #9005