search

nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.67k stars 4k forks source link

Pick vCard version by url parameters #16942

Open MetaEntropy opened 5 years ago

MetaEntropy commented 5 years ago

vcard-defaults-to-v4.patch.txt

Describe the bug

When downloading a VCF file from the CardDAV with the HTTP GET verb from a common web browser (Firefox, Chrome, Internet Explorer) one would expect to

get a version 4.0 vCard, which is the best format supported by the CardDAV; nextcloud returns a version 3.0 vCard, downgrading the content.

This is due to the fact that most browsers do not explicitly add "text/vcard; version=4.0" on their "Accept" HTTP header line; in place they use /. Consequently, the negotiateVCard() function called by httpAfterGet() in the 3rdparty/sabre/dav/lib/CardDAV/Plugin.php script negotiates the content

type as "text/vcard" then the text/vcard content type defaults to version 3.0.

The same behavior is observed for a "Accept: text/vcard" header. To get a version 4.0 vCard, one has to set "Accept: text/vcard; version=4.0" explicitly in the header of the HTTP GET request.

Actually, RFC 6350, in section 10.1, defines the "text/vcard" type but does not tell that it defaults to version 3.0.

RFC 6350 section 10.1 contains the following paragraph

Interoperability considerations: The text/vcard media type is intended to identify vCard data of any version. There are older specifications of vCard [RFC2426][vCard21] still in common use. While these formats are similar, they are not strictly compatible. In general, it is necessary to inspect the value of the VERSION property (see Section 6.7.9) for identifying the standard to which a given vCard object conforms.

It seems that defaulting to version 4.0 would be better since most tools work with version 4.0 nowadays.

To Reproduce (easy way) Steps to reproduce the behavior (way 1):

  1. Login to nextcloud as a user on a common browser (tested on: Firefox, Internet Explorer, Chrome)
  2. Click on the "Contacts" application
  3. Click on any contact in order to open it (or create one if none exists)
  4. Click on the menu (three dots icon) of the contact
  5. Click "Download"
  6. In the browser, Save the file on the disk
  7. Open the file with a text editor
  8. See that the "VERSION" field of the contact is 3.0 while it should be 4.0

To Reproduce (complex way, but easier to debug)

  1. Reproduce steps 1 to 5 of the easy way

  2. copy the URL such as http://example.com/owncloud/remote.php/dav/addressbooks/users/john/contacts/7a77045a-ff45-44d6-8f46-5294d600269a.vcf

  3. Use the curl command: curl --basic --user XXXXXX:XXXXX -X GET -H 'Accept: /' http://example.com/owncloud/remote.php/dav/addressbooks/users/john/contacts/7a77045a-ff45-44d6-8f46-5294d600269a.vcf

  4. Read the output

  5. See that the "VERSION" field of the contact is 3.0 while it should be 4.0

Expected behavior A version 4.0 file is expected with a field "VERSION:4.0"

Actual behavior A version 3.0 file is observed with a field "VERSION:3.0"

Comments The attached patch changes the behavior of HTTP GET requests on vCards. It does not affect REPORT requests since these requests use the negotiateVCard function

but first get the version from an XML attribute in the HTTP request body (defaulting to version 3.0).

Client configuration

Firefox 68

Windows 7 (6.1.7601 SP1)

CardDAV-clients: Nextcloud Contacts web interface (application for nextcloud) DAVx5 (backend) + builtin Contacts application in Android 4.4 (frontend) (does not really matter)

kesselb commented 5 years ago

Thanks for your research :+1:

Could you submit the patch upstream to https://github.com/sabre-io/dav? If they accept the patch we can backport it to https://github.com/nextcloud/3rdparty/tree/master/sabre/dav.

cc @georgehrke fyi

MetaEntropy commented 5 years ago

Thank you. This bug has been submitted upstream: https://github.com/sabre-io/dav/issues/1190

MetaEntropy commented 5 years ago

The upstream patch has not been accepted. Defaulting to version 3.0 is intentional. It makes sense for CardDAV clients. Indeed vCard clients are likely to declare support of version 4.0 of they do support it ; web browsers are different beasts with a generic Accept line. I would expect the Nextcloud Contacts application to provide a way to download a vCard 4.0 in a web browser. In my opinion, this is possible without breaking compatibility: generate an augmented URL with explicit vCard version specification for all VCF entries, bypassing HTTP Accept headers. The Nextcloud Contacts application would provide two links (v3 and v4) so that the end user can choose the format.

kesselb commented 5 years ago

Sounds like an enhancement for the contacts app then.

cc @skjnldsv I moved this here and changed the title.

skjnldsv commented 5 years ago

@kesselb the vcard implementation is on server, contacts only read the data from the carddav backend :thinking:

kesselb commented 5 years ago

Hmm :see_no_evil: If I understand @MetaEntropy correctly then the server already returns a vCard 4.0 if we request it (by Accept: text/vcard; version=4.0). I'm not sure how the contacts app request the vcards.

skjnldsv commented 5 years ago

We just let the user open the direct link to a new tab :p

kesselb commented 5 years ago

Oh. We need to move it back then? Is it a bug or enhancement?

generate an augmented URL with explicit vCard version specification for all VCF entries, bypassing HTTP Accept headers.

This needs to be done for the carddav server?

skjnldsv commented 5 years ago

No idea, on contacts the download button is just a link to the /remote.php/dav/users/xxx/addressbook/filename.vcf

MetaEntropy commented 5 years ago

This becomes a feature request. It requires modifications of both the nextcloud server and the Nextcloud Contacts application.

kesselb commented 5 years ago

https://github.com/nextcloud/3rdparty/blob/e724ba5a7af5db92c5b7d9e021d4e3c9ef1d32e2/sabre/dav/lib/CardDAV/Plugin.php#L805-L842

Hmm. The only way to change the vcard version is the accept header.

MetaEntropy commented 5 years ago

Please, find attached patches that implements the requested feature.

The nextcloud server patch applies to version 16.0.4 nextcloud-server-vcard-explicit-version-export-support.patch.txt

The contacts application patch applies to the 31 august 2019 master branch (commit c83a813b8412aa008dcbe04da150fc60dc6fd89c) nextcloud-contacts-vcard-explicit-version-export-support.patch.txt

The nextcloud server patch adds an URL query parameter version=(2.1|3.0|4.0|native) to the HTTP GET request on a vCard

The contacts application patch replaces the "Download" link by a "Download (vCard 3.0)" and "Download (vCard 4.0)" links. Warning: an automatic translation has been applied for the 72 language files. The strings "(vCard 3.0)" and "(vCard 4.0)" have been appended to the translation of the "Download" word. This may not make sense for some obscure languages.

skjnldsv commented 4 years ago

Wow, I'm late to thsi party. @kesselb shall we change the default so we don't rely on the header?

Btw original pr is 6 years old https://github.com/sabre-io/dav/pull/505

skjnldsv commented 4 years ago

Related/duplicate https://github.com/nextcloud/contacts/issues/4038

kesselb commented 4 years ago

Wow, I'm late to thsi party. @kesselb shall we change the default so we don't rely on the header?

Btw original pr is 6 years old https://github.com/sabre-io/dav/pull/505

The current behavior looks fine to me. A download option for contacts app would be nice. I guess it should be possible to send a xhr request with the accept header v4 and offer the content to the browser.

skjnldsv commented 4 years ago

The current behavior looks fine to me. A download option for contacts app would be nice. I guess it should be possible to send a xhr request with the accept header v4 and offer the content to the browser.

It feels weird that the default when requesting a resource is not the original resource itself 🤔