Bogus versions behavior with federated shares

[PVince81]

Steps

1. Setup two servers user0@OC_A and user1@OC_B
2. Set a quota for "user1" to "20 B"
3. Login as user0@OC_A
4. Create a folder "test"
5. Share "test" with user1@OC_B
6. Login as user1@OC_B
7. Accept the share
8. Set a breakpoint in OCA\Files_Versions\::store()
9. Upload a file into "test/test.txt"
10. Check the logs

Expected result

Versions code not called.

Actual result

Versions code is called, and it tries to copy a version for "user1" from "test/test.txt". It fails, no file is created.
When a quota is set for user1, it will log a warning:

{"reqId":"1epAaHOvFhKhzXccgDQQ","level":3,"time":"2017-10-13T13:46:03+00:00","remoteAddr":"127.0.0.1","user":"user1","app":"PHP","method":"PUT","url":"\/owncloud\/remote.php\/webdav\/abc\/bacon.txt","message":"Undefined index: size at \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/View.php#1296"}

Without quota it will not log the warning, but still attempt to create a version.

This doesn't seem to cause any harm.

Versions

ownCloud 10.0.3 for both instances

[PVince81]

The first question would be, why is the version code triggered at all ?

0  OCA\Files_Versions\Storage::store() /srv/www/htdocs/owncloud/apps/files_versions/lib/Storage.php:193
1  OCA\Files_Versions\Hooks::write_hook() /srv/www/htdocs/owncloud/apps/files_versions/lib/Hooks.php:60
2  OC_Hook::emit() /srv/www/htdocs/owncloud/lib/private/legacy/hook.php:105
3  OCA\DAV\Connector\Sabre\File->emitPreHooks() /srv/www/htdocs/owncloud/apps/dav/lib/Connector/Sabre/File.php:283
4  OCA\DAV\Connector\Sabre\File->put() /srv/www/htdocs/owncloud/apps/dav/lib/Connector/Sabre/File.php:189
5  OCA\DAV\Connector\Sabre\Directory->createFile() /srv/www/htdocs/owncloud/apps/dav/lib/Connector/Sabre/Directory.php:162
6  OCA\DAV\Connector\Sabre\Server->createFile() /srv/www/htdocs/owncloud/lib/composer/sabre/dav/lib/DAV/Server.php:1095
7  Sabre\DAV\CorePlugin->httpPut() /srv/www/htdocs/owncloud/lib/composer/sabre/dav/lib/DAV/CorePlugin.php:525
8  call_user_func_array:{/srv/www/htdocs/owncloud/lib/composer/sabre/event/lib/EventEmitterTrait.php:105}() /srv/www/htdocs/owncloud/lib/composer/sabre/event/lib/EventEmitterTrait.php:105
9  OCA\DAV\Connector\Sabre\Server->emit() /srv/www/htdocs/owncloud/lib/composer/sabre/event/lib/EventEmitterTrait.php:105
10 OCA\DAV\Connector\Sabre\Server->invokeMethod() /srv/www/htdocs/owncloud/lib/composer/sabre/dav/lib/DAV/Server.php:479
11 OCA\DAV\Connector\Sabre\Server->exec() /srv/www/htdocs/owncloud/lib/composer/sabre/dav/lib/DAV/Server.php:254
12 require_once()  /srv/www/htdocs/owncloud/apps/dav/appinfo/v1/webdav.php:63
13 {main}          /srv/www/htdocs/owncloud/remote.php:175

The code in store() checks whether the file "test/test.txt" exists and sees that the file exists, because somehow it was already created on the remote federated share.

The reason why this is happening at this stage needs to be looked into. Could be due to the fact that the file was written but is supposed to be a part file, so triggering the hooks would operate on the previous version of the file, if exists. But since we don't use part files for federated shares, the file is directly written there, so it's directly available and mistakenly be interpreted as being an older file to be versioned.

When a small quota is set, the version copy operation will fail and display the warning: Undefined index: size at \/srv\/www\/htdocs\/owncloud\/lib\/private\/Files\/View.php#1296"} because there is not enough quota for the version itself.

When no quota is set or enough space is available, the code will indeed create a duplicate version.

I'm not sure whether we should look into ways for fixing this. The best approach is likely to make the storage versions-aware. So OC_B's versions handler would not operate at all and simply rely on reading OC_A's versions. #12274

[phil-davis]

  Scenario: Overwrite a federated shared file as recipient                                              # /var/lib/jenkins/workspace/owncloud-core_core_PR-29230-KCWDEFX4HFTQB5I6U3B2FOQKN75PWS2MJF3QCI2GWLNB4R2EJP5Q/tests/integration/federation_features/federated.feature:123

passes but produces these messages in the output:

[Thu Oct 12 23:23:32 2017] copy(//var/lib/jenkins/workspace/owncloud-core_core_PR-29230-KCWDEFX4HFTQB5I6U3B2FOQKN75PWS2MJF3QCI2GWLNB4R2EJP5Q/data/3d517fe6ebab7b8cfcf98db6259c8a59/files_versions/textfile0.txt.v1507843412): failed to open stream: No such file or directory at /var/lib/jenkins/workspace/owncloud-core_core_PR-29230-KCWDEFX4HFTQB5I6U3B2FOQKN75PWS2MJF3QCI2GWLNB4R2EJP5Q/lib/private/Files/Storage/Local.php#276
[Thu Oct 12 23:23:32 2017] Undefined index: size at /var/lib/jenkins/workspace/owncloud-core_core_PR-29230-KCWDEFX4HFTQB5I6U3B2FOQKN75PWS2MJF3QCI2GWLNB4R2EJP5Q/lib/private/Files/View.php#1296

I noticed this the other day while I happened to be running some tests locally. In this particular test, it seems that the file owner does not yet have a files_versions folder (I guess because they have not made a file change themselves). And so when the remote user uploads a changed file, files_versions kicks in to save the previous contents, but somehow in that context is not able to be the first to create the files_versions folder. PHP copy() fails if any of the target folders does not exist.

Is this a related thing to the undefined index coming straight after? Does it give any clue about this problem?

[PVince81]

@phil-davis I already know what the problem is with this warning, see #29246

[phil-davis]

I just wanted to add the "no such file or directory" message so you are aware of that happening also.

[ownclouders]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

[PVince81]

We need to rework the version app anyway. I don't see a quick solution for this apart from hiding the warning message...

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.