Failing smashbox test on master with encryption + sharing

[PVince81]

Master cd16ba5
With encryption enabled.

2016-02-04 12:59:10,940 - ERROR - sharer - md5 of modified file /home/vincent/work/smashdir/test_shareFile/sharer/TEST_FILE_MODIFIED_USER_SHARE.dat did not change: expected 45b565af28a2998fa8d5dae7eb6e660e "md5 of modified file %s did not change%s: expected %s" % (fn, comment, md5)) failed in expect_modified() ["/home/vincent/work/workspace/smashbox/python/smashbox/utilities/__init__.py" at line 781]

CC @nickvergessen @rullzer @schiesbn

Will bisect it.

[PVince81]

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
2dcdc81
3ab0ccd
5d0451b
cdc8c40
6d321f5

[PVince81]

I have reverified that the commit 8f89e35 before this set works fine and the commit 5fae07f after that is broken.

@icewind1991 looks like the new cache interface broke encryption + sharing somehow.
Maybe you missed a location when adjusting the cache stuff ?
Or something is wrong with etag propagation.

The failing test is bin/smash -a lib/oc-tests/test_shareFile.py

[nickvergessen]

the first three commits are impossible from reading them.

[icewind1991]

6d321f5 seems like the most likely but I dont know why

[PVince81]

If it's only with encryption, I wonder if it's another case of "we need to forward the call to the proper storage/wrapper" like #18010

[PVince81]

Steps to reproduce manually:

1. Enable encryption
2. Create users "user1", "user2", "user3"
3. Login as "user1"
4. Create a test file "test.txt"
5. Share "test.txt" with "user2" and "user3"
6. Using a Webdav client (I used cadaver), login as "user2"
7. Try overwriting "test.txt" with a PUT

=> getting a 500 Internal Server Error

{"reqId":"mpSSffpyTs5HLMGMSHLt","remoteAddr":"127.0.0.1","app":"PHP","message":"Call to a member function getData() on array at \/srv\/www\/htdocs\/owncloud\/lib\/private\/files\/storage\/wrapper\/encryption.php#134","level":3,"time":"2016-02-08T19:31:10+00:00","method":"PUT","url":"\/owncloud\/remote.php\/webdav\/test.txt"}

Seems this was somehow buried in the big log.

[PVince81]

This is the line where it fails:

core/lib/private/files/storage/wrapper/encryption.php

Line 134 in 698a6b0

$this->getCache()->put($path, $info->getData());

The $info object only has two properties at this stage:

$info                            = (array[2]);
  $info['encrypted']             = (bool) '1';
  $info['size']                  = (int) '2348';

maybe it's an incomplete cache entry. I could compare it with what it was supposed to be before the regression.

@icewind1991 I remember you replaced all arrays with ICacheEntry or something, you probably missed a few ?

[PVince81]

actual full stack when the error occurs:

0  OC\Files\Storage\Wrapper\Encryption->filesize() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/encryption.php:134
1  OC\Files\Storage\Wrapper\Encryption->fopen() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/encryption.php:360
2  OC\Files\Storage\Wrapper\Wrapper->fopen() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/wrapper.php:287
3  OC\Files\Storage\Wrapper\Encryption->copyBetweenStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/encryption.php:531
4  OC\Files\Storage\Wrapper\Encryption->moveFromStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/encryption.php:446
5  OC\Files\Storage\Wrapper\Wrapper->moveFromStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/wrapper.php:558
6  OC\Files\Storage\Shared->moveFromStorage() /srv/www/htdocs/owncloud/apps/files_sharing/lib/sharedstorage.php:657
7  OC\Files\Storage\Wrapper\Wrapper->moveFromStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/wrapper.php:558
8  OC\Files\Storage\Wrapper\Availability->moveFromStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/availability.php:437
9  OC\Files\Storage\Wrapper\Wrapper->moveFromStorage() /srv/www/htdocs/owncloud/lib/private/files/storage/wrapper/wrapper.php:558
10 OCA\DAV\Connector\Sabre\File->put() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/file.php:182
11 Sabre\DAV\Server->updateFile() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:1070
12 Sabre\DAV\CorePlugin->httpPut() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/CorePlugin.php:513
13 call_user_func_array:{/srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105}() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
14 Sabre\Event\EventEmitter->emit() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
15 Sabre\DAV\Server->invokeMethod() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:459
16 Sabre\DAV\Server->exec() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:248
17 require_once()  /srv/www/htdocs/owncloud/apps/dav/appinfo/v1/webdav.php:54
18 {main}          /srv/www/htdocs/owncloud/remote.php:137

[PVince81]

Even weirder, actually the get call from above returns "false".
But then PHP is so clever that calling $info[] transforms it in an array...

The path in question is the path of the part file, which obviously has no cache entry.
IIRC the encryption code was using a partial cache entry for storing the file size temporarily or so.

Let's compare with v8.2.2

[PVince81]

i checked the v8.2.2 and indeed it's expected that $info is false in that situation and PHP magically transforms it into an array. I'll see if I can patch it up to work as it did before.