Skip to content
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.

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

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

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
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
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;
Fixed Show fixed Hide fixed

/** @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);
Fixed Show fixed Hide fixed
}

/**
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

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