Nextcloud serves VCard in version 3.0 even if the source is 4.0

schlagi123 commented 5 years ago

Describe the bug

I have problems with the importing of VCards over the web-interface. I import the following VCard:

 BEGIN:VCARD
 VERSION:4.0
 FN:A4
 N:A4;;;;
 ANNIVERSARY:20181206
 END:VCARD

The anniversary is not shown in the web-interface. After exporting the same VCard I get the following VCard:

BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.1.2//EN
FN:A4
N:A4;;;;
ITEM1.X-ABDATE;VALUE=DATE-AND-OR-TIME:20181206
ITEM1.X-ABLABEL:_$!<Anniversary>!$_
X-ANNIVERSARY;VALUE=DATE-AND-OR-TIME:20181206
UID:cf9b96df-b1ad-4744-9854-b16a9a565efa
END:VCARD

The exported VCard is in version 3.0. Do you not support VCard version 4.0?

After that, I test to importing the same VCard, but with the version 3.0:

 BEGIN:VCARD
 VERSION:3.0
 FN:A3
 N:A3;;;;
 ANNIVERSARY:20181206
 END:VCARD

The anniversary is shown on the web-interface, and also present in the export:

BEGIN:VCARD
VERSION:3.0
FN:A3
N:A3;;;;
ANNIVERSARY:20181206
UID:567f7dd2-ecc2-4ada-943d-becbf351a2f0
END:VCARD

The confusing is, that VCard in 3.0 version do not support anniversary. So the import of 4.0 with anniversary not work as expected because the import must contain the anniversary tag. The import of 3.0 with anniversary should not contain anniversary, but it does. At last I habe expected, that the exported VCard is in the 4.0 version.

Server configuration

Operating system: Ubuntu 18.04 LTS Web server: Apache Database: MariaDB PHP version: 7.2 Nextcloud version: 14.0.4 Contacts version: 2.1.8

Updated from an older Nextcloud or fresh install: Upgraded from 14.0.3 (this was the first installation).

Signing status: No errors have been found.

Client configuration

Browser: Firefox 63.0.3 (64-Bit)

Operating system: Windows 10

CardDAV-clients: Nextcloud Web-Interface

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/67114898-import-vcard-in-version-4-0-is-not-correct?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).

nextcloud-bot commented 5 years ago

GitMate.io thinks possibly related issues are https://github.com/nextcloud/contacts/issues/239 (From nextcloud exported vcard files can't be imported correctly), https://github.com/nextcloud/contacts/issues/389 (Import of Outlook VCARD failing), https://github.com/nextcloud/contacts/issues/433 (vCard import broken: window.localStorage is null), https://github.com/nextcloud/contacts/issues/55 (Allow vCard importing into custom AddressBook), and https://github.com/nextcloud/contacts/issues/682 (Respect vCard UID on Contacts Import).

skjnldsv commented 5 years ago

Hello @schlagi123 Do you have any external sync with your setup? Android, Iphone, outlook, thunderbird? We will never change the VERSION fields and the properties you see like ITEM1.X-ABDATE are not supported anywhere on nextcloud, so I really doubt we added this. :)

schlagi123 commented 5 years ago

I use external Sync, but for this test I have disabled all synctools to minimize the problem vector. In my testcase of this issue, I only use the nextcloud web interface.

skjnldsv commented 5 years ago

@schlagi123 Can you make sure of that please? Or try by creating a new addressbook that is not linked to any of your device, import the contact and download it again.

schlagi123 commented 5 years ago

I create a new fresh addressbook and import to this. I can see the same effects.

The question is now: is it a problem in the UI, in the PHP code in Nextcload or in the Sabre library?

skjnldsv commented 5 years ago

Let me try then. :)

skjnldsv commented 5 years ago

I can indeed reproduce. The data in the database does not get changed, This happens on the download. Moving to nextcloud/server.

schlagi123 commented 5 years ago

I find the code, who converts the VCard wrong in the Sabre library. In the file 3rdpary/sabre/dav/lib/CardDAV/Plugin.php in the addressbookMultiGetReport method.

In this method the negotiateVCard method (line 245) is called and return, that the requested VCard sould be in Version 3.0.
After that, the method convertVCard (line 258) is called and convert the VCard from Version 4.0 to Versoin 3.0. Because ANNIVERSARY is only in Versoin 4.0 supported, ITEM1.X-ABDATE are created.

If nextcloud supports VCard 4.0 this conversion must be avoided.

Sorry, currently I habe no more time to investigate to this issue.

choreutes commented 5 years ago

I just hit this issue as well. I moved my contacts to a fresh installation of NextCloud via the CardBook plugin for Thunderbird. Some of them contain ANNIVERSARY and/or GENDER information only supported in vCard 4.0. In the web interface everything seems fine, both anniversary and gender are displayed as expected. But when I download the vCard of such a contact or sync my contacts to another client all contacts are indeed converted to vCard 3.0.

