Port hack from 5.0.12 to fix wrongly escaped parameters. #146

Add a test for it and remove validator plugin as it's now done
in the CardDAV plugin to avoid parsing unnecessarily.

appinfo/remote.php

@@ -61,7 +61,6 @@ $server->addPlugin(new OCA\Contacts\CardDAV\Plugin());
 $server->addPlugin(new Sabre_DAVACL_Plugin());
 $server->addPlugin(new Sabre_DAV_Browser_Plugin(false)); // Show something in the Browser, but no upload
 $server->addPlugin(new Sabre_CardDAV_VCFExportPlugin());
-$server->addPlugin(new OCA\Contacts\CardDAV\ValidatorPlugin());
 
 if(defined('DEBUG') && DEBUG) {
 	$server->debugExceptions = true;

lib/carddav/validatorplugin.php

@@ -1,124 +0,0 @@
-<?php
-
-namespace OCA\Contacts\CardDAV;
-
-use Sabre\VObject;
-
-/**
- * vCard validator
- *
- * Validates and tries to fix broken vCards before they're being
- * handed over to Sabre and written to storage.
- *
- * @copyright Copyright (C) 2013 Thomas Tanghus
- * @author Thomas Tanghus (http://tanghus.net/)
- */
-class ValidatorPlugin extends \Sabre_DAV_ServerPlugin {
-
-	/**
-	* Reference to Server class
-	*
-	* @var Sabre_DAV_Server
-	*/
-	protected $server;
-
-	/**
-	* Initializes the plugin and registers event handlers
-	*
-	* @param Sabre_DAV_Server $server
-	* @return void
-	*/
-	public function initialize(\Sabre_DAV_Server $server) {
-
-		$this->server = $server;
-		$server->subscribeEvent('beforeWriteContent', array($this, 'beforeWriteContent'), 90);
-		$server->subscribeEvent('beforeCreateFile', array($this, 'beforeCreateFile'), 90);
-
-	}
-
-	/**
-	* This method is triggered before a file gets updated with new content.
-	*
-	* This plugin uses this method to ensure that Card nodes receive valid
-	* vcard data.
-	*
-	* @param string $path
-	* @param Sabre_DAV_IFile $node
-	* @param resource $data
-	* @return void
-	*/
-	public function beforeWriteContent($path, \Sabre_DAV_IFile $node, &$data) {
-
-		if (!$node instanceof \Sabre_CardDAV_ICard) {
-			return;
-		}
-
-		$this->validateVCard($data);
-
-	}
-
-	/**
-	* This method is triggered before a new file is created.
-	*
-	* This plugin uses this method to ensure that Card nodes receive valid
-	* vcard data.
-	*
-	* @param string $path
-	* @param resource $data
-	* @param Sabre_DAV_ICollection $parentNode
-	* @return void
-	*/
-	public function beforeCreateFile($path, &$data, \Sabre_DAV_ICollection $parentNode) {
-
-		if (!$parentNode instanceof \Sabre_CardDAV_IAddressBook) {
-			return;
-		}
-
-		$this->validateVCard($data);
-
-	}
-
-	/**
-	* Checks if the submitted vCard data is in fact, valid.
-	*
-	* An exception is thrown if it's not.
-	*
-	* @param resource|string $data
-	* @return void
-	*/
-	protected function validateVCard(&$data) {
-
-		// If it's a stream, we convert it to a string first.
-		if (is_resource($data)) {
-			$data = stream_get_contents($data);
-		}
-
-		// Converting the data to unicode, if needed.
-		$data = \Sabre_DAV_StringUtil::ensureUTF8($data);
-
-		try {
-
-			$vobj = VObject\Reader::read($data);
-
-		} catch (VObject\ParseException $e) {
-
-			throw new \Sabre_DAV_Exception_UnsupportedMediaType(
-				'This resource only supports valid vcard data. Parse error: ' . $e->getMessage()
-			);
-
-		}
-
-		if ($vobj->name !== 'VCARD') {
-			throw new \Sabre_DAV_Exception_UnsupportedMediaType(
-				'This collection can only support vcard objects.'
-			);
-		}
-
-		if (!isset($vobj->UID)) {
-			$uid = substr(md5(rand().time()), 0, 10);
-			\OCP\Util::writeLog('contacts', __METHOD__.', Adding UID: ' . $uid, \OCP\Util::DEBUG);
-			$vobj->add('UID', $uid);
-		}
-
-	}
-}

lib/contact.php

@@ -97,6 +97,8 @@ class Contact extends VObject\VCard implements IPIMObject {
 						case 'fullname':
 							$this->props['displayname'] = $value;
 							$this->FN = $value;
+							// Set it to saved again as we're not actually changing anything
+							$this->setSaved();
 							break;
 					}
 				}
