[#7407] Photos: storing in LDAP and broken contact sync
Summary Photos: storing in LDAP and broken contact sync
Queue Turba
Queue Version 2.3
Type Bug
State Resolved
Priority 2. Medium
Owners jan@horde.org
Requester xk3@mompl.org
Created 2008-09-29 (4307 days ago)
Due
Updated 2010-04-13 (3746 days ago)
Assigned 2008-10-24 (4282 days ago)
Resolved 2009-08-18 (3984 days ago)
Milestone
Patch Yes

Comments
xk3@mompl.org 2008-09-29 14:11:12
As follow up to closed #5971



Hi, just upgraded to current turba 2.3

and run into some problems with PHOTO



1) storing contact without a photo in LDAP



The attribute array always contains an array for the photo item. If

there is no photo set, this array is empty, but is not catched by the

function _emptyAttributeFilter in turba/lib/Driver/ldap.php. This

results in an error in _add to LDAP. Stripping empty arrays too solves

at least this problem.



      function _emptyAttributeFilter($var)

      {

          if (!is_array($var)) {

              return ($var != '');

          } else {

              if (count($var) == 0) { return false; }    <<===

              foreach ($var as $v) {

         ...



Storing in turba DB doesn't show this bug, but photos are stored as 0B

blobs instead of NULL blobs.







2) base64 is terminated by a blank line, so eliminate it before

     matching the following vCalender attributes. Otherwise the name of

     one following PHOTO will start with a \r\n, so it will be

     discarded afterward



     in /lib/Horde/iCalendar.php



          // Unfold any folded lines.

-       $vCal = preg_replace('/[\r\n]+[ \t]/', '', $vCal);

+       $vCal = preg_replace('/[\r\n]+[ \t]+/', '', $vCal);

+       $vCal = preg_replace('/\r\n\r\n/', "\r\n", $vCal);



     (First line only beautifies the base64 code by stripping all

     whitespace. The second line does the necessary work by deleting all

     blank lines.)



     and add the blank line in /turba/lib/Driver.php again before

     sending to mobile:



              case 'photo':

              case 'logo':

                  $params = array('ENCODING' => 'base64');

                  if (isset($hash[$key . 'type'])) {

                      $params['TYPE'] = $hash[$key . 'type'];

                  }

                  $vcard->setAttribute(String::upper($key),

-                                    base64_encode($val),

+                                    base64_encode($val) . "\r\n",

                                       $params);

                  break;







3) Browsing the address book with photo as a visible column results in a

     warning per entry containing a photo:



     Warning: htmlspecialchars() expects parameter 1 to be string, array

     given in

/var/www/org.afaik.duff_ssl/mail-new/turba/templates/browse/row.inc

     on line 90



         if ($ob->hasValue($this->columns[$c - 1])) {

          $value = $ob->getValue($this->columns[$c - 1]);

-        $shown_columns[$c] = htmlspecialchars($value)

+        $shown_columns[$c] = is_array($value) ? $value :

htmlspecialchars($value);

          if ($type == 'email') {



     No thinking on my side, just getting rid of this warning, so check

     again.







4) In toHash and tovCard, the encoding of a photo is tested against

     and set to 'b'. Is that a valid encoding name at all? Changed

     it to 'base64' and it worked again. In turba/lib/Driver.php



              case 'photo':

              case 'logo':

-               $params = array('ENCODING' => 'b');

+               $params = array('ENCODING' => 'base64');





                  if (!isset($item['params']['ENCODING']) ||

-                   String::lower($item['params']['ENCODING']) != 'b') {

+                   String::lower($item['params']['ENCODING']) != 'base64') {

                      // Invalid property.

                      break;





5) Finally, how am I supposed to delete a photo from a contact or save

     it to a local file (e.g. for doing some cropping stuff in an external

     tool)? There are no delete and save buttons as for a common file.





Having this patched, I can add (ldap), change, view and sync photos



       Martin



PS: any thoughts on adding crop support? Photos taken by mobiles have

a stupid format for contact photos. Otherwise, I could live with as

save as local file button.

robb@wtg.cw.com 2008-09-30 13:22:40
have test above changes, works for me



have created patches for convenience

Jan Schneider <jan@horde.org> 2008-10-23 22:24:50
Please provide a real world example vCard. These patches break all 
kind of stuff.

robb@wtg.cw.com 2008-10-24 09:59:21
> Please provide a real world example vCard. These patches break all

> kind of stuff.



as attached





CVS Commit <cvs@lists.horde.org> 2008-10-28 23:35:18
Changes have been made in CVS for this ticket:

http://cvs.horde.org/diff.php/turba/lib/Driver.php?r1=1.222&r2=1.223&ty=u

CVS Commit <cvs@lists.horde.org> 2008-10-28 23:36:04

Jan Schneider <jan@horde.org> 2008-10-29 00:28:00
> 2) base64 is terminated by a blank line, so eliminate it before

>     matching the following vCalender attributes. Otherwise the name of

>     one following PHOTO will start with a \r\n, so it will be

>     discarded afterward



This change is not necessary. The attribute is parsed fine with the 
extra blank line already.



> 4) In toHash and tovCard, the encoding of a photo is tested against

