Skip to content

handle cases where SharedStorage::init isn't initializing the storage #33985

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.

Sign up for GitHub

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

Closed
icewind1991 wants to merge 3 commits into from
Closed

handle cases where SharedStorage::init isn't initializing the storage #33985

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
21f09ad
handle cases where SharedStorage::init isn't initializing the storage
icewind1991 Sep 8, 2022
0f617e7
also set nonMaskedStorage when shared setup fails
icewind1991 Sep 16, 2022
d20b53b
switch to new logger interface
icewind1991 Sep 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions apps/files_sharing/lib/SharedStorage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,17 @@
use OCP\IUserManager;
use OCP\Lock\ILockingProvider;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;

/**
* Convert target path to source path and pass the function call to the correct storage provider
*/
class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedStorage, IDisableEncryptionStorage {
/**
* @psalm-suppress NonInvariantDocblockPropertyType
* @var \OC\Files\Storage\Storage|null $storage
*/
protected $storage;

/** @var \OCP\Share\IShare */
private $superShare;
Expand All @@ -82,10 +88,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto
/** @var string */
private $user;

/**
* @var \OCP\ILogger
*/
private $logger;
private LoggerInterface $logger;

/** @var IStorage */
private $nonMaskedStorage;
Expand All @@ -102,7 +105,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto

public function __construct($arguments) {
$this->ownerView = $arguments['ownerView'];
$this->logger = \OC::$server->getLogger();
$this->logger = \OC::$server->get(LoggerInterface::class);

$this->superShare = $arguments['superShare'];
$this->groupedShares = $arguments['groupedShares'];
Expand Down Expand Up @@ -164,18 +167,21 @@ private function init() {
} catch (NotFoundException $e) {
// original file not accessible or deleted, set FailedStorage
$this->storage = new FailedStorage(['exception' => $e]);
$this->nonMaskedStorage = $this->storage;
$this->cache = new FailedCache();
$this->rootPath = '';
} catch (NoUserException $e) {
// sharer user deleted, set FailedStorage
$this->storage = new FailedStorage(['exception' => $e]);
$this->nonMaskedStorage = $this->storage;
$this->cache = new FailedCache();
$this->rootPath = '';
} catch (\Exception $e) {
$this->storage = new FailedStorage(['exception' => $e]);
$this->nonMaskedStorage = $this->storage;
$this->cache = new FailedCache();
$this->rootPath = '';
$this->logger->logException($e);
$this->logger->error($e->getMessage(), ['exception' => $e]);
}

if (!$this->nonMaskedStorage) {
Expand All @@ -200,7 +206,17 @@ public function instanceOfStorage($class): bool {
])) {
return false;
}
return parent::instanceOfStorage($class);
if (ltrim($class, '\\') === 'OC\Files\Storage\Shared') {
// FIXME Temporary fix to keep existing checks working
return true;
}
if (is_a($this, $class)) {
return true;
}

$storage = $this->getWrapperStorage();

return $storage->instanceOfStorage($class);
}

/**
Expand Down Expand Up @@ -532,6 +548,17 @@ public function getSourceStorage() {

public function getWrapperStorage() {
$this->init();

// `init` should handle this, but apparently it sometimes doesn't
Copy link
Member

@Pytal Pytal Sep 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the reason for why it doesn't known?

if (!$this->storage instanceof IStorage) {
$e = new \Exception('Share source storage is null after initializing for share: ' . $this->getShare()->getId());
$this->storage = new FailedStorage(['exception' => $e]);
$this->nonMaskedStorage = $this->storage;
$this->cache = new FailedCache();
$this->rootPath = '';
$this->logger->error($e->getMessage(), ['exception' => $e]);
}

return $this->storage;
}

Expand Down