nextcloud / contacts

📇 Contacts app for Nextcloud
https://apps.nextcloud.com/apps/contacts
GNU Affero General Public License v3.0
569 stars 173 forks source link

multiple N properties are forbidden and not repaired #378

Open devz3r0 opened 7 years ago

devz3r0 commented 7 years ago

Steps to reproduce

  1. share a contact from user 1 to user 2
  2. switch to user 2
  3. try to change the "group" via user 2 to a contact entry from user 1

as requested from: https://help.nextcloud.com/t/contacts-2-0-1-cant-change-group-on-shared-contacts/21870

Expected behaviour

contact get assigned to group

Actual behaviour

Sabre\DAV\Exception\UnsupportedMediaType: Validation error in vCard: N MUST NOT appear more than once in a VCARD component

/var/www/owncloud/3rdparty/sabre/dav/lib/CardDAV/Plugin.php - line 294: Sabre\CardDAV\Plugin->validateVCard('BEGIN VCARD\r\nVE...', false)
[internal function] Sabre\CardDAV\Plugin->beforeWriteContent('addressbooks/us...', Object(Sabre\CardDAV\Card), 'BEGIN VCARD\r\nVE...', false)
/var/www/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php - line 105: call_user_func_array(Array, Array)
/var/www/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php - line 1128: Sabre\Event\EventEmitter->emit('beforeWriteCont...', Array)
/var/www/owncloud/3rdparty/sabre/dav/lib/DAV/CorePlugin.php - line 513: Sabre\DAV\Server->updateFile('addressbooks/us...', 'BEGIN VCARD\r\nVE...', NULL)
[internal function] Sabre\DAV\CorePlugin->httpPut(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
/var/www/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php - line 105: call_user_func_array(Array, Array)
/var/www/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php - line 479: Sabre\Event\EventEmitter->emit('method PUT', Array)
/var/www/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php - line 254: Sabre\DAV\Server->invokeMethod(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
/var/www/owncloud/apps/dav/lib/Server.php - line 258: Sabre\DAV\Server->exec()
/var/www/owncloud/apps/dav/appinfo/v2/remote.php - line 33: OCA\DAV\Server->exec()
/var/www/owncloud/remote.php - line 162: require_once('/var/www/ownclo...')
{main}

Server configuration

nextcoud 12.0.3 wioth contact 2.0.1

Operating system: ubuntu 16.04 Web server: apache 2.4 Database: mariadb 10 PHP version: 7.0 Nextcloud version: (see Nextcloud admin page) 12.0.3 Contacts version: (see Nextcloud apps page) 2.0.1 Updated from an older Nextcloud or fresh install: updated Signing status: official via appstore

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/50039995-multiple-n-properties-are-forbidden-and-not-repaired?utm_campaign=plugin&utm_content=tracker%2F46751899&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F46751899&utm_medium=issues&utm_source=github).
irgendwie commented 7 years ago

The problem here is not the moving of a contact but rather that we don't validate the N property. The N property is allowed max. once in a vcard. Could you open the database and export that specific contact? It should reside in the %prefix%_cards table.

I would like to see an example, such that we can see how we can fix the vcard. We can't simply drop one N property since it is user data and could be the wrong one to delete.

devz3r0 commented 7 years ago

Can I somehow anonymize that contact without tampering the problem-part?

irgendwie commented 7 years ago

Replace names/dates/... with general ones, without removing properties from the vcard. I want to see if we can drop one N property, or if both values need to be preserved/merged/etc.

devz3r0 commented 7 years ago

it looks like that contact got some how destroyed. I can't alter anything. But I can export it normally:

As I see it has double properties:

BEGIN:VCARD VERSION:3.0 TEL;TYPE=CELL,VOICE:+910000 TEL;TYPE=CELL,VOICE:+910001 FN: Name N:; Name;;; N:; Name;;; FN: Name PRODID:-//ownCloud//NONSGML Contacts 0.5.0.0//EN REV:2015-12-25T19:18:26+00:00 UID:41347906-0bf7-4fce-b295-751dc0bdc9d0 END:VCARD

Edit: the 2 telephone numbers are indeed different.

the dump from the DB is just one long string, so if you need that really, I might send it to one directly who handles that with discretion.

irgendwie commented 7 years ago

Two TEL properties are no problem, but the double FN and N properties are... @skjnldsv Maybe we should let the user decide which N property to keep (a selection of all N properties in the vcard) and/or allow a manual update, otherwise data loss through auto repair can't be avoided

skjnldsv commented 7 years ago

With a dialog?

irgendwie commented 7 years ago

Something like this (I need a mockup tool >__<):

img_20171004_160132

Edit: We should do this whenever we try to push a vcard (changes to the contact) and FN or N property is multiple times in the vcard Edit Edit: We can also add a text saying, that this isn't necessarily our fault but could have been done through any syncing client or application the user was using.

irgendwie commented 7 years ago

@devz3r0 I don't think we can hack this together very quickly, so for now please correct the corrupted vcards manually in the database by removing multiple properties 😞 Or you could simply export the broken contact, edit it with an editor of your choice, delete the old contact and reimport the corrected one. (<= safer way) Sorry for the inconveniences.

devz3r0 commented 7 years ago

no problem thats yet as far the only contact I found, which has that issue.

koalajoe23 commented 6 years ago

Can confirm this, my contacts have been managed by own/nextcloud since version 5 with multiple DAV-Clients. A lot of contacts have multiple "Detailed Info" sections and cannot be moved to another group or even edited in any way.