Merge pull request #19901 from nextcloud/bugfix/noid/vcard-photo-hand…

…ling Improved vcard photo handling
nextcloud · Apr 17, 2020 · ed56619 · ed56619
2 parents 7c4b7e9 + fe4527a
commit ed56619
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 27 deletions.
diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -449,7 +449,7 @@ public function createAddressBook($principalUri, $url, array $properties) {
 	 */
 	public function deleteAddressBook($addressBookId) {
 		$query = $this->db->getQueryBuilder();
-		$query->delete('cards')
+		$query->delete($this->dbCardsTable)
 			->where($query->expr()->eq('addressbookid', $query->createParameter('addressbookid')))
 			->setParameter('addressbookid', $addressBookId)
 			->execute();
@@ -493,15 +493,21 @@ public function deleteAddressBook($addressBookId) {
 	public function getCards($addressBookId) {
 		$query = $this->db->getQueryBuilder();
 		$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
-			->from('cards')
+			->from($this->dbCardsTable)
 			->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)));
 
 		$cards = [];
 
 		$result = $query->execute();
 		while ($row = $result->fetch()) {
 			$row['etag'] = '"' . $row['etag'] . '"';
-			$row['carddata'] = $this->readBlob($row['carddata']);
+
+			$modified = false;
+			$row['carddata'] = $this->readBlob($row['carddata'], $modified);
+			if ($modified) {
+				$row['size'] = strlen($row['carddata']);
+			}
+
 			$cards[] = $row;
 		}
 		$result->closeCursor();
@@ -524,7 +530,7 @@ public function getCards($addressBookId) {
 	public function getCard($addressBookId, $cardUri) {
 		$query = $this->db->getQueryBuilder();
 		$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
-			->from('cards')
+			->from($this->dbCardsTable)
 			->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
 			->andWhere($query->expr()->eq('uri', $query->createNamedParameter($cardUri)))
 			->setMaxResults(1);
@@ -535,7 +541,12 @@ public function getCard($addressBookId, $cardUri) {
 			return false;
 		}
 		$row['etag'] = '"' . $row['etag'] . '"';
-		$row['carddata'] = $this->readBlob($row['carddata']);
+
+		$modified = false;
+		$row['carddata'] = $this->readBlob($row['carddata'], $modified);
+		if ($modified) {
+			$row['size'] = strlen($row['carddata']);
+		}
 
 		return $row;
 	}
@@ -562,7 +573,7 @@ public function getMultipleCards($addressBookId, array $uris) {
 
 		$query = $this->db->getQueryBuilder();
 		$query->select(['id', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
-			->from('cards')
+			->from($this->dbCardsTable)
 			->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
 			->andWhere($query->expr()->in('uri', $query->createParameter('uri')));
 
@@ -572,7 +583,13 @@ public function getMultipleCards($addressBookId, array $uris) {
 
 			while ($row = $result->fetch()) {
 				$row['etag'] = '"' . $row['etag'] . '"';
-				$row['carddata'] = $this->readBlob($row['carddata']);
+
+				$modified = false;
+				$row['carddata'] = $this->readBlob($row['carddata'], $modified);
+				if ($modified) {
+					$row['size'] = strlen($row['carddata']);
+				}
+
 				$cards[] = $row;
 			}
 			$result->closeCursor();
@@ -611,7 +628,7 @@ public function createCard($addressBookId, $cardUri, $cardData) {
 
 		$q = $this->db->getQueryBuilder();
 		$q->select('uid')
-			->from('cards')
+			->from($this->dbCardsTable)
 			->where($q->expr()->eq('addressbookid', $q->createNamedParameter($addressBookId)))
 			->andWhere($q->expr()->eq('uid', $q->createNamedParameter($uid)))
 			->setMaxResults(1);
@@ -676,7 +693,7 @@ public function updateCard($addressBookId, $cardUri, $cardData) {
 		$uid = $this->getUID($cardData);
 		$etag = md5($cardData);
 		$query = $this->db->getQueryBuilder();
-		$query->update('cards')
+		$query->update($this->dbCardsTable)
 			->set('carddata', $query->createNamedParameter($cardData, IQueryBuilder::PARAM_LOB))
 			->set('lastmodified', $query->createNamedParameter(time()))
 			->set('size', $query->createNamedParameter(strlen($cardData)))
@@ -712,7 +729,7 @@ public function deleteCard($addressBookId, $cardUri) {
 			$cardId = null;
 		}
 		$query = $this->db->getQueryBuilder();
-		$ret = $query->delete('cards')
+		$ret = $query->delete($this->dbCardsTable)
 			->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
 			->andWhere($query->expr()->eq('uri', $query->createNamedParameter($cardUri)))
 			->execute();
@@ -872,12 +889,41 @@ protected function addChange($addressBookId, $objectUri, $operation) {
 		]);
 	}
 
-	private function readBlob($cardData) {
+	/**
+	 * @param resource|string $cardData
+	 * @param bool $modified
+	 * @return string
+	 */
+	private function readBlob($cardData, &$modified=false) {
 		if (is_resource($cardData)) {
-			return stream_get_contents($cardData);
+			$cardData = stream_get_contents($cardData);
+		}
+
+		$cardDataArray = explode("\r\n", $cardData);
+
+		$cardDataFiltered = [];
+		$removingPhoto = false;
+		foreach ($cardDataArray as $line) {
+			if (strpos($line, 'PHOTO:data:') === 0
+				&& strpos($line, 'PHOTO:data:image/') !== 0) {
+				// Filter out PHOTO data of non-images
+				$removingPhoto = true;
+				$modified = true;
+				continue;
+			}
+
+			if ($removingPhoto) {
+				if (strpos($line, ' ') === 0) {
+					continue;
+				}
+				// No leading space means this is a new property
+				$removingPhoto = false;
+			}
+
+			$cardDataFiltered[] = $line;
 		}
 
-		return $cardData;
+		return implode("\r\n", $cardDataFiltered);
 	}
 
 	/**
@@ -929,7 +975,11 @@ public function search($addressBookId, $pattern, $searchProperties, $options = [
 		$result->closeCursor();
 
 		return array_map(function ($array) {
-			$array['carddata'] = $this->readBlob($array['carddata']);
+			$modified = false;
+			$array['carddata'] = $this->readBlob($array['carddata'], $modified);
+			if ($modified) {
+				$array['size'] = strlen($array['carddata']);
+			}
 			return $array;
 		}, $cards);
 	}
@@ -994,6 +1044,13 @@ public function getContact($addressBookId, $uri) {
 		$queryResult->closeCursor();
 
 		if (is_array($contact)) {
+			$modified = false;
+			$contact['etag'] = '"' . $contact['etag'] . '"';
+			$contact['carddata'] = $this->readBlob($contact['carddata'], $modified);
+			if ($modified) {
+				$contact['size'] = strlen($contact['carddata']);
+			}
+
 			$result = $contact;
 		}
 

diff --git a/apps/dav/lib/CardDAV/HasPhotoPlugin.php b/apps/dav/lib/CardDAV/HasPhotoPlugin.php
@@ -62,7 +62,12 @@ public function propFind(PropFind $propFind, INode $node) {
 		if ($node instanceof Card) {
 			$propFind->handle($ns . 'has-photo', function () use ($node) {
 				$vcard = Reader::read($node->get());
-				return ($vcard instanceof VCard && $vcard->PHOTO);
+				return $vcard instanceof VCard
+					&& $vcard->PHOTO
+					// Either the PHOTO is a url (doesn't start with data:) or the mimetype has to start with image/
+					&& (strpos($vcard->PHOTO->getValue(), 'data:') !== 0
+						|| strpos($vcard->PHOTO->getValue(), 'data:image/') === 0)
+				;
 			});
 		}
 	}

diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php
@@ -630,7 +630,7 @@ public function testGetCardId() {
 			$this->invokePrivate($this->backend, 'getCardId', [1, 'uri']));
 	}
 
-	
+
 	public function testGetCardIdFailed() {
 		$this->expectException(\InvalidArgumentException::class);
 
@@ -690,15 +690,15 @@ public function testSearch($pattern, $properties, $options, $expected) {
 			);
 		$query->execute();
 		$query->insert($this->dbCardsPropertiesTable)
-				->values(
-						[
-							'addressbookid' => $query->createNamedParameter(0),
-							'cardid' => $query->createNamedParameter($vCardIds[0]),
-							'name' => $query->createNamedParameter('CLOUD'),
-							'value' => $query->createNamedParameter('John@nextcloud.com'),
-							'preferred' => $query->createNamedParameter(0)
-						]
-				);
+			->values(
+				[
+					'addressbookid' => $query->createNamedParameter(0),
+					'cardid' => $query->createNamedParameter($vCardIds[0]),
+					'name' => $query->createNamedParameter('CLOUD'),
+					'value' => $query->createNamedParameter('John@nextcloud.com'),
+					'preferred' => $query->createNamedParameter(0)
+				]
+			);
 		$query->execute();
 		$query->insert($this->dbCardsPropertiesTable)
 			->values(
@@ -783,7 +783,7 @@ public function testGetCardUri() {
 		$this->assertSame('uri', $this->backend->getCardUri($id));
 	}
 
-	
+
 	public function testGetCardUriFailed() {
 		$this->expectException(\InvalidArgumentException::class);
 
@@ -812,7 +812,7 @@ public function testGetContact() {
 		$this->assertSame(0, (int)$result['addressbookid']);
 		$this->assertSame('uri0', $result['uri']);
 		$this->assertSame(5489543, (int)$result['lastmodified']);
-		$this->assertSame('etag0', $result['etag']);
+		$this->assertSame('"etag0"', $result['etag']);
 		$this->assertSame(120, (int)$result['size']);
 
 		// this shouldn't return any result because 'uri1' is in address book 1