mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #12242 from nextcloud/uid-conccurency 

Uid concurrency check fix for vcards
This commit is contained in:
John Molakvoæ 2018-11-05 15:24:44 +01:00 committed by GitHub
parent 41cace8c49 68968a870b
commit 9ea6573416
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 139 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
apps/dav/lib/CardDAV/CardDavBackend.php
View File

@ -611,6 +611,19 @@ class CardDavBackend implements BackendInterface, SyncSupport {
$etag = md5($cardData);
$uid = $this->getUID($cardData);
$q = $this->db->getQueryBuilder();
$q->select('uid')
->from('cards')
->where($q->expr()->eq('addressbookid', $q->createNamedParameter($addressBookId)))
->andWhere($q->expr()->eq('uid', $q->createNamedParameter($uid)))
->setMaxResults(1);
$result = $q->execute();
$count = (bool) $result->fetchColumn();
$result->closeCursor();
if ($count) {
throw new \Sabre\DAV\Exception\BadRequest('VCard object with uid already exists in this addressbook collection.');
}
$query = $this->db->getQueryBuilder();
$query->insert('cards')
->values([

167
apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
View File

@ -43,6 +43,7 @@ use OCP\IL10N;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Share\IManager as ShareManager;
use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\PropPatch;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Property\Text;
@ -87,7 +88,7 @@ class CardDavBackendTest extends TestCase {
const UNIT_TEST_USER1 = 'principals/users/carddav-unit-test1';
const UNIT_TEST_GROUP = 'principals/groups/carddav-unit-test-group';
private $vcardTest = 'BEGIN:VCARD'.PHP_EOL.
private $vcardTest0 = 'BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:Test'.PHP_EOL.
@ -95,6 +96,29 @@ class CardDavBackendTest extends TestCase {
'N:Test;;;;'.PHP_EOL.
'END:VCARD';
private $vcardTest1 = 'BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:Test2'.PHP_EOL.
'FN:Test2'.PHP_EOL.
'N:Test2;;;;'.PHP_EOL.
'END:VCARD';
private $vcardTest2 = 'BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'UID:Test3'.PHP_EOL.
'FN:Test3'.PHP_EOL.
'N:Test3;;;;'.PHP_EOL.
'END:VCARD';
private $vcardTestNoUID = 'BEGIN:VCARD'.PHP_EOL.
'VERSION:3.0'.PHP_EOL.
'PRODID:-//Sabre//Sabre VObject 4.1.2//EN'.PHP_EOL.
'FN:TestNoUID'.PHP_EOL.
'N:TestNoUID;;;;'.PHP_EOL.
'END:VCARD';
public function setUp() {
parent::setUp();
@ -224,8 +248,8 @@ class CardDavBackendTest extends TestCase {
$uri = $this->getUniqueID('card');
// updateProperties is expected twice, once for createCard and once for updateCard
$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, $this->vcardTest);
$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, $this->vcardTest);
$backend->expects($this->at(0))->method('updateProperties')->with($bookId, $uri, $this->vcardTest0);
$backend->expects($this->at(1))->method('updateProperties')->with($bookId, $uri, $this->vcardTest0);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -233,16 +257,16 @@ class CardDavBackendTest extends TestCase {
->with('\OCA\DAV\CardDAV\CardDavBackend::createCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
return $e->getArgument('addressBookId') === $bookId &&
$e->getArgument('cardUri') === $uri &&
$e->getArgument('cardData') === $this->vcardTest;
$e->getArgument('cardData') === $this->vcardTest0;
}));
// create a card
$backend->createCard($bookId, $uri, $this->vcardTest);
$backend->createCard($bookId, $uri, $this->vcardTest0);
// get all the cards
$cards = $backend->getCards($bookId);
$this->assertEquals(1, count($cards));
$this->assertEquals($this->vcardTest, $cards[0]['carddata']);
$this->assertEquals($this->vcardTest0, $cards[0]['carddata']);
// get the cards
$card = $backend->getCard($bookId, $uri);
@ -252,7 +276,7 @@ class CardDavBackendTest extends TestCase {
$this->assertArrayHasKey('lastmodified', $card);
$this->assertArrayHasKey('etag', $card);
$this->assertArrayHasKey('size', $card);
$this->assertEquals($this->vcardTest, $card['carddata']);
$this->assertEquals($this->vcardTest0, $card['carddata']);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -260,13 +284,13 @@ class CardDavBackendTest extends TestCase {
->with('\OCA\DAV\CardDAV\CardDavBackend::updateCard', $this->callback(function(GenericEvent $e) use ($bookId, $uri) {
return $e->getArgument('addressBookId') === $bookId &&
$e->getArgument('cardUri') === $uri &&
$e->getArgument('cardData') === $this->vcardTest;
$e->getArgument('cardData') === $this->vcardTest0;
}));
// update the card
$backend->updateCard($bookId, $uri, $this->vcardTest);
$backend->updateCard($bookId, $uri, $this->vcardTest0);
$card = $backend->getCard($bookId, $uri);
$this->assertEquals($this->vcardTest, $card['carddata']);
$this->assertEquals($this->vcardTest0, $card['carddata']);
// Expect event
$this->dispatcher->expects($this->at(0))
@ -284,7 +308,78 @@ class CardDavBackendTest extends TestCase {
}
public function testMultiCard() {
$this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
->setMethods(['updateProperties'])->getMock();
// create a new address book
$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []);
$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER);
$this->assertEquals(1, count($books));
$bookId = $books[0]['id'];
// create a card
$uri0 = self::getUniqueID('card');
$this->backend->createCard($bookId, $uri0, $this->vcardTest0);
$uri1 = self::getUniqueID('card');
$this->backend->createCard($bookId, $uri1, $this->vcardTest1);
$uri2 = self::getUniqueID('card');
$this->backend->createCard($bookId, $uri2, $this->vcardTest2);
// get all the cards
$cards = $this->backend->getCards($bookId);
$this->assertEquals(3, count($cards));
usort($cards, function ($a, $b) { return $a['id'] < $b['id'] ? -1 : 1; });
$this->assertEquals($this->vcardTest0, $cards[0]['carddata']);
$this->assertEquals($this->vcardTest1, $cards[1]['carddata']);
$this->assertEquals($this->vcardTest2, $cards[2]['carddata']);
// get the cards 1 & 2 (not 0)
$cards = $this->backend->getMultipleCards($bookId, [$uri1, $uri2]);
$this->assertEquals(2, count($cards));
usort($cards, function ($a, $b) { return $a['id'] < $b['id'] ? -1 : 1; });
foreach($cards as $index => $card) {
$this->assertArrayHasKey('id', $card);
$this->assertArrayHasKey('uri', $card);
$this->assertArrayHasKey('lastmodified', $card);
$this->assertArrayHasKey('etag', $card);
$this->assertArrayHasKey('size', $card);
$this->assertEquals($this->{ 'vcardTest'.($index+1) }, $card['carddata']);
}
// delete the card
$this->backend->deleteCard($bookId, $uri0);
$this->backend->deleteCard($bookId, $uri1);
$this->backend->deleteCard($bookId, $uri2);
$cards = $this->backend->getCards($bookId);
$this->assertEquals(0, count($cards));
}
public function testMultipleUIDOnDifferentAddressbooks() {
$this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
->setMethods(['updateProperties'])->getMock();
// create 2 new address books
$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []);
$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example2', []);
$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER);
$this->assertEquals(2, count($books));
$bookId0 = $books[0]['id'];
$bookId1 = $books[1]['id'];
// create a card
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId0, $uri0, $this->vcardTest0);
// create another card with same uid but in second address book
$uri1 = $this->getUniqueID('card');
$this->backend->createCard($bookId1, $uri1, $this->vcardTest0);
}
public function testMultipleUIDDenied() {
$this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
->setMethods(['updateProperties'])->getMock();
@ -297,37 +392,29 @@ class CardDavBackendTest extends TestCase {
// create a card
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, $this->vcardTest);
$this->backend->createCard($bookId, $uri0, $this->vcardTest0);
// create another card with same uid
$uri1 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri1, $this->vcardTest);
$uri2 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri2, $this->vcardTest);
// get all the cards
$cards = $this->backend->getCards($bookId);
$this->assertEquals(3, count($cards));
$this->assertEquals($this->vcardTest, $cards[0]['carddata']);
$this->assertEquals($this->vcardTest, $cards[1]['carddata']);
$this->assertEquals($this->vcardTest, $cards[2]['carddata']);
// get the cards
$cards = $this->backend->getMultipleCards($bookId, [$uri1, $uri2]);
$this->assertEquals(2, count($cards));
foreach($cards as $card) {
$this->assertArrayHasKey('id', $card);
$this->assertArrayHasKey('uri', $card);
$this->assertArrayHasKey('lastmodified', $card);
$this->assertArrayHasKey('etag', $card);
$this->assertArrayHasKey('size', $card);
$this->assertEquals($this->vcardTest, $card['carddata']);
$this->expectException(BadRequest::class);
$test = $this->backend->createCard($bookId, $uri1, $this->vcardTest0);
}
// delete the card
$this->backend->deleteCard($bookId, $uri0);
$this->backend->deleteCard($bookId, $uri1);
$this->backend->deleteCard($bookId, $uri2);
$cards = $this->backend->getCards($bookId);
$this->assertEquals(0, count($cards));
public function testNoValidUID() {
$this->backend = $this->getMockBuilder(CardDavBackend::class)
->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher])
->setMethods(['updateProperties'])->getMock();
// create a new address book
$this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []);
$books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER);
$this->assertEquals(1, count($books));
$bookId = $books[0]['id'];
// create a card without uid
$uri1 = $this->getUniqueID('card');
$this->expectException(BadRequest::class);
$test = $this->backend->createCard($bookId, $uri1, $this->vcardTestNoUID);
}
public function testDeleteWithoutCard() {
@ -364,7 +451,7 @@ class CardDavBackendTest extends TestCase {
->method('purgeProperties');
// create a card
$this->backend->createCard($bookId, $uri, $this->vcardTest);
$this->backend->createCard($bookId, $uri, $this->vcardTest0);
// delete the card
$this->assertTrue($this->backend->deleteCard($bookId, $uri));
@ -387,7 +474,7 @@ class CardDavBackendTest extends TestCase {
// add a change
$uri0 = $this->getUniqueID('card');
$this->backend->createCard($bookId, $uri0, $this->vcardTest);
$this->backend->createCard($bookId, $uri0, $this->vcardTest0);
// look for changes
$changes = $this->backend->getChangesForAddressBook($bookId, $syncToken, 1);