Page MenuHomePhorge

VCard Photo Export does not meet RFC 2426 specification - suggest fix in iRony
Closed, ResolvedPublic

Description

I noticed, that Thunderbird is not syncing photos from Kolab-Server.

rpm -qv pykolab
pykolab-0.8.0-4.2.el7.kolab_16.noarch

Thunderbird / Sogo-Connector needs the optional TYPE-Field to accept the photo. The current code does this for v4 only, but v3 does need it too. So I suggest the following change:

@@ -821,10 +821,8 @@ class ContactsBackend extends CardDAV\Backend\AbstractBackend

         if (!empty($contact['photo'])) {
             $vc->PHOTO = $contact['photo'];
-            $vc->PHOTO['ENCODING'] = 'B';
-            if ($v4) {
-                $vc->PHOTO['TYPE'] = strtoupper(substr(rcube_mime::image_content_type($contact['photo']), 6));
-            }
+            $vc->PHOTO['ENCODING'] = 'b';
+            $vc->PHOTO['TYPE'] = strtoupper(substr(rcube_mime::image_content_type($contact['photo']), 6));
         }

         // add custom properties

The lower case b in the encoding is according RFC 2426 Section 3.1.4.

I tried to implement the v4 format also, but this seams to trigger bigger changes in Sabre to support prepending string 'base64' in the Serialising, if (and only if) it is a version 4 vcard.

Details

Ticket Type
Task

Event Timeline

machniak added a project: iRony.
machniak subscribed.

As this contradicts with rId20afc78222279, would be nice to have feedback from Thomas.

I don't recall where that change came from and I can't find an according test/sample case. We should comply with RCF here.

Perhaps it has come from section 2.4.1?

The question here is what the Apple Mac OS X / iPhone / iPad / ... native address book do, since this saturates the majority of the client applications.

Perhaps it has come from section 2.4.1?

Good find. But this references the "B" Encoding in RFC 2047 Section 4.1.

The "B" encoding is identical to the "BASE64" encoding defined by RFC 2045.

So this is only the name of the encoding. Not the literal char used to define the encoding.

You find the lower letter "b" as litteral char in

  • The Grammer in Section 4 of RFC 2426 - not only for img-inline-param, but also for snd-inline-param, key-inline-paarm
img-inline-param     = ("VALUE" "=" "binary")
                  / ("ENCODING" "=" "b")
                  / ("TYPE" "=" param-value
  ;TYPE value MUST be an IANA registered image type
  • In the Secition 2.4.1 there is a reference to MIME-DIR, which is RFC 2425. In Section 5.8.3 of RFC 2425 the encoding is also defined as:

...

encodingparm = "encoding" "=" encodingtype

encodingtype = "b"       ; from RFC 2047

...

So the "B" Format is the base64 which ist declared with a lower letter "b".
Surely not the best way to do it, but RFC way is "b".

The question here is what the Apple Mac OS X / iPhone / iPad / ... native address book do, since this saturates the majority of the client applications.

I agree, if most clients expect a "B" we should keep it so. It would also be consistent with the (Sabre?) Export in the Kolab-Roundcube-Adressbook. It exports also "ENCODING=B".

If you prefer keeping the big letter B, could you please add the rest of the patch to the code, so thunderbird can sync images? Or should I create a differnt patch for that, so you can close this one?

"B" and "b" both work with iOS7 Contacts app.

"B" and "b" both work with iOS7 Contacts app.

Let's use the lower-case 'b' then, since it seems the better compatible option. Please add commentary in the code to reflect the information in this ticket.

Can we make a test for this to more explicitly verify we continue to communicate a lower-case 'b' as well?

Yes, Apple clients also use "b" when exporting and synching contact photos although they can handle upper-cased values as well. But the way to go definitely is lower-cased.

FWIW: the contact export in Roundcube doesn't use the same codebase and should be adjusted accordingly.

Also, adding the TYPE attribute in v3 seems a reasonable change to me.

I recently run iRony's tests on Kolab Winterfell, and it gave me 14 failed tests. It looks like we don't really use these for CI, are we?

Anyway, it looks like pycalendar also does not use ENCODING=b nor TYPE specified. At least tests fail on PHOTO element. There's too much magic in this testing suite. I'll likely create some simple unittest for this ticket.

All the tests which are in the iRony repository used to run through without errors. So if that changed on Winterfell, it should be looked at.

Anyway, those are functional tests and what we want for this ticket is more a phpunit test case. The calendaring part is already covered in rRPK with the unit tests from the libcalendaring plugin.

Is this related to why setting a picture for a contact in Android fails when syncing with DAVdroid? Should another bug be filed for that?