>     and set to 'b'. Is that a valid encoding name at all? Changed

>     it to 'base64' and it worked again. In turba/lib/Driver.php



It's actually the only valid value. BASE64 is not valid and breaking 
RFCs. We are reading it now anyway.



> 5) Finally, how am I supposed to delete a photo from a contact or save



Not at all, yet.



>     it to a local file (e.g. for doing some cropping stuff in an external

>     tool)? There are no delete and save buttons as for a common file.



Right mouse click.



> PS: any thoughts on adding crop support? Photos taken by mobiles have

> a stupid format for contact photos. Otherwise, I could live with as

> save as local file button.



That would be nice indeed, and would have to be added to 
Horde_Form_Type_image. Please file a separate enhancement request for 
that.

Michael Rubinsky <mrubinsk@horde.org> 2008-10-29 00:47:47
>> PS: any thoughts on adding crop support? Photos taken by mobiles have

>> a stupid format for contact photos. Otherwise, I could live with as

>> save as local file button.

>

> That would be nice indeed, and would have to be added to

> Horde_Form_Type_image. Please file a separate enhancement request for

> that.



FYI - there is crop support in Ansel's photo editor, so you can look 
at the code there possibly for implementation, or maybe even somehow 
time the editing into Ansel itself.

xk3@mompl.org 2009-03-22 14:44:23
     * Horde: 3.3.3

     * Turba: H3 (2.3.1)



I would like to add a comment on syncing photos with Nokia E51: data 
from the phone is parsed ok now, but syncing to the phone still needs 
some tweaking: I added the following function in 
./lib/SyncML/Device/Nokia.php



function convertServer2Client($content, $contentType, $database)

{

   list($content, $contentType) =

     parent::convertServer2Client($content, $contentType);



     $content = 
preg_replace('/(\r\n|\r|\n)PHOTO;ENCODING=b[^:]*:(.+?)(\r\n|\r|\n)/',

       '\1PHOTO;ENCODING=BASE64:\1\2\1\1',

       $content, 1);



     $GLOBALS['backend']->logFile(

       SYNCML_LOGFILE_DATA,

       "\noutput converted for client ($contentType):\n$content\n");

     return array($content, $contentType);

}



Reasons:

a) Nokia expects "BASE64" as encoding string, not "b" (which is base64)

b) Extra file type info confuses the phone, so strip everything else 
before ":"

c) base64 has to end with an extra blank line (the double \1\1)





Finally, the problem 1) of comment #1 of this ticket is still not 
solved. I still have to patch the function _emptyAttributeFilter.



cheers,

    Martin



xk3@mompl.org 2009-03-22 14:47:02


> Finally, the problem 1) of comment #1 of this ticket is still not

> solved. I still have to patch the function _emptyAttributeFilter.



I meant:  1) of the 1st comment of this ticket #7407

CVS Commit <cvs@lists.horde.org> 2009-08-18 17:00:10

CVS Commit <cvs@lists.horde.org> 2009-08-18 17:05:54

Jan Schneider <jan@horde.org> 2009-08-18 17:07:29
Committed your changes, thanks!

CVS Commit <cvs@lists.horde.org> 2010-01-13 00:10:38

