upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-29 18:11:41 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

fix(config): Throw PreconditionException always when it didn't match

Browse source 
Previously even when the precondition did not match, the call "passed"
when the after value was the expected one. This however can lead to
race conditions, duplicate code excutions and other things.

Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2024-09-13 09:16:35 +02:00
parent c9e4598360
commit dcd97e1234
No known key found for this signature in database
GPG key ID: 74434EFE0D2E2205
2 changed files with 39 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/private/AllConfig.php
View file

@ -242,10 +242,10 @@ class AllConfig implements IConfig {
$prevValue = $this->getUserValue($userId, $appName, $key, null);
if ($prevValue !== null) {
if ($prevValue === (string)$value) {
return;
} elseif ($preCondition !== null && $prevValue !== (string)$preCondition) {
if ($preCondition !== null && $prevValue !== (string)$preCondition) {
throw new PreConditionNotMetException();
} elseif ($prevValue === (string)$value) {
return;
} else {
$qb = $this->connection->getQueryBuilder();
$qb->update('preferences')

36
tests/lib/AllConfigTest.php
View file

@ -182,6 +182,42 @@ class AllConfigTest extends \Test\TestCase {
$config->deleteUserValue('userPreCond1', 'appPreCond', 'keyPreCond');
}
public function testSetUserValueWithPreConditionFailureWhenResultStillMatches(): void {
$this->expectException(\OCP\PreConditionNotMetException::class);
$config = $this->getConfig();
$selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?';
$config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond');
$result = $this->connection->executeQuery($selectAllSQL, ['userPreCond1'])->fetchAll();
$this->assertCount(1, $result);
$this->assertEquals([
'userid' => 'userPreCond1',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond'
], $result[0]);
// test if the method throws with invalid precondition when the value is the same
$config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond', 'valuePreCond3');
$result = $this->connection->executeQuery($selectAllSQL, ['userPreCond1'])->fetchAll();
$this->assertCount(1, $result);
$this->assertEquals([
'userid' => 'userPreCond1',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond'
], $result[0]);
// cleanup
$config->deleteUserValue('userPreCond1', 'appPreCond', 'keyPreCond');
}
public function testSetUserValueUnchanged() {
// TODO - FIXME until the dependency injection is handled properly (in AllConfig)
$this->markTestSkipped('Skipped because this is just testable if database connection can be injected');