External storage encryption delete error by dassio · Pull Request #22598 · nextcloud/server

dassio · 2020-09-04T16:40:48Z

fix issue caused by d3b6dbc

Error Message:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Error</s:exception>
  <s:message>Call to a member function getId() on array</s:message>
</d:error>

Analysis

File Upload

the metadata encrypted is not saved

[0] OC\Files\Cache\Cache->insert @ /var/www/html/lib/private/Files/Cache/Cache.php:262
[1] OC\Files\Cache\Wrapper\CachePermissionsMask->insert @ /var/www/html/lib/private/Files/Cache/Wrapper/CacheWrapper.php:131
[2] OC\Files\Cache\Scanner->addToCache @ /var/www/html/lib/private/Files/Cache/Scanner.php:294
[3] OC\Files\Cache\Scanner->scanFile @ /var/www/html/lib/private/Files/Cache/Scanner.php:224
[4] OC\Files\Cache\Scanner->scan @ /var/www/html/lib/private/Files/Cache/Scanner.php:338
[5] OC\Files\Cache\Updater->update @ /var/www/html/lib/private/Files/Cache/Updater.php:126
[6] OCA\DAV\Connector\Sabre\File->put @ /var/www/html/apps/dav/lib/Connector/Sabre/File.php:299
[7] OCA\DAV\Connector\Sabre\Directory->createFile @ /var/www/html/apps/dav/lib/Connector/Sabre/Directory.php:155
[8] OCA\DAV\Connector\Sabre\Server->createFile @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:1104
[9] Sabre\DAV\CorePlugin->httpPut @ /var/www/html/3rdparty/sabre/dav/lib/DAV/CorePlugin.php:527
[10] OCA\DAV\Connector\Sabre\Server->emit @ /var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php:89
[11] OCA\DAV\Connector\Sabre\Server->invokeMethod @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:474
[12] OCA\DAV\Connector\Sabre\Server->start @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:251
[13] OCA\DAV\Connector\Sabre\Server->exec @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:319
[14] require_once @ /var/www/html/apps/dav/appinfo/v1/webdav.php:84
[15] {main} @ /var/www/html/remote.php:167

 _ $builder = (uninitialized)
 _ $column = (uninitialized)
 _ $data = (array [10])
   _ $data["mimetype"] = (string [10]) `image/jpeg`
   _ $data["mtime"] = (int) 1599234758
   _ $data["size"] = (int) 267298
   _ $data["etag"] = (string [13]) `5f5262c877be5`
   _ $data["storage_mtime"] = (int) 1599234758
   _ $data["permissions"] = (int) 27
   _ $data["name"] = (string [12]) `IMG_2080.JPG`
   _ $data["scan_permissions"] = (int) 27
   _ $data["parent"] = (int) 12674
   _ $data["checksum"] = (string [0]) ``
 _ $e = (uninitialized)
 _ $extensionValues = (uninitialized)
 _ $field = (uninitialized)
 _ $file = (string [21]) `app-test/IMG_2080.JPG`
 _ $fileId = (uninitialized)
 _ $id = (uninitialized)
 _ $query = (uninitialized)
 _ $requiredFields = (uninitialized)
 _ $value = (uninitialized)
 _ $values = (uninitialized)
 _ $this = (OC\Files\Cache\Cache [8])
   _ $this->partial = (array)
   _ $this->storageId = (string [15]) `amazon::app-dev`
   _ $this->storage = (OCA\Files_Trashbin\Storage [12])
   _ $this->storageCache = (OC\Files\Cache\Storage [3])
   _ $this->mimetypeLoader = (OC\Files\Type\Loader [3])
   _ $this->connection = (OC\DB\Connection [19])
   _ $this->eventDispatcher = (OC\EventDispatcher\SymfonyAdapter [2])
   _ $this->querySearchHelper = (OC\Files\Cache\QuerySearchHelper [3])

File Delete

