jmap_contact: encode vCard v4.0 blobs as data: URI

This fixes a bug where setting an avatar with JMAP Contact/set on an existing vCard 4.0 card resulted in the vCard having a b-encoded PHOTO value. But PHOTO must have a data: URI value in vCard 4.0 Signed-off-by: Robert Stepanek <rsto@fastmailteam.com>
rsto · Apr 2, 2024 · b8a879c · b8a879c
1 parent fc2a1f7
commit b8a879c
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 14 deletions.
diff --git a/cassandane/Cassandane/Cyrus/JMAPContacts.pm b/cassandane/Cassandane/Cyrus/JMAPContacts.pm
@@ -49,10 +49,12 @@ use Data::Dumper;
 use Storable 'dclone';
 use File::Basename;
 use File::Copy;
+use Cwd qw(abs_path getcwd);
 
 use lib '.';
 use base qw(Cassandane::Cyrus::TestCase);
 use Cassandane::Util::Log;
+use Cassandane::Util::Slurp;
 
 use charnames ':full';
 

diff --git a/cassandane/tiny-tests/JMAPContacts/contact_set_avatar_v4 b/cassandane/tiny-tests/JMAPContacts/contact_set_avatar_v4
@@ -0,0 +1,67 @@
+#!perl
+use Cassandane::Tiny;
+
+sub test_contact_set_avatar_v4
+    :needs_component_jmap
+{
+    my ($self) = @_;
+    my $jmap = $self->{jmap};
+    my $carddav = $self->{carddav};
+
+    xlog $self, "Create a v4 vCard over CardDAV";
+    my $id = '816ad14a-f9ef-43a8-9039-b57bf321de1f';
+    my $href = "Default/$id.vcf";
+    my $card = <<EOF;
+BEGIN:VCARD
+VERSION:4.0
+PRODID:+//IDN bitfire.at//DAVx5/4.2.0.3-gplay ez-vcard/0.11.3
+UID:$id
+FN:Foo
+N:;Foo;;;
+REV:20220504T040120Z
+END:VCARD
+EOF
+    $card =~ s/\r?\n/\r\n/gs;
+
+    $carddav->Request('PUT', $href, $card, 'Content-Type' => 'text/vcard');
+
+    xlog $self, "Get JMAP Contact";
+    my $res = $jmap->CallMethods([
+        ['Contact/get', {
+            properties => ['avatar', 'x-hasPhoto'],
+        }, 'R1']
+    ]);
+    my $contactId = $res->[0][1]{list}[0]{id};
+    $self->assert_not_null($contactId);
+    $self->assert_null($res->[0][1]{list}[0]{avatar});
+
+    xlog $self, "Set avatar on contact";
+    my $binary = slurp_file(abs_path('data/logo.gif'));
+    my $data = $jmap->Upload($binary, "image/gif");
+    $res = $jmap->CallMethods([
+        ['Contact/set', {
+            update => {
+                $contactId => {
+                    avatar => {
+                        blobId => $data->{blobId},
+                        type => "image/gif",
+                    }
+                }
+            }
+        }, 'R1']
+    ]);
+    my $avatarBlobId = $res->[0][1]{updated}{$contactId}{avatar}{blobId};
+    $self->assert_not_null($avatarBlobId);
+
+    xlog $self, "Get vCard over CardDAV as version 4.0";
+    $res = $carddav->Request('GET', $href, undef,
+        "Accept" => "text/vcard; version=4.0");
+    my $vcard = Net::CardDAVTalk::VCard->new_fromstring($res->{content});
+    my $photo = $vcard->{properties}->{photo}->[0] // undef;
+    $self->assert(not $photo->{binary});
+    $self->assert_equals("data:image/gif;base64,", substr($photo->{value}, 0, 22));
+
+    xlog $self, "Assert avatar blob contents";
+    $data = $jmap->Download('cassandane', $avatarBlobId);
+    $self->assert($binary eq $data->{content});
+}
diff --git a/imap/jmap_contact.c b/imap/jmap_contact.c
@@ -3565,30 +3565,48 @@ static int _blob_to_card(struct jmap_req *req,
         goto done;
     }
 
+    /* (Re)write vCard property */
+    vparse_delete_entries(card, NULL, prop);
+
     base = buf_base(&ctx.blob);
     len = buf_len(&ctx.blob);
 
     /* Pre-flight base64 encoder to determine length */
     size_t len64 = 0;
     charset_b64encode_mimebody(NULL, len, NULL, &len64, NULL, 0 /* no wrap */);
 
-    /* Now encode the blob */
-    encbuf = xzmalloc(len64+1);
-    charset_b64encode_mimebody(base, len, encbuf, &len64, NULL, 0 /* no wrap */);
-    base = encbuf;
-
-    /* (Re)write vCard property */
-    vparse_delete_entries(card, NULL, prop);
-
-    struct vparse_entry *entry = vparse_add_entry(card, NULL, prop, base);
+    /* Now create the property */
+    char *version = vparse_get_value(vparse_get_entry(card, NULL, "VERSION"));
+    if (!strcmpsafe("4.0", version)) {
+        // Create version 4.0 data: URI property
+        buf_setcstr(&buf, "data:");
+        if (buf_len(&ctx.content_type)) {
+            buf_append(&buf, &ctx.content_type);
+        }
+        buf_appendcstr(&buf, ";base64,");
+        encbuf = xzmalloc(buf_len(&buf) + len64 + 1);
+        memcpy(encbuf, buf_base(&buf), buf_len(&buf));
+        charset_b64encode_mimebody(
+            base, len, encbuf + buf_len(&buf), &len64, NULL, 0 /* no wrap */);
+        vparse_add_entry(card, NULL, prop, encbuf);
+    }
+    else {
+        // Create version 3.0 binary property
+        encbuf = xzmalloc(len64 + 1);
+        charset_b64encode_mimebody(
+            base, len, encbuf, &len64, NULL, 0 /* no wrap */);
 
-    vparse_add_param(entry, "ENCODING", "b");
+        struct vparse_entry *entry = vparse_add_entry(card, NULL, prop, encbuf);
+        vparse_add_param(entry, "ENCODING", "b");
 
-    if (buf_len(&ctx.content_type)) {
-        char *subtype = xstrdupnull(strchr(buf_cstring(&ctx.content_type), '/'));
-        vparse_add_param(entry, "TYPE", ucase(subtype+1));
-        free(subtype);
+        if (buf_len(&ctx.content_type)) {
+            char *subtype =
+                xstrdupnull(strchr(buf_cstring(&ctx.content_type), '/'));
+            vparse_add_param(entry, "TYPE", ucase(subtype + 1));
+            free(subtype);
+        }
     }
+    xzfree(version);
 
     /* Add this blob to our list */
     property_blob_t *blob = property_blob_new(key, prop,