Server: ContactManager CreateOrUpdate function messes up contact

[call-me-matt]

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

Background

Hey, I am trying to modify a contact within an app-controller, but for some reason the modified contacts are messed up during the storage process. I looked at the sources, but could not find out if its a bug or just me misunderstanding the headers. Tried to search in the documentation, but only found an empty page at https://docs.nextcloud.com/server/latest/developer_manual/api.html . That brings me here...

Steps to reproduce

1. grab a contact $contact = $addressBook->search($contactId, ['UID'], [])[0];
2. make a copy $changed = $contact;
3. change it (or not)
4. save it $addressBook->createOrUpdate($changed);
5. verify: $addressBook->search($contactId, ['UID'], [])[0];

Expected behaviour

The contact is changed (or not) and remains with correct syntax.

Actual behaviour

The contact fields are not as expected:

the contact:
array(9) { ["URI"]=> string(40) "EBD3B9AA-FC6E-45DF-9294-C069141DA1F0.vcf" ["VERSION"]=> string(3) "4.0" ["PRODID"]=> string(28) "-//Nextcloud Contacts v3.2.0" ["UID"]=> string(36) "5ee6e61a-e281-47c6-ab17-11fb37bb7740" ["REV"]=> string(16) "20200417T190130Z" ["FN"]=> string(4) "Test" ["ADR"]=> string(6) ";;;;;;" ["EMAIL"]=> array(1) { [0]=> string(0) "" } ["TEL"]=> array(1) { [0]=> string(0) "" } }

the updated contact (identical):
array(9) { ["URI"]=> string(40) "EBD3B9AA-FC6E-45DF-9294-C069141DA1F0.vcf" ["VERSION"]=> string(3) "4.0" ["PRODID"]=> string(28) "-//Nextcloud Contacts v3.2.0" ["UID"]=> string(36) "5ee6e61a-e281-47c6-ab17-11fb37bb7740" ["REV"]=> string(16) "20200417T190130Z" ["FN"]=> string(4) "Test" ["ADR"]=> string(6) ";;;;;;" ["EMAIL"]=> array(1) { [0]=> string(0) "" } ["TEL"]=> array(1) { [0]=> string(0) "" } }

the return value of the update function (identical):
array(9) { ["URI"]=> string(40) "EBD3B9AA-FC6E-45DF-9294-C069141DA1F0.vcf" ["VERSION"]=> string(3) "4.0" ["PRODID"]=> string(28) "-//Nextcloud Contacts v3.2.0" ["UID"]=> string(36) "5ee6e61a-e281-47c6-ab17-11fb37bb7740" ["REV"]=> string(16) "20200417T190130Z" ["FN"]=> string(4) "Test" ["ADR"]=> string(6) ";;;;;;" ["EMAIL"]=> array(1) { [0]=> string(0) "" } ["TEL"]=> array(1) { [0]=> string(0) "" } }

but the stored contact (not identical, see ADR):
array(9) { ["URI"]=> string(40) "EBD3B9AA-FC6E-45DF-9294-C069141DA1F0.vcf" ["VERSION"]=> string(3) "4.0" ["PRODID"]=> string(28) "-//Nextcloud Contacts v3.2.0" ["UID"]=> string(36) "5ee6e61a-e281-47c6-ab17-11fb37bb7740" ["REV"]=> string(16) "20200417T190130Z" ["FN"]=> string(4) "Test" ["ADR"]=> string(18) "\;\;\;\;\;\;;;;;;;" ["EMAIL"]=> array(1) { [0]=> string(0) "" } ["TEL"]=> array(1) { [0]=> string(0) "" } }

Also trying the types-option did not change things $contact = $addressBook->search($contactId, ['UID'], ['types' => true])[0];, as the function seems to convert the array to vCard. For me it looks like there is a bug happening somewhere behind that.

Server configuration

latest nextcloud docker on ubuntu

[call-me-matt]

@skjnldsv, @ChristophWurst can you help me here?

[skjnldsv]

Maybe it's because createOrUpdate is not expecting the same result as what search returns? For example ADR is most likely expecting an array [], and not a string of semi-colons :)

Or this is a bug in the serialize methods
https://github.com/nextcloud/3rdparty/blob/b0afba6d6508a1c85332cf8c61e90ad91b289ebc/sabre/vobject/lib/Component.php#L329
https://github.com/nextcloud/3rdparty/blob/b0afba6d6508a1c85332cf8c61e90ad91b289ebc/sabre/vobject/lib/Property.php#L279

[skjnldsv]

Regarding data, we're using the library sabre/dav & sabre/vobject
So you have documentation there: https://sabre.io/vobject/vcard/

[call-me-matt]

Thank you for your instant reply!

Maybe it's because createOrUpdate is not expecting the same result as what search returns?

I think it is just taking the identifier of the contact and then downloading the vCard, so the search doesn't have an impact here.

[skjnldsv]

Also, they apparently recommand to not use createProperty anymore
https://sabre.io/vobject/upgrade_to_3/#creating-properties

Can you make sure it still misbehave with ->add directly?

[skjnldsv]

https://sabre.io/vobject/vcard/#working-with-parameters

[call-me-matt]

Can you make sure it still misbehave with ->add directly?

Unfortunately it didn't change things when I modified /var/www/html/apps/dav/lib/CardDAV/AddressBookImpl.php, replacing the createProperty by add. Even though the linked description sounded like it could help.

[skjnldsv]

I commented on the contacts draft pr as well :)

[skjnldsv]

What's the status of this issue btw?

[call-me-matt]

Well, the createProperty-function does not handle nested arrays well, it seems. Or maybe it's the foreach-loop? In any case this is where the backslashes are introduced for addresses and types (e. g. for phone or mail) seem to get lost. Maybe @georgehrke or @nickvergessen can take a look?

[call-me-matt]

I think the loop in AddressBookImpl needs to be extended:

foreach ($properties as $key => $value) {
    if (is_array($value)) {
        $vCard->remove($key);
        foreach ($value as $entry) {
            if (($key === "ADR" || $key === "PHOTO") && is_string($entry["value"])) {
                $entry["value"] = stripslashes($entry["value"]);
                $entry["value"] = explode(';', $entry["value"]);
            }
            $property = $vCard->createProperty($key, $entry["value"]);
            if (isset($entry["type"])) {
                $property->add('TYPE', $entry["type"]);
            }
            $vCard->add($property);
        }
    }
    else {
        $vCard->$key = $vCard->createProperty($key, $value);
    }
}

As a bonus, it could be checked if the input is well formatted.

[nickvergessen]

Looks good

[call-me-matt]

Realized, the URI gets added as attribute to the vCard - which should not be done:

elseif ($key !== 'URI') {
        $vCard->$key = $vCard->createProperty($key, $value);
}