asd@asdf.com 2010-04-13 09:23:33
> As follow up to closed #5971
>
>
>
> Hi, just upgraded to current turba 2.3
>
> and run into some problems with PHOTO
>
>
>
> 1) storing contact without a photo in LDAP
>
>
>
> The attribute array always contains an array for the photo item. If
>
> there is no photo set, this array is empty, but is not catched by the
>
> function _emptyAttributeFilter in turba/lib/Driver/ldap.php. This
>
> results in an error in _add to LDAP. Stripping empty arrays too solves
>
> at least this problem.
>
>
>
>      function _emptyAttributeFilter($var)
>
>      {
>
>          if (!is_array($var)) {
>
>              return ($var != '');
>
>          } else {
>
>              if (count($var) == 0) { return false; }    <<===
>
>              foreach ($var as $v) {
>
>         ...
>
>
>
> Storing in turba DB doesn't show this bug, but photos are stored as 0B
>
> blobs instead of NULL blobs.
>
>
>
>
>
>
>
> 2) base64 is terminated by a blank line, so eliminate it before
>
>     matching the following vCalender attributes. Otherwise the name of
>
>     one following PHOTO will start with a \r\n, so it will be
>
>     discarded afterward
>
>
>
>     in /lib/Horde/iCalendar.php
>
>
>
>          // Unfold any folded lines.
>
> -       $vCal = preg_replace('/[\r\n]+[ \t]/', '', $vCal);
>
> +       $vCal = preg_replace('/[\r\n]+[ \t]+/', '', $vCal);
>
> +       $vCal = preg_replace('/\r\n\r\n/', "\r\n", $vCal);
>
>
>
>     (First line only beautifies the base64 code by stripping all
>
>     whitespace. The second line does the necessary work by deleting all
>
>     blank lines.)
>
>
>
>     and add the blank line in /turba/lib/Driver.php again before
>
>     sending to mobile:
>
>
>
>              case 'photo':
>
>              case 'logo':
>
>                  $params = array('ENCODING' => 'base64');
>
>                  if (isset($hash[$key . 'type'])) {
>
>                      $params['TYPE'] = $hash[$key . 'type'];
>
>                  }
>
>                  $vcard->setAttribute(String::upper($key),
>
> -                                    base64_encode($val),
>
> +                                    base64_encode($val) . "\r\n",
>
>                                       $params);
>
>                  break;
>
>
>
>
>
>
>
> 3) Browsing the address book with photo as a visible column results in a
>
>     warning per entry containing a photo:
>
>
>
>     Warning: htmlspecialchars() expects parameter 1 to be string, array
>
>     given in
>
> /var/www/org.afaik.duff_ssl/mail-new/turba/templates/browse/row.inc
>
>     on line 90
>
>
>
>         if ($ob->hasValue($this->columns[$c - 1])) {
>
>          $value = $ob->getValue($this->columns[$c - 1]);
>
> -        $shown_columns[$c] = htmlspecialchars($value)
>
> +        $shown_columns[$c] = is_array($value) ? $value :
>
> htmlspecialchars($value);
>
>          if ($type == 'email') {
>
>
>
>     No thinking on my side, just getting rid of this warning, so check
>
>     again.
>
>
>
>
>
>
>
> 4) In toHash and tovCard, the encoding of a photo is tested against
>
>     and set to 'b'. Is that a valid encoding name at all? Changed
>
>     it to 'base64' and it worked again. In turba/lib/Driver.php
>
>
>
>              case 'photo':
>
>              case 'logo':
>
> -               $params = array('ENCODING' => 'b');
>
> +               $params = array('ENCODING' => 'base64');
>
>
>
>
>
>                  if (!isset($item['params']['ENCODING']) ||
>
> -                   String::lower($item['params']['ENCODING']) != 'b') {
>
> +                   String::lower($item['params']['ENCODING']) != 'base64') {
>
>                      // Invalid property.
>
>                      break;
>
>
>
>
>
> 5) Finally, how am I supposed to delete a photo from a contact or save
>
>     it to a local file (e.g. for doing some cropping stuff in an external
>
>     tool)? There are no delete and save buttons as for a common file.
>
>
>
>
>
> Having this patched, I can add (ldap), change, view and sync photos
>
>
>
>       Martin
>
>
>
> PS: any thoughts on adding crop support? Photos taken by mobiles have
>
> a stupid format for contact photos. Otherwise, I could live with as
>
> save as local file button.