fix(jobs): Limit command jobs to known cases

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/55748/head
Joas Schilling 2025-10-14 13:56:42 +07:00
parent df8d838186
commit 34f1ae62a1
No known key found for this signature in database
GPG Key ID: F72FA5B49FFA96B0
12 changed files with 75 additions and 214 deletions

@ -1795,6 +1795,9 @@
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
</DeprecatedClass>
<DeprecatedInterface>
<code><![CDATA[Expire]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
@ -1826,6 +1829,9 @@
</DeprecatedMethod>
</file>
<file src="apps/files_trashbin/lib/Command/Size.php">
<DeprecatedInterface>
<code><![CDATA[private]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[setAppValue]]></code>
@ -2046,6 +2052,11 @@
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
</DeprecatedMethod>
</file>
<file src="apps/files_versions/lib/Command/Expire.php">
<DeprecatedInterface>
<code><![CDATA[Expire]]></code>
</DeprecatedInterface>
</file>
<file src="apps/files_versions/lib/Command/ExpireVersions.php">
<DeprecatedClass>
<code><![CDATA[\OC_Util::setupFS($user)]]></code>
@ -2095,6 +2106,9 @@
<code><![CDATA[Files::streamCopy($source, $target, true)]]></code>
<code><![CDATA[\OC_Util::setupFS($uid)]]></code>
</DeprecatedClass>
<DeprecatedInterface>
<code><![CDATA[$bus]]></code>
</DeprecatedInterface>
<DeprecatedMethod>
<code><![CDATA[Files::streamCopy($source, $target, true)]]></code>
<code><![CDATA[dispatch]]></code>
@ -3468,15 +3482,19 @@
}, $context->getCalendarProviders())]]></code>
</NamedArgumentNotAllowed>
</file>
<file src="lib/private/Command/CallableJob.php">
<ParamNameMismatch>
<code><![CDATA[$serializedCallable]]></code>
</ParamNameMismatch>
<file src="lib/private/Command/CommandJob.php">
<UndefinedClass>
<code><![CDATA[\Test\Command\FilesystemCommand]]></code>
<code><![CDATA[\Test\Command\SimpleCommand]]></code>
<code><![CDATA[\Test\Command\StateFullCommand]]></code>
</UndefinedClass>
</file>
<file src="lib/private/Command/ClosureJob.php">
<InvalidArgument>
<code><![CDATA[[LaravelClosure::class]]]></code>
</InvalidArgument>
<file src="lib/private/Command/QueueBus.php">
<UndefinedClass>
<code><![CDATA[\Test\Command\FilesystemCommand]]></code>
<code><![CDATA[\Test\Command\SimpleCommand]]></code>
<code><![CDATA[\Test\Command\StateFullCommand]]></code>
</UndefinedClass>
</file>
<file src="lib/private/Comments/Manager.php">
<RedundantCast>

