Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CardDAV: rewrite vCard 4.0 in CardDAV GET #4880

Merged
merged 2 commits into from
Apr 15, 2024
Merged

CardDAV: rewrite vCard 4.0 in CardDAV GET #4880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c178a49
vcard_support: make vcard_to_v4 code idempotent for v4.0 cards
rsto Apr 3, 2024
b469efc
http_carddav: rewrite vCard 4.0 in CardDAV GET
rsto Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cassandane/Cassandane/Cyrus/FastMail.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ use warnings;
use DateTime;
use JSON::XS;
use Net::CalDAVTalk 0.09;
use Net::CardDAVTalk 0.03;
use Net::CardDAVTalk 0.05;
use Net::CardDAVTalk::VCard;
use Mail::JMAPTalk 0.12;
use Data::Dumper;
use Storable 'dclone';
Expand Down
86 changes: 86 additions & 0 deletions cassandane/tiny-tests/FastMail/carddav_rewrite_v4card_on_get
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!perl
use Cassandane::Tiny;

sub test_carddav_rewrite_v4card_on_get
:needs_component_httpd :needs_dependency_icalvcard
{
my ($self) = @_;
my $CardDAV = $self->{carddav};

my $Id = $CardDAV->NewAddressBook('foo');
my $href = "$Id/test.vcf";
my $uid = "3b678b69-ca41-461e-b2c7-f96b9fe48d68";
my $image = "R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==";

# The UID and PHOTO property values of this version 4.0
# vCard are bogus. The UID property value should either
# be a valid URI or it should have the VALUE=TEXT
# parameter set. The PHOTO property value must be a URI
# but instead it uses version 3.0 encoding for embedding
# data.
#
# This test asserts that we accept such data but rewrite on GET.
#
# The alternatives are:
# 1. Reject on PUT. This is problematic, as we might have
# been the ones writing that bogus data in the first place:
# https://github.com/cyrusimap/cyrus-imapd/commit/b8a879ccf22d52d336662d506d3a14ddf341b60b
#
# 2. Rewrite on PUT. This looks to be the preferrable
# solution in the long run, but it will require us to
# check how clients deal with us rewriting the UID value
# on PUT.

my $card = <<EOF;
BEGIN:VCARD
VERSION:4.0
UID:$uid
PHOTO;ENCODING=b;TYPE=GIF:$image
N:Gump;Forrest;;Mr.
FN:Forrest Gump
REV:2008-04-24T19:52:43Z
END:VCARD
EOF

my %Headers = (
'Content-Type' => 'text/vcard; version=4.0',
'Authorization' => $CardDAV->auth_header(),
);

xlog $self, "PUT vCard v4 with v3 values";
my $Response = $CardDAV->{ua}->request('PUT', $CardDAV->request_url($href), {
content => $card,
headers => \%Headers,
});
$self->assert_num_equals(201, $Response->{status});
$self->assert_not_null($Response->{headers}{etag});

xlog $self, "GET as vCard v4";
my $response = $CardDAV->Request('GET', $href, '',
'Accept' => 'text/vcard; version=4.0');
my $newcard = $response->{content};
$newcard =~ s/\r?\n[ \t]+//gs; # unfold long properties
$self->assert_matches(qr/PHOTO:data:image\/gif;base64,$image/, $newcard);

xlog $self, "GET as vCard v3";
$response = $CardDAV->Request('GET', $href, '',
'Accept' => 'text/vcard; version=3.0');
$newcard = $response->{content};
$newcard =~ s/\r?\n[ \t]+//gs; # unfold long properties
$self->assert_matches(qr/PHOTO;ENCODING=[bB];TYPE=GIF:$image/, $newcard);

xlog $self, "GET without explicit version in Accept header";
$response = $CardDAV->Request('GET', $href, '',
'Accept' => 'text/vcard');
$newcard = $response->{content};
$newcard =~ s/\r?\n[ \t]+//gs; # unfold long properties
$self->assert_matches(qr/VERSION:3.0/, $newcard);
$self->assert_matches(qr/PHOTO;ENCODING=[bB];TYPE=GIF:$image/, $newcard);

xlog $self, "GET without Accept header";
$response = $CardDAV->Request('GET', $href, '');
$newcard = $response->{content};
$newcard =~ s/\r?\n[ \t]+//gs; # unfold long properties
$self->assert_matches(qr/VERSION:3.0/, $newcard);
$self->assert_matches(qr/PHOTO;ENCODING=[bB];TYPE=GIF:$image/, $newcard);
}
12 changes: 6 additions & 6 deletions imap/http_carddav.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ static int export_addressbook(struct transaction_t *txn,
unsigned version =
(vcardcomponent_get_version(vcard) == VCARD_VERSION_40) ? 4 : 3;

if (version != want_ver) {
if (version != want_ver || want_ver == 4) {
if (want_ver == 4) vcard_to_v4_x(vcard);
else vcard_to_v3_x(vcard);
}
Expand Down Expand Up @@ -979,7 +979,7 @@ static int export_addressbook(struct transaction_t *txn,
vparse_get_entry(vcard, NULL, "version");
unsigned version = (ventry && ventry->v.value[0] == '4') ? 4 : 3;

if (version != want_ver) {
if (version != want_ver || want_ver == 4) {
if (want_ver == 4) vcard_to_v4(vcard);
else vcard_to_v3(vcard);
}
Expand Down Expand Up @@ -1336,7 +1336,7 @@ static int carddav_get(struct transaction_t *txn, struct mailbox *mailbox,
struct carddav_data *cdata = (struct carddav_data *) data;
unsigned want_ver = (mime && mime->version[0] == '4') ? 4 : 3;

if (cdata->version != want_ver) {
if (cdata->version != want_ver || want_ver == 4) {
/* Translate between vCard versions */
*obj = record_to_vcard_x(mailbox, record);
if (want_ver == 4) vcard_to_v4_x(*obj);
Expand Down Expand Up @@ -1726,7 +1726,7 @@ static int carddav_get(struct transaction_t *txn, struct mailbox *mailbox,
struct carddav_data *cdata = (struct carddav_data *) data;
unsigned want_ver = (mime && mime->version[0] == '4') ? 4 : 3;

if (cdata->version != want_ver) {
if (cdata->version != want_ver || want_ver == 4) {
/* Translate between vCard versions */
*obj = record_to_vcard(mailbox, record);
if (want_ver == 4) vcard_to_v4(*obj);
Expand Down Expand Up @@ -2224,7 +2224,7 @@ static int propfind_addrdata(const xmlChar *name, xmlNsPtr ns,

want_ver = (out_type->version[0] == '4') ? 4 : 3;

if (cdata->version != want_ver) {
if (cdata->version != want_ver || want_ver == 4) {
/* Translate between vCard versions */
vcard = fctx->obj;

Expand Down Expand Up @@ -2334,7 +2334,7 @@ static int propfind_addrdata(const xmlChar *name, xmlNsPtr ns,

want_ver = (out_type->version[0] == '4') ? 4 : 3;

if (cdata->version != want_ver) {
if (cdata->version != want_ver || want_ver == 4) {
/* Translate between vCard versions */
vcard = fctx->obj;

Expand Down
49 changes: 42 additions & 7 deletions imap/vcard_support.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include <config.h>
#include <libxml/parser.h>
#include <libxml/tree.h>
#include <libxml/uri.h>

#include "vcard_support.h"
#include "syslog.h"
Expand Down Expand Up @@ -362,11 +363,28 @@ EXPORTED void vcard_to_v3(struct vparse_card *vcard)
buf_free(&buf);
}

static int vcard_value_is_uri(const char *val)
rsto marked this conversation as resolved.
Show resolved Hide resolved
{
if (!val) return 0;
xmlURIPtr xuri = xmlParseURI(val);
int is_uri = xuri && xuri->scheme;
xmlFreeURI(xuri);
return is_uri;
}

EXPORTED void vcard_to_v4(struct vparse_card *vcard)
{
struct vparse_entry *ventry, *next;
struct vparse_param *vparam;
struct buf buf = BUF_INITIALIZER;
int is_v4 = 0;

ventry = vparse_get_entry(vcard->objects, NULL, "VERSION");
if (ventry) {
char *val = vparse_get_value(ventry);
is_v4 = !strcmpsafe("4.0", val);
free(val);
}

for (ventry = vcard->objects->properties; ventry; ventry = next) {
const char *name = ventry->name;
Expand All @@ -377,7 +395,7 @@ EXPORTED void vcard_to_v4(struct vparse_card *vcard)
if (!name) continue;
if (!propval) continue;

if (!strcasecmp(name, "version")) {
if (!is_v4 && !strcasecmp(name, "version")) {
/* Set proper VERSION */
vparse_set_value(ventry, "4.0");
}
Expand All @@ -388,7 +406,7 @@ EXPORTED void vcard_to_v4(struct vparse_card *vcard)
/* uri -> uri (default) */
vparse_delete_params(ventry, "value");
}
else if (strncmp(propval, "urn:uuid:", 9)) {
else if (!is_v4 && strncmp(propval, "urn:uuid:", 9)) {
/* text (default) -> uuid URN */
buf_setcstr(&buf, "urn:uuid:");
buf_appendcstr(&buf, propval);
Expand All @@ -401,7 +419,12 @@ EXPORTED void vcard_to_v4(struct vparse_card *vcard)
!strcasecmp(name, "sound")) {
/* Rewrite KEY, LOGO, PHOTO, SOUND properties */
vparam = vparse_get_param(ventry, "value");
if (!vparam || strcasecmp(vparam->value, "uri")) {

if ((!vparam && !is_v4) ||
(vparam && strcasecmp(vparam->value, "uri")) ||
vparse_get_param(ventry, "encoding") ||
vparse_get_param(ventry, "type") ||
!vcard_value_is_uri(propval)) {
/* Rewrite 'b' encoded value as data: URI */
buf_setcstr(&buf, "data:");

Expand Down Expand Up @@ -698,6 +721,10 @@ EXPORTED void vcard_to_v4_x(vcardcomponent *vcard)
{
vcardproperty *prop, *next;
struct buf buf = BUF_INITIALIZER;
int is_v4 = 0;

prop = vcardcomponent_get_first_property(vcard, VCARD_VERSION_PROPERTY);
is_v4 = prop && vcardproperty_get_version(prop) == VCARD_VERSION_40;

for (prop = vcardcomponent_get_first_property(vcard, VCARD_ANY_PROPERTY);
prop; prop = next) {
Expand All @@ -709,8 +736,10 @@ EXPORTED void vcard_to_v4_x(vcardcomponent *vcard)

switch (vcardproperty_isa(prop)) {
case VCARD_VERSION_PROPERTY:
/* Set proper VERSION */
vcardproperty_set_version(prop, VCARD_VERSION_40);
if (!is_v4) {
/* Set proper VERSION */
vcardproperty_set_version(prop, VCARD_VERSION_40);
}
break;

case VCARD_UID_PROPERTY:
Expand All @@ -721,7 +750,7 @@ EXPORTED void vcard_to_v4_x(vcardcomponent *vcard)
/* uri -> uri (default) */
vcardproperty_remove_parameter_by_ref(prop, param);
}
else if (strncmp(propval, "urn:uuid:", 9)) {
else if (!is_v4 && strncmp(propval, "urn:uuid:", 9)) {
/* text (default) -> uuid URN */
buf_setcstr(&buf, "urn:uuid:");
buf_appendcstr(&buf, propval);
Expand All @@ -746,7 +775,13 @@ EXPORTED void vcard_to_v4_x(vcardcomponent *vcard)

param = vcardproperty_get_first_parameter(prop,
VCARD_VALUE_PARAMETER);
if (!param || vcardparameter_get_value(param) != VCARD_VALUE_URI) {
if ((!param && !is_v4) ||
(param && vcardparameter_get_value(param) != VCARD_VALUE_URI) ||
vcardproperty_get_first_parameter(prop,
VCARD_ENCODING_PARAMETER) ||
vcardproperty_get_first_parameter(prop, VCARD_TYPE_PARAMETER) ||
!vcard_value_is_uri(vcardproperty_get_value_as_string(prop))) {

/* Rewrite 'b' encoded value as data: URI */
buf_setcstr(&buf, "data:");

Expand Down
Loading