when deleting file, 'encrypted' is added, so it pass condition check if ($entry) {, result in the error

[0] OC\Files\Cache\Cache->insert @ /var/www/html/lib/private/Files/Cache/Cache.php:262
[1] OC\Files\Cache\Wrapper\CachePermissionsMask->insert @ /var/www/html/lib/private/Files/Cache/Wrapper/CacheWrapper.php:131
[2] OC\Files\Cache\Wrapper\CachePermissionsMask->put @ /var/www/html/lib/private/Files/Cache/Wrapper/CacheWrapper.php:117
[3] OC\Files\Storage\Wrapper\Encryption->updateEncryptedVersion @ /var/www/html/lib/private/Files/Storage/Wrapper/Encryption.php:688
[4] OC\Files\Storage\Wrapper\Encryption->copyBetweenStorage @ /var/www/html/lib/private/Files/Storage/Wrapper/Encryption.php:769
[5] OC\Files\Storage\Wrapper\Encryption->moveFromStorage @ /var/www/html/lib/private/Files/Storage/Wrapper/Encryption.php:625
[6] OCA\Files_Trashbin\Storage->moveFromStorage @ /var/www/html/lib/private/Files/Storage/Wrapper/Wrapper.php:575
[7] OCA\Files_Trashbin\Trashbin::move2trash @ /var/www/html/apps/files_trashbin/lib/Trashbin.php:296
[8] OCA\Files_Trashbin\Trash\LegacyTrashBackend->moveToTrash @ /var/www/html/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php:108
[9] OCA\Files_Trashbin\Trash\TrashManager->moveToTrash @ /var/www/html/apps/files_trashbin/lib/Trash/TrashManager.php:103
[10] OCA\Files_Trashbin\Storage->doDelete @ /var/www/html/apps/files_trashbin/lib/Storage.php:192
[11] OCA\Files_Trashbin\Storage->unlink @ /var/www/html/apps/files_trashbin/lib/Storage.php:99
[12] OC\Files\View->basicOperation @ /var/www/html/lib/private/Files/View.php:1161
[13] OC\Files\View->unlink @ /var/www/html/lib/private/Files/View.php:718
[14] OCA\DAV\Connector\Sabre\File->delete @ /var/www/html/apps/dav/lib/Connector/Sabre/File.php:455
[15] OCA\DAV\Connector\Sabre\CachingTree->delete @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Tree.php:183
[16] Sabre\DAV\CorePlugin->httpDelete @ /var/www/html/3rdparty/sabre/dav/lib/DAV/CorePlugin.php:295
[17] OCA\DAV\Connector\Sabre\Server->emit @ /var/www/html/3rdparty/sabre/event/lib/WildcardEmitterTrait.php:89
[18] OCA\DAV\Connector\Sabre\Server->invokeMethod @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:474
[19] OCA\DAV\Connector\Sabre\Server->start @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:251
[20] OCA\DAV\Connector\Sabre\Server->exec @ /var/www/html/3rdparty/sabre/dav/lib/DAV/Server.php:319
[21] OCA\DAV\Server->exec @ /var/www/html/apps/dav/lib/Server.php:325
[22] require_once @ /var/www/html/apps/dav/appinfo/v2/remote.php:35
[23] {main} @ /var/www/html/remote.php:167

 _ $builder = (uninitialized)
 _ $column = (uninitialized)
 _ $data = (array [1])
   _ $data["encrypted"] = (bool) 0
 _ $e = (uninitialized)
 _ $extensionValues = (uninitialized)
 _ $field = (uninitialized)
 _ $file = (string [21]) `app-test/IMG_2192.PNG`
 _ $fileId = (uninitialized)
 _ $id = (uninitialized)
 _ $query = (uninitialized)
 _ $requiredFields = (uninitialized)
 _ $value = (uninitialized)
 _ $values = (uninitialized)
 _ $this = (OC\Files\Cache\Cache [8])
   _ $this->partial = (array)
   _ $this->storageId = (string [15]) `amazon::app-dev`
   _ $this->storage = (OCA\Files_Trashbin\Storage [12])
   _ $this->storageCache = (OC\Files\Cache\Storage [3])
   _ $this->mimetypeLoader = (OC\Files\Type\Loader [3])
   _ $this->connection = (OC\DB\Connection [19])
   _ $this->eventDispatcher = (OC\EventDispatcher\SymfonyAdapter [2])
   _ $this->querySearchHelper = (OC\Files\Cache\QuerySearchHelper [3])

kesselb

Nice catch!

getId is only available if $entry is a instance of CacheEntry. I would fix it like if ($entry instanceof CacheEntry) { but a storage expert should have a look because get should return a CacheEntry or false. An array with one or more keys is not documented.

kesselb · 2020-09-04T20:08:18Z

lib/private/Files/Cache/Cache.php

 		$entry = $this->get($file);
-
+		// when deleting the entry, encrypted is not save in database, so we ignore it
+		unset($entry["encrypted"]);


Suggested change

unset($entry["encrypted"]);

unset($entry['encrypted']);

Signed-off-by: xiangbin.li <dassio@icloud.com>

juliusknorr · 2020-09-10T08:55:52Z

getId is only available if $entry is a instance of CacheEntry. I would fix it like if ($entry instanceof CacheEntry) { but a storage expert should have a look because get should return a CacheEntry or false. An array with one or more keys is not documented.

There should indeed not be an array returned. Would be interesting to know where that is coming from to fix that actually.

While the unset might still work if false is returned I'd prefer to figure out the original source of the array and see why there is no CacheEntry returned.

dassio · 2020-09-10T10:51:51Z

There should indeed not be an array returned. Would be interesting to know where that is coming from to fix that actually.

you can check the second track trace to see where this encrypted is added to CacheEntry instance.

it is added by the encryption wrapper, so if encrtiption is not eabled, we will not have this issue

dassio

154 >->-$data = $query->execute()->fetch();                                                                     
 155 
 156 >->-//merge partial data                                                                                   
 157 >->-if (!$data and is_string($file) and isset($this->partial[$file])) {                                                                                      
 158 >->->-return $this->partial[$file];                                                                                                                 
 159 >->-} elseif (!$data) {
 160 >->->-return $data;
 161 >->-} else {
 162 >->->-return self::cacheEntryFromData($data, $this->mimetypeLoader);                                                                      
 163 >->-}
 164 >-}

even though the query did not return anything, we still have the encrypted data from the encryption wrapper

skjnldsv · 2022-01-17T08:10:05Z

/rebase

mejo- · 2022-06-29T12:13:56Z

Shall we close this one, now that #33050 got merged? @kesselb

come-nc · 2022-09-15T14:56:12Z

Shall we close this one, now that #33050 got merged? @kesselb

Yes, sounds sane.

dassio changed the title ~~External storage encription delete error~~ External storage encryption delete error Sep 4, 2020

kesselb reviewed Sep 4, 2020

View reviewed changes

kesselb requested review from icewind1991, juliusknorr and rullzer September 4, 2020 20:18

kesselb added 3. to review Waiting for reviews bug labels Sep 4, 2020

ignore encrypted metadata in cache

97a3883

Signed-off-by: xiangbin.li <dassio@icloud.com>

dassio commented Sep 10, 2020

View reviewed changes

skjnldsv requested review from a team, CarlSchwan and kesselb and removed request for a team January 17, 2022 08:10

skjnldsv added this to the Nextcloud 24 milestone Jan 17, 2022

skjnldsv mentioned this pull request Mar 24, 2022

24.0.0 beta 1 #31693

Merged

blizzz mentioned this pull request Mar 31, 2022

24.0.0 beta 2 #31769

Merged

This was referenced Apr 7, 2022

24.0.0 beta 3 #31871

Merged

24.0.0 RC1 #31962

Merged

blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022

kesselb mentioned this pull request Jun 26, 2022

[Bug]: Files/Cache/Cache.php->get() returns partial array, leads to errors #33023

Closed

8 tasks

mejo- mentioned this pull request Jun 28, 2022

Check whether entry is of type ICacheEntry in Cache->remove() #33050

Merged

This was referenced Aug 12, 2022

25.0.0 beta 1 #33505

Merged

25.0.0 beta 2 #33592

Merged

This was referenced Aug 24, 2022

25.0.0 beta 3 #33674

Merged

25.0.0 beta 4 #33762

Merged

This was referenced Sep 6, 2022

25.0.0 beta 5 #33913

Merged

25.0.0 beta 6 #34002

Merged

skjnldsv mentioned this pull request Sep 15, 2022

25.0.0 beta 7 #34095

Merged

come-nc closed this Sep 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

External storage encryption delete error #22598

External storage encryption delete error #22598
dassio wants to merge 1 commit intonextcloud:masterfrom
dassio:delete_ignore_encription_parameter

dassio commented Sep 4, 2020 •

edited by kesselb

Loading

Uh oh!

kesselb left a comment

Uh oh!

kesselb Sep 4, 2020

Uh oh!

juliusknorr commented Sep 10, 2020 •

edited

Loading

Uh oh!

dassio commented Sep 10, 2020 •

edited

Loading

Uh oh!

dassio left a comment •

edited

Loading

Uh oh!

skjnldsv commented Jan 17, 2022

Uh oh!

mejo- commented Jun 29, 2022

Uh oh!

come-nc commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

dassio commented Sep 4, 2020 • edited by kesselb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Error Message:

Analysis

File Upload

File Delete

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

kesselb Sep 4, 2020

Choose a reason for hiding this comment

Uh oh!

juliusknorr commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dassio commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dassio left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Jan 17, 2022

Uh oh!

mejo- commented Jun 29, 2022

Uh oh!

come-nc commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dassio commented Sep 4, 2020 •

edited by kesselb

Loading

juliusknorr commented Sep 10, 2020 •

edited

Loading

dassio commented Sep 10, 2020 •

edited

Loading

dassio left a comment •

edited

Loading