@@ -338,7 +340,9 @@ class Contact extends VObject\VCard implements IPIMObject {
 						// Save internal values
 						$data = $result['carddata'];
 						$this->props['carddata'] = $result['carddata'];
-						$this->props['lastmodified'] = $result['lastmodified'];
+						$this->props['lastmodified'] = isset($result['lastmodified'])
+							? $result['lastmodified']
+							: null;
 						$this->props['displayname'] = $result['displayname'];
 						$this->props['permissions'] = $result['permissions'];
 					} else {
@@ -760,7 +764,7 @@ class Contact extends VObject\VCard implements IPIMObject {
 		return $this->props['retrieved'];
 	}
 
-	public function setSaved($state) {
+	public function setSaved($state = true) {
 		$this->props['saved'] = $state;
 	}
 

lib/vobject/vcard.php

@@ -96,8 +96,9 @@ class VCard extends VObject\Component\VCard {
 	}
 
 	/**
-	* @brief Decode properties for upgrading from v. 2.1
+	* Decode properties for upgrading from v. 2.1
-	* @param $property Reference to a \Sabre\VObject\Property.
+	*
+	* @param Sabre_VObject_Property $property Reference to a \Sabre\VObject\Property.
 	* The only encoding allowed in version 3.0 is 'b' for binary. All encoded strings
 	* must therefore be decoded and the parameters removed.
 	*/
@@ -128,6 +129,29 @@ class VCard extends VObject\Component\VCard {
 		}
 	}
 
+	/**
+	* Work around issue in older VObject sersions
+	* https://github.com/fruux/sabre-vobject/issues/24
+	*
+	* @param Sabre_VObject_Property $property Reference to a Sabre_VObject_Property.
+	*/
+	public function fixPropertyParameters(&$property) {
+		// Work around issue in older VObject sersions
+		// https://github.com/fruux/sabre-vobject/issues/24
+		foreach($property->parameters as $key=>$parameter) {
+			$delim = '';
+			if(strpos($parameter->value, ',') === false) {
+				continue;
+			}
+			$values = explode(',', $parameter->value);
+			$values = array_map('trim', $values);
+			$parameter->value = array_shift($values);
+			foreach($values as $value) {
+				$property->add($parameter->name, $value);
+			}
+		}
+	}
+
 	/**
 	* Validates the node for correctness.
 	*
@@ -154,6 +178,8 @@ class VCard extends VObject\Component\VCard {
 			$this->VERSION = self::DEFAULT_VERSION;
 			foreach($this->children as &$property) {
 				$this->decodeProperty($property);
+				$this->fixPropertyParameters($property);
+				/* What exactly was I thinking here?
 				switch((string)$property->name) {
 					case 'LOGO':
 					case 'SOUND':
@@ -161,7 +187,7 @@ class VCard extends VObject\Component\VCard {
 						if(isset($property['TYPE']) && strpos((string)$property['TYPE'], '/') === false) {
 							$property['TYPE'] = 'image/' . strtolower($property['TYPE']);
 						}
-				}
+				}*/
 			}
 		}
 
@@ -251,6 +277,11 @@ class VCard extends VObject\Component\VCard {
 			}
 		}
 
+		if (($options & self::REPAIR) || ($options & self::UPGRADE)) {
+			$now = new \DateTime;
+			$this->REV = $now->format(\DateTime::W3C);
+		}
+
 		return array_merge(
 			parent::validate($options),
 			$warnings

tests/data/test4.vcf

@@ -1,11 +1,9 @@
 BEGIN:VCARD
 VERSION:2.1
-N:Adieu Berth
+FN:Adieu Demain
-FN:Adieu Berth
-ORG:Restaurant
 NOTE;ENCODING=QUOTED-PRINTABLE:Ouverture =E0 partir de 19h=0D=0AFerm=E9e le mardi hors saison
-TEL;WORK;VOICE:04 93 34 78 84
+TEL;WORK;VOICE:12 34 56 78
-ADR;HOME;ENCODING=QUOTED-PRINTABLE:;;26=0D=0ARue Vauban;Antibes;;06600
+ADR;HOME;ENCODING=QUOTED-PRINTABLE:;;666=0D=0ARue Vauban;Antibes;;666
-LABEL;HOME;ENCODING=QUOTED-PRINTABLE:26=0D=0ARue Vauban=0D=0AAntibes 06600
+LABEL;HOME;ENCODING=QUOTED-PRINTABLE:666=0D=0ARue Vauban=0D=0AAntibes 666
 REV:20120610T192843Z
 END:VCARD

tests/data/test6.vcf

@@ -0,0 +1,8 @@
+BEGIN:VCARD
+VERSION:3.0
+PRODID://WebDAV Collaborator 1.1.35-615//Import from Outlook//EN
+CLASS:PUBLIC
+UID:00000000CBAB1D2F0B42AE46B5131D1FA629CACD84082100
+FN:Escaped Parameters
+TEL;TYPE=PREF\,WORK\,VOICE:123456789
+END:VCARD

tests/lib/vobject_test.php

@@ -27,6 +27,19 @@ class Test_VObjects extends PHPUnit_Framework_TestCase {
 		$this->assertEquals('Fermée;Adèle;;;', (string)$obj->N);
 	}
 
+	public function testEscapedParameters() {
+		$carddata = file_get_contents(__DIR__ . '/../data/test6.vcf');
+		$obj = \Sabre\VObject\Reader::read(
+			$carddata,
+			\Sabre\VObject\Reader::OPTION_IGNORE_INVALID_LINES
+		);
+		$obj->validate($obj::REPAIR|$obj::UPGRADE);
+
+		$this->assertEquals('3.0', (string)$obj->VERSION);
+		$this->assertEquals('Parameters;Escaped;;;', (string)$obj->N);
+		$this->assertEquals('TEL;TYPE=PREF;TYPE=WORK;TYPE=VOICE:123456789' . "\r\n", $obj->TEL->serialize());
+	}
+
 	public function testGroupProperty() {
 		$arr = array(
 			'Home',