[Pkg-owncloud-commits] [php-sabredav] 122/220: Strict vCard validation + repair
David Prévot
taffit at moszumanska.debian.org
Thu May 12 01:21:15 UTC 2016
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository php-sabredav.
commit 5eabd34a7d6003c5fdb1b4ef687cf82b85964e93
Author: Evert Pot <me at evertpot.com>
Date: Thu Mar 31 21:37:52 2016 -0400
Strict vCard validation + repair
---
composer.json | 2 +-
lib/CardDAV/Plugin.php | 55 ++++++++--
tests/Sabre/CardDAV/Backend/Mock.php | 90 +++++++++++++++-
tests/Sabre/CardDAV/ValidateVCardTest.php | 165 ++++++++++++++++++++++++------
4 files changed, 268 insertions(+), 44 deletions(-)
diff --git a/composer.json b/composer.json
index 87c344b..31124f7 100644
--- a/composer.json
+++ b/composer.json
@@ -15,7 +15,7 @@
],
"require": {
"php": ">=5.5.0",
- "sabre/vobject": "~4.0",
+ "sabre/vobject": "dev-master",
"sabre/event" : ">=2.0.0, <4.0.0",
"sabre/xml" : "^1.3.0",
"sabre/http" : "^4.2.1",
diff --git a/lib/CardDAV/Plugin.php b/lib/CardDAV/Plugin.php
index 2149c78..6f046a8 100644
--- a/lib/CardDAV/Plugin.php
+++ b/lib/CardDAV/Plugin.php
@@ -334,12 +334,7 @@ class Plugin extends DAV\ServerPlugin {
$data = stream_get_contents($data);
}
- $before = md5($data);
-
- // Converting the data to unicode, if needed.
- $data = DAV\StringUtil::ensureUTF8($data);
-
- if (md5($data) !== $before) $modified = true;
+ $before = $data;
try {
@@ -366,11 +361,51 @@ class Plugin extends DAV\ServerPlugin {
throw new DAV\Exception\UnsupportedMediaType('This collection can only support vcard objects.');
}
- if (!isset($vobj->UID)) {
- // No UID in vcards is invalid, but we'll just add it in anyway.
- $vobj->add('UID', DAV\UUIDUtil::getUUID());
+ $messages = $vobj->validate(
+ VObject\Node::PROFILE_CARDDAV | VObject\Node::REPAIR
+ );
+
+ $highestLevel = 0;
+ $warningMessage = null;
+
+ // $messages contains a list of problems with the vcard, along with
+ // their severity.
+ foreach($messages as $message) {
+
+ if ($message['level'] > $highestLevel) {
+ // Recording the highest reported error level.
+ $highestLevel = $message['level'];
+ $warningMessage = $message['message'];
+ }
+
+ switch($message['level']) {
+
+ case 1 :
+ // Level 1 means that there was a problem, but it was repaired.
+ $modified = true;
+ break;
+ case 2 :
+ // Level 2 means a warning, but not critical
+ break;
+ case 3 :
+ // Level 3 means a critical error
+ throw new DAV\Exception\UnsupportedMediaType('Validation error in vCard: ' . $message['message']);
+
+ }
+
+ }
+ if ($warningMessage) {
+ $this->server->httpResponse->setHeader(
+ 'Ew',
+ 'vCard validation warning: ' . $warningMessage
+ );
+
+ // Re-serializing object.
$data = $vobj->serialize();
- $modified = true;
+ if (strcmp($data, $before) !== 0) {
+ // This ensures that the system does not send an ETag back.
+ $modified = true;
+ }
}
// Destroy circular references to PHP will GC the object.
diff --git a/tests/Sabre/CardDAV/Backend/Mock.php b/tests/Sabre/CardDAV/Backend/Mock.php
index 52e2fff..ba6da4d 100644
--- a/tests/Sabre/CardDAV/Backend/Mock.php
+++ b/tests/Sabre/CardDAV/Backend/Mock.php
@@ -101,6 +101,25 @@ class Mock extends AbstractBackend {
}
+ /**
+ * Returns all cards for a specific addressbook id.
+ *
+ * This method should return the following properties for each card:
+ * * carddata - raw vcard data
+ * * uri - Some unique url
+ * * lastmodified - A unix timestamp
+ *
+ * It's recommended to also return the following properties:
+ * * etag - A unique etag. This must change every time the card changes.
+ * * size - The size of the card in bytes.
+ *
+ * If these last two properties are provided, less time will be spent
+ * calculating them. If they are specified, you can also ommit carddata.
+ * This may speed up certain requests, especially with large cards.
+ *
+ * @param mixed $addressbookId
+ * @return array
+ */
function getCards($addressBookId) {
$cards = [];
@@ -108,34 +127,103 @@ class Mock extends AbstractBackend {
$cards[] = [
'uri' => $uri,
'carddata' => $data,
+ 'etag' => '"' . md5($data) . '"',
+ 'size' => strlen($data)
];
}
return $cards;
}
+ /**
+ * Returns a specfic card.
+ *
+ * The same set of properties must be returned as with getCards. The only
+ * exception is that 'carddata' is absolutely required.
+ *
+ * If the card does not exist, you must return false.
+ *
+ * @param mixed $addressBookId
+ * @param string $cardUri
+ * @return array
+ */
function getCard($addressBookId, $cardUri) {
if (!isset($this->cards[$addressBookId][$cardUri])) {
return false;
}
+ $data = $this->cards[$addressBookId][$cardUri];
return [
'uri' => $cardUri,
- 'carddata' => $this->cards[$addressBookId][$cardUri],
+ 'carddata' => $data,
+ 'etag' => '"' . md5($data) . '"',
+ 'size' => strlen($data)
];
}
+ /**
+ * Creates a new card.
+ *
+ * The addressbook id will be passed as the first argument. This is the
+ * same id as it is returned from the getAddressBooksForUser method.
+ *
+ * The cardUri is a base uri, and doesn't include the full path. The
+ * cardData argument is the vcard body, and is passed as a string.
+ *
+ * It is possible to return an ETag from this method. This ETag is for the
+ * newly created resource, and must be enclosed with double quotes (that
+ * is, the string itself must contain the double quotes).
+ *
+ * You should only return the ETag if you store the carddata as-is. If a
+ * subsequent GET request on the same card does not have the same body,
+ * byte-by-byte and you did return an ETag here, clients tend to get
+ * confused.
+ *
+ * If you don't return an ETag, you can just return null.
+ *
+ * @param mixed $addressBookId
+ * @param string $cardUri
+ * @param string $cardData
+ * @return string|null
+ */
function createCard($addressBookId, $cardUri, $cardData) {
$this->cards[$addressBookId][$cardUri] = $cardData;
+ return '"' . md5($cardData) . '"';
}
+ /**
+ * Updates a card.
+ *
+ * The addressbook id will be passed as the first argument. This is the
+ * same id as it is returned from the getAddressBooksForUser method.
+ *
+ * The cardUri is a base uri, and doesn't include the full path. The
+ * cardData argument is the vcard body, and is passed as a string.
+ *
+ * It is possible to return an ETag from this method. This ETag should
+ * match that of the updated resource, and must be enclosed with double
+ * quotes (that is: the string itself must contain the actual quotes).
+ *
+ * You should only return the ETag if you store the carddata as-is. If a
+ * subsequent GET request on the same card does not have the same body,
+ * byte-by-byte and you did return an ETag here, clients tend to get
+ * confused.
+ *
+ * If you don't return an ETag, you can just return null.
+ *
+ * @param mixed $addressBookId
+ * @param string $cardUri
+ * @param string $cardData
+ * @return string|null
+ */
function updateCard($addressBookId, $cardUri, $cardData) {
$this->cards[$addressBookId][$cardUri] = $cardData;
+ return '"' . md5($cardData) . '"';
}
diff --git a/tests/Sabre/CardDAV/ValidateVCardTest.php b/tests/Sabre/CardDAV/ValidateVCardTest.php
index 8929360..ca69ded 100644
--- a/tests/Sabre/CardDAV/ValidateVCardTest.php
+++ b/tests/Sabre/CardDAV/ValidateVCardTest.php
@@ -42,11 +42,26 @@ class ValidateVCardTest extends \PHPUnit_Framework_TestCase {
}
- function request(HTTP\Request $request) {
+ function request(HTTP\Request $request, $expectedStatus = null) {
$this->server->httpRequest = $request;
$this->server->exec();
+ if ($expectedStatus) {
+
+ $realStatus = $this->server->httpResponse->getStatus();
+
+ $msg = '';
+ if ($realStatus!==$expectedStatus) {
+ $msg = 'Response body: ' . $this->server->httpResponse->getBodyAsString();
+ }
+ $this->assertEquals(
+ $expectedStatus,
+ $realStatus,
+ $msg
+ );
+ }
+
return $this->server->httpResponse;
}
@@ -66,38 +81,125 @@ class ValidateVCardTest extends \PHPUnit_Framework_TestCase {
function testCreateFileValid() {
- $request = HTTP\Sapi::createFromServerArray([
- 'REQUEST_METHOD' => 'PUT',
- 'REQUEST_URI' => '/addressbooks/admin/addressbook1/blabla.vcf',
- ]);
- $request->setBody("BEGIN:VCARD\r\nUID:foo\r\nEND:VCARD\r\n");
+ $request = new HTTP\Request(
+ 'PUT',
+ '/addressbooks/admin/addressbook1/blabla.vcf'
+ );
+
+ $vcard = <<<VCF
+BEGIN:VCARD
+VERSION:4.0
+UID:foo
+FN:Firstname LastName
+N:LastName;FirstName;;;
+END:VCARD
+VCF;
+ $request->setBody($vcard);
+
+ $response = $this->request($request, 201);
+
+ // The custom Ew header should not be set
+ $this->assertNull(
+ $response->getHeader('Ew')
+ );
+ // Valid, non-auto-fixed responses should contain an ETag.
+ $this->assertTrue(
+ $response->getHeader('ETag') !== null,
+ 'We did not receive an etag'
+ );
- $response = $this->request($request);
- $this->assertEquals(201, $response->status, 'Incorrect status returned! Full response body: ' . $response->body);
$expected = [
- 'uri' => 'blabla.vcf',
- 'carddata' => "BEGIN:VCARD\r\nUID:foo\r\nEND:VCARD\r\n",
+ 'uri' => 'blabla.vcf',
+ 'carddata' => $vcard,
+ 'size' => strlen($vcard),
+ 'etag' => '"' . md5($vcard) . '"',
];
$this->assertEquals($expected, $this->cardBackend->getCard('addressbook1', 'blabla.vcf'));
}
- function testCreateFileNoUID() {
+ /**
+ * This test creates an intentionally broken vCard that vobject is able
+ * to automatically repair.
+ *
+ * @depends testCreateFileValid
+ */
+ function testCreateVCardAutoFix() {
$request = new HTTP\Request(
'PUT',
'/addressbooks/admin/addressbook1/blabla.vcf'
);
- $request->setBody("BEGIN:VCARD\r\nEND:VCARD\r\n");
- $response = $this->request($request);
+ // The error in this vcard is that there's not enough semi-colons in N
+ $vcard = <<<VCF
+BEGIN:VCARD
+VERSION:4.0
+UID:foo
+FN:Firstname LastName
+N:LastName;FirstName;;
+END:VCARD
+VCF;
- $this->assertEquals(201, $response->status, 'Incorrect status returned! Full response body: ' . $response->body);
+ $request->setBody($vcard);
+
+ $response = $this->request($request, 201);
+
+ // Auto-fixed vcards should NOT return an etag
+ $this->assertNull(
+ $response->getHeader('ETag')
+ );
+
+ // We should have gotten an Ew header
+ $this->assertNotNull(
+ $response->getHeader('Ew')
+ );
+
+ $expectedVCard = <<<VCF
+BEGIN:VCARD\r
+VERSION:4.0\r
+UID:foo\r
+FN:Firstname LastName\r
+N:LastName;FirstName;;;\r
+END:VCARD\r
+
+VCF;
+
+ $expected = [
+ 'uri' => 'blabla.vcf',
+ 'carddata' => $expectedVCard,
+ 'size' => strlen($expectedVCard),
+ 'etag' => '"' . md5($expectedVCard) . '"',
+ ];
+
+ $this->assertEquals($expected, $this->cardBackend->getCard('addressbook1', 'blabla.vcf'));
+
+ }
+
+ function testCreateFileNoUID() {
+
+ $request = new HTTP\Request(
+ 'PUT',
+ '/addressbooks/admin/addressbook1/blabla.vcf'
+ );
+ $vcard = <<<VCF
+BEGIN:VCARD
+VERSION:4.0
+FN:Firstname LastName
+N:LastName;FirstName;;;
+END:VCARD
+VCF;
+ $request->setBody($vcard);
+
+ $response = $this->request($request, 201);
$foo = $this->cardBackend->getCard('addressbook1', 'blabla.vcf');
- $this->assertTrue(strpos($foo['carddata'], 'UID') !== false);
+ $this->assertTrue(
+ strpos($foo['carddata'], 'UID') !== false,
+ print_r($foo,true)
+ );
}
function testCreateFileJson() {
@@ -106,14 +208,14 @@ class ValidateVCardTest extends \PHPUnit_Framework_TestCase {
'PUT',
'/addressbooks/admin/addressbook1/blabla.vcf'
);
- $request->setBody('[ "vcard" , [ [ "UID" , {}, "text", "foo" ] ] ]');
+ $request->setBody('[ "vcard" , [ [ "VERSION", {}, "text", "4.0"], [ "UID" , {}, "text", "foo" ], [ "FN", {}, "text", "FirstName LastName"] ] ]');
$response = $this->request($request);
$this->assertEquals(201, $response->status, 'Incorrect status returned! Full response body: ' . $response->body);
$foo = $this->cardBackend->getCard('addressbook1', 'blabla.vcf');
- $this->assertEquals("BEGIN:VCARD\r\nUID:foo\r\nEND:VCARD\r\n", $foo['carddata']);
+ $this->assertEquals("BEGIN:VCARD\r\nVERSION:4.0\r\nUID:foo\r\nFN:FirstName LastName\r\nEND:VCARD\r\n", $foo['carddata']);
}
@@ -134,34 +236,33 @@ class ValidateVCardTest extends \PHPUnit_Framework_TestCase {
function testUpdateFile() {
$this->cardBackend->createCard('addressbook1', 'blabla.vcf', 'foo');
- $request = HTTP\Sapi::createFromServerArray([
- 'REQUEST_METHOD' => 'PUT',
- 'REQUEST_URI' => '/addressbooks/admin/addressbook1/blabla.vcf',
- ]);
-
- $response = $this->request($request);
+ $request = new HTTP\Request(
+ 'PUT',
+ '/addressbooks/admin/addressbook1/blabla.vcf'
+ );
- $this->assertEquals(415, $response->status);
+ $response = $this->request($request, 415);
}
function testUpdateFileParsableBody() {
$this->cardBackend->createCard('addressbook1', 'blabla.vcf', 'foo');
- $request = HTTP\Sapi::createFromServerArray([
- 'REQUEST_METHOD' => 'PUT',
- 'REQUEST_URI' => '/addressbooks/admin/addressbook1/blabla.vcf',
- ]);
- $body = "BEGIN:VCARD\r\nUID:foo\r\nEND:VCARD\r\n";
- $request->setBody($body);
+ $request = new HTTP\Request(
+ 'PUT',
+ '/addressbooks/admin/addressbook1/blabla.vcf'
+ );
- $response = $this->request($request);
+ $body = "BEGIN:VCARD\r\nVERSION:4.0\r\nUID:foo\r\nFN:FirstName LastName\r\nEND:VCARD\r\n";
+ $request->setBody($body);
- $this->assertEquals(204, $response->status);
+ $response = $this->request($request, 204);
$expected = [
'uri' => 'blabla.vcf',
'carddata' => $body,
+ 'size' => strlen($body),
+ 'etag' => '"' . md5($body) . '"',
];
$this->assertEquals($expected, $this->cardBackend->getCard('addressbook1', 'blabla.vcf'));
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-php/php-sabredav.git
More information about the Pkg-owncloud-commits
mailing list