@ -1220,8 +1220,6 @@ return array(
'OC\\Collaboration\\Resources\\Resource' => $baseDir . '/lib/private/Collaboration/Resources/Resource.php',
'OC\\Color' => $baseDir . '/lib/private/Color.php',
'OC\\Command\\AsyncBus' => $baseDir . '/lib/private/Command/AsyncBus.php',
'OC\\Command\\CallableJob' => $baseDir . '/lib/private/Command/CallableJob.php',
'OC\\Command\\ClosureJob' => $baseDir . '/lib/private/Command/ClosureJob.php',
'OC\\Command\\CommandJob' => $baseDir . '/lib/private/Command/CommandJob.php',
'OC\\Command\\CronBus' => $baseDir . '/lib/private/Command/CronBus.php',
'OC\\Command\\FileAccess' => $baseDir . '/lib/private/Command/FileAccess.php',

@ -1261,8 +1261,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Collaboration\\Resources\\Resource' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Resource.php',
'OC\\Color' => __DIR__ . '/../../..' . '/lib/private/Color.php',
'OC\\Command\\AsyncBus' => __DIR__ . '/../../..' . '/lib/private/Command/AsyncBus.php',
'OC\\Command\\CallableJob' => __DIR__ . '/../../..' . '/lib/private/Command/CallableJob.php',
'OC\\Command\\ClosureJob' => __DIR__ . '/../../..' . '/lib/private/Command/ClosureJob.php',
'OC\\Command\\CommandJob' => __DIR__ . '/../../..' . '/lib/private/Command/CommandJob.php',
'OC\\Command\\CronBus' => __DIR__ . '/../../..' . '/lib/private/Command/CronBus.php',
'OC\\Command\\FileAccess' => __DIR__ . '/../../..' . '/lib/private/Command/FileAccess.php',

@ -1,5 +1,7 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@ -19,14 +21,12 @@ abstract class AsyncBus implements IBus {
*
* @var string[]
*/
private $syncTraits = [];
private array $syncTraits = [];
/**
* Schedule a command to be fired
*
* @param \OCP\Command\ICommand | callable $command
*/
public function push($command) {
public function push(ICommand $command): void {
if ($this->canRunAsync($command)) {
$this->queueCommand($command);
} else {
@ -36,39 +36,29 @@ abstract class AsyncBus implements IBus {
/**
* Queue a command in the bus
*
* @param \OCP\Command\ICommand | callable $command
*/
abstract protected function queueCommand($command);
abstract protected function queueCommand(ICommand $command);
/**
* Require all commands using a trait to be run synchronous
*
* @param string $trait
*/
public function requireSync($trait) {
public function requireSync(string $trait): void {
$this->syncTraits[] = trim($trait, '\\');
}
/**
* @param \OCP\Command\ICommand | callable $command
*/
private function runCommand($command) {
if ($command instanceof ICommand) {
$command->handle();
} else {
$command();
}
private function runCommand(ICommand $command): void {
$command->handle();
}
/**
* @param \OCP\Command\ICommand | callable $command
* @return bool
*/
private function canRunAsync($command) {
private function canRunAsync(ICommand $command): bool {
$traits = $this->getTraits($command);
foreach ($traits as $trait) {
if (in_array($trait, $this->syncTraits)) {
if (in_array($trait, $this->syncTraits, true)) {
return false;
}
}
@ -76,14 +66,9 @@ abstract class AsyncBus implements IBus {
}
/**
* @param \OCP\Command\ICommand | callable $command
* @return string[]
*/
private function getTraits($command) {
if ($command instanceof ICommand) {
return class_uses($command);
} else {
return [];
}
private function getTraits(ICommand $command): array {
return class_uses($command);
}
}

@ -1,21 +0,0 @@
<?php
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/
namespace OC\Command;
use OCP\BackgroundJob\QueuedJob;
class CallableJob extends QueuedJob {
protected function run($serializedCallable) {
$callable = unserialize($serializedCallable);
if (is_callable($callable)) {
$callable();
} else {
throw new \InvalidArgumentException('Invalid serialized callable');
}
}
}

@ -1,23 +0,0 @@
<?php
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/
namespace OC\Command;
use Laravel\SerializableClosure\SerializableClosure as LaravelClosure;
use OCP\BackgroundJob\QueuedJob;
class ClosureJob extends QueuedJob {
protected function run($argument) {
$callable = unserialize($argument, [LaravelClosure::class]);
$callable = $callable->getClosure();
if (is_callable($callable)) {
$callable();
} else {
throw new \InvalidArgumentException('Invalid serialized callable');
}
}
}

@ -15,7 +15,13 @@ use OCP\Command\ICommand;
*/
class CommandJob extends QueuedJob {
protected function run($argument) {
$command = unserialize($argument);
$command = unserialize($argument, ['allowed_classes' => [
\Test\Command\SimpleCommand::class,
\Test\Command\StateFullCommand::class,
\Test\Command\FilesystemCommand::class,
\OCA\Files_Trashbin\Command\Expire::class,
\OCA\Files_Versions\Command\Expire::class,
]]);
if ($command instanceof ICommand) {
$command->handle();
} else {

@ -1,13 +1,13 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Command;
use Laravel\SerializableClosure\SerializableClosure;
use OCP\BackgroundJob\IJob;
use OCP\BackgroundJob\IJobList;
use OCP\Command\ICommand;
@ -17,37 +17,7 @@ class CronBus extends AsyncBus {
) {
}
protected function queueCommand($command): void {
$this->jobList->add($this->getJobClass($command), $this->serializeCommand($command));
}
/**
* @param ICommand|callable $command
* @return class-string<IJob>
*/
private function getJobClass($command): string {
if ($command instanceof \Closure) {
return ClosureJob::class;
} elseif (is_callable($command)) {
return CallableJob::class;
} elseif ($command instanceof ICommand) {
return CommandJob::class;
} else {
throw new \InvalidArgumentException('Invalid command');
}
}
/**
* @param ICommand|callable $command
* @return string
*/
private function serializeCommand($command): string {
if ($command instanceof \Closure) {
return serialize(new SerializableClosure($command));
} elseif (is_callable($command) || $command instanceof ICommand) {
return serialize($command);
} else {
throw new \InvalidArgumentException('Invalid command');
}
protected function queueCommand(ICommand $command): void {
$this->jobList->add(CommandJob::class, serialize($command));
}
}

@ -1,5 +1,7 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@ -12,45 +14,40 @@ use OCP\Command\ICommand;
class QueueBus implements IBus {
/**
* @var ICommand[]|callable[]
* @var ICommand[]
*/
private $queue = [];
private array $queue = [];
/**
* Schedule a command to be fired
*
* @param \OCP\Command\ICommand | callable $command
*/
public function push($command) {
public function push(ICommand $command): void {
$this->queue[] = $command;
}
/**
* Require all commands using a trait to be run synchronous
*
* @param string $trait
*/
public function requireSync($trait) {
public function requireSync(string $trait): void {
}
/**
* @param \OCP\Command\ICommand | callable $command
*/
private function runCommand($command) {
if ($command instanceof ICommand) {
// ensure the command can be serialized
$serialized = serialize($command);
if (strlen($serialized) > 4000) {
throw new \InvalidArgumentException('Trying to push a command which serialized form can not be stored in the database (>4000 character)');
}
$unserialized = unserialize($serialized);
$unserialized->handle();
} else {
$command();
private function runCommand(ICommand $command): void {
// ensure the command can be serialized
$serialized = serialize($command);
if (strlen($serialized) > 4000) {
throw new \InvalidArgumentException('Trying to push a command which serialized form can not be stored in the database (>4000 character)');
}
$unserialized = unserialize($serialized, ['allowed_classes' => [
\Test\Command\SimpleCommand::class,
\Test\Command\StateFullCommand::class,
\Test\Command\FilesystemCommand::class,
\OCA\Files_Trashbin\Command\Expire::class,
\OCA\Files_Versions\Command\Expire::class,
]]);
$unserialized->handle();
}
public function run() {
public function run(): void {
while ($command = array_shift($this->queue)) {
$this->runCommand($command);
}

@ -10,16 +10,18 @@ namespace OCP\Command;
/**
* Interface IBus
*
* @deprecated 33.0.0 The interface is considered internal going forward and should not be implemented by apps anymore
* @since 8.1.0
*/
interface IBus {
/**
* Schedule a command to be fired
*
* @param \OCP\Command\ICommand | callable $command
* @param \OCP\Command\ICommand $command
* @since 33.0.0 Only allowed for {@see \OCA\Files_Trashbin\Command\Expire} and {@see \OCA\Files_Versions\Command\Expire}
* @since 8.1.0
*/
public function push($command);
public function push(ICommand $command): void;
/**
* Require all commands using a trait to be run synchronous
@ -27,5 +29,5 @@ interface IBus {
* @param string $trait
* @since 8.1.0
*/
public function requireSync($trait);
public function requireSync(string $trait): void;
}

@ -10,6 +10,7 @@ namespace OCP\Command;
/**
* Interface ICommand
*
* @deprecated 33.0.0 The interface is considered internal going forward and should not be implemented by apps anymore
* @since 8.1.0
*/
interface ICommand {

@ -38,23 +38,6 @@ class FilesystemCommand implements ICommand {
}
}
function basicFunction() {
AsyncBusTestCase::$lastCommand = 'function';
}
// clean class to prevent phpunit putting closure in $this
class ThisClosureTest {
private function privateMethod() {
AsyncBusTestCase::$lastCommand = 'closure-this';
}
public function test(IBus $bus) {
$bus->push(function (): void {
$this->privateMethod();
});
}
}
abstract class AsyncBusTestCase extends TestCase {
/**
* Basic way to check output from a command
@ -105,59 +88,6 @@ abstract class AsyncBusTestCase extends TestCase {
$this->assertEquals('foo', self::$lastCommand);
}
public function testStaticCallable(): void {
$this->getBus()->push(['\Test\Command\AsyncBusTestCase', 'DummyCommand']);
$this->runJobs();
$this->assertEquals('static', self::$lastCommand);
}
public function testMemberCallable(): void {
$command = new StateFullCommand('bar');
$this->getBus()->push([$command, 'handle']);
$this->runJobs();
$this->assertEquals('bar', self::$lastCommand);
}
public function testFunctionCallable(): void {
$this->getBus()->push('\Test\Command\BasicFunction');
$this->runJobs();
$this->assertEquals('function', self::$lastCommand);
}
public function testClosure(): void {
$this->getBus()->push(function (): void {
AsyncBusTestCase::$lastCommand = 'closure';
});
$this->runJobs();
$this->assertEquals('closure', self::$lastCommand);
}
public function testClosureSelf(): void {
$this->getBus()->push(function (): void {
AsyncBusTestCase::$lastCommand = 'closure-self';
});
$this->runJobs();
$this->assertEquals('closure-self', self::$lastCommand);
}
public function testClosureThis(): void {
// clean class to prevent phpunit putting closure in $this
$test = new ThisClosureTest();
$test->test($this->getBus());
$this->runJobs();
$this->assertEquals('closure-this', self::$lastCommand);
}
public function testClosureBind(): void {
$state = 'bar';
$this->getBus()->push(function () use ($state): void {
AsyncBusTestCase::$lastCommand = 'closure-' . $state;
});
$this->runJobs();
$this->assertEquals('closure-bar', self::$lastCommand);
}
public function testFileFileAccessCommand(): void {
$this->getBus()->push(new FilesystemCommand());
$this->assertEquals('', self::$lastCommand);