I will try to find out about the state of the database in the next few days and post my results if needed. It would be quite helpful if this could be fixed in NextCloud 16. Sadly I do not have any PHP programming experience to help write a fix myself :(

spoorun commented 5 years ago

1) This bug affects contacts created in Nextcloud not just imported contacts. Please update the title to reflect this, to something like: vCard 4 contacts incorrectly saved as vCard 3

2) This bug is fairly crucial for smartphone and desktop app users, as it can result in data loss (since the server is holding contacs with some vCard 4 data, but declaring it as vCard 3.

It was scheduled for 16 release. Is it possible to move it back forward?

@skjnldsv

skjnldsv commented 5 years ago

@olantrust Milestones are not definitive, those are thing we want to work on. If we create a fix, it can be backported later from 17 to 16 (or from 18, 19, 20.. if no one fix it until then) :)

choreutes commented 4 years ago

I finally found some time to examine this bug a little closer, and I am fairly certain now that this is, in fact, not a server issue. At least not the way I thought before.

I was at a loss for quite some time about how to debug this issue because my experience with PHP is almost non-existent and NextCloud is a relatively large project. But from what I saw in the source code, especially the CardDAV part that @schlagi123 mentioned, everything seems to do what the RFC expects it to do. So I tried to “catch the bug in the act” by observing a sync of one of my CardDAV clients. But since most of them are plugins or apps of some kind, this was also not as easy as I would have liked.

However, all I really needed was the response to one single http request to my NextCloud server, namely a download query for a single vcard file. What I ended up doing was therefore to send that request manually via curl sticking as close to the RFC as I could. The resulting command looked like this:

curl -Lv \
--request REPORT \
--header DEPTH:1 \
--user $user \
--data '<c:addressbook-multiget xmlns:d="DAV:" xmlns:c="urn:ietf:params:xml:ns:carddav"><d:prop><d:getetag /><c:address-data content-type="text/vcard" version="4.0" /></d:prop><d:href>/remote.php/dav/addressbooks/users/$user/$addressbook/$contact.vcf</d:href></c:addressbook-multiget>' \
$url/remote.php/dav/addressbooks/users/$user/$addressbook

The response I got was indeed the contact I requested as a vcard 4.0! The crucial part in this request is the version="4.0" in the address-data tag. Leaving it out causes the server to default to vcard 3.0. The same result can be achieved without the surrounding XML by content negotiation through the Accept header of a GET request:

curl -Lv \
--request GET \
--header "Accept: text/vcard; version=4.0" \
--user $user \
$url/remote.php/dav/addressbooks/users/$user/$addressbook/$contact.vcf

My (sad) conclusion from all of this is that NextCloud is perfectly capable of serving CardDAV with vcard 4.0 and just about every client I have tried gets it wrong. This seems to also include the download option in the web interface. I have not looked for the the code which actually starts the download yet, because I do not use it too often. But I also think that vcard 3.0 might be a sane default. Maybe the user should be offered a choice before downloading the contact?

Sorry for this awfully long post! It just took me forever to puzzle all this together and so I tried to write it up as accurately as possible to save everybody else the trouble.

creopard commented 3 years ago

Here's another thing:

Importing a version 4.0 vcard via webgui won't display the base64 encoded photo.

BEGIN:VCARD VERSION:4.0 PRODID:-//Sabre//Sabre VObject 4.3.0//EN UID:urn:uuid:b86aa944-d999-46c6-b4a3-ed92fea8aee5 CATEGORIES:Friend FN:Test Name N:Name;Test;;; TEL;TYPE=CELL:+49 123456789 REV:20200826T090531Z PHOTO:DATA:IMAGE/--JPEG;BASE64,/9j/4AAQSkZJRgABAQAAAQABAAD/...

When I manually change the photo and export the vcard, the vcard is exported as version 3.0

BEGIN:VCARD VERSION:3.0 PRODID:-//Sabre//Sabre VObject 4.3.0//EN UID:urn:uuid:b86aa944-d999-46c6-b4a3-ed92fea8aee5 CATEGORIES:Friend FN:Test Name N:Name;Test;;; TEL;TYPE=CELL:+49 123456789 REV;VALUE=DATE-AND-OR-TIME:20201021T082026Z PHOTO;ENCODING=b;TYPE=JPEG:/9j/4AAQSkZJRgABAQAAAQABAAD/...

The base64-string of the photo is the same, just the initial PHOTO-tag is different...

skjnldsv commented 3 years ago

PHOTO:DATA:IMAGE/--JPEG;BASE64,/9j/4AAQSkZJRgABAQAAAQABAAD

This is definitely not valid. PHOTO:data:image/jpeg;base64,/9j/4AAQS is. (note the jpeg) Also, this is irrelevant to this issue, PHOTO;ENCODING=b;TYPE=JPEG: is valid vcard3 format :)