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.

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

1) Crash on error during photo cropping, 2) exported VCard contains broken PHOTO property #345

Closed
thopico opened this issue Jun 14, 2021 · 11 comments
Assignees
Labels

Comments

@thopico
Copy link

thopico commented Jun 14, 2021

Bug description

Hi,
I am synchronizing an addressbook from Nextcloud where I have a contact with a photo.
Trying to access it through Roundcube Carddav interface results in a blank panel as in the photo.
rc-carddav
Comparing the vcard from NC and RC gives :

--- contact-from-nc.vcf	2021-06-14 10:27:06.260948963 +0200
+++ contact-from-rc.vcf	2021-06-14 10:28:03.900217469 +0200
@@ -1,15525 +1,13 @@
 BEGIN:VCARD
 VERSION:3.0
 N:;xxx;;;
 FN:xxx
-EMAIL;TYPE="INTERNET,HOME":xxx
-TEL;TYPE=CELL;VALUE=UNKNOWN:xxx
+BDAY;VALUE=date:xxx
-BDAY;VALUE=DATE:xxx
-PHOTO;ENCODING=b;TYPE=JPEG;X-ABCROP-RECTANGLE=ABClipRect_1&0&0&2634&2634&Tn
- onh/ZHSO5hTUxo8B2EwQ==;VALUE=BINARY:/9j/4AAQSkZJRgABAQAAAQABAAD/4ZSKRXhpZgA
- ASUkqAAgAAAALAA4BAgAgAAAAkgAAAA8BAgAFAAAAsgAAABABAgAKAAAAuAAAABIBAwABAAAAAQ
- ...
-REV;VALUE=DATE-AND-OR-TIME:20210614T081943Z
-EMAIL;TYPE=WORK:xxx
-EMAIL;TYPE=HOME:xxx
-END:VCARD
\ Pas de fin de ligne à la fin du fichier
+PHOTO;ENCODING=b:Q29udGFjdHMgKGh0dHBzOi8vY2xvdWQuZ25vdXMuZnIvcmVtb3RlLnBocC9
+ kYXYvYWRkcmVzc2Jvb2tzL3VzZXJzL3Rob21hcy9jb250YWN0cy8p
+EMAIL;TYPE=INTERNET;TYPE=OTHER:xxx
+EMAIL;TYPE=INTERNET;TYPE=WORK:xxx
+EMAIL;TYPE=INTERNET;TYPE=HOME:xxx
+TEL;TYPE=OTHER:xxx
+END:VCARD

As you can see the RC data fits in 2 lines, while for NC there are 15510 lines for data encoding...

Thank you!

Versions

Roundcube Webmail 1.4.11
carddav | v4.1.0

@mstilkerich
Copy link
Owner

Hello, can you please share the long entries generated when opening the contact? Check the error log and the carddav log.

@thopico
Copy link
Author

thopico commented Jun 14, 2021

I didn't find any carddav log but here are the generated log entries :

[Mon Jun 14 09:36:59.170030 2021] [php7:warn] [pid 122] [client 172.18.0.2:43118] PHP Warning:  imagecreatefromstring(): No JPEG support in this PHP build in /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php on line 270, referer: https://my.domain.tld/?_task=addressbook&_source=carddav_16&_page=2
[Mon Jun 14 09:36:59.170244 2021] [php7:error] [pid 122] [client 172.18.0.2:43118] PHP Fatal error:  Uncaught TypeError: imagesy() expects parameter 1 to be resource, bool given in /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php:272
  Stack trace:
    #0 /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php(272): imagesy(false)
    #1 /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php(133): MStilkerich\\CardDavAddressbook4Roundcube\\DelayedPhotoLoader->xabcropphoto('\\xFF\\xD8\\xFF\\xE0\\x00\\x10JFIF\\x00\\x01\\x01\\x00\\x00...', Object(Sabre\\VObject\\Parameter))
    #2 /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php(83): MStilkerich\\CardDavAddressbook4Roundcube\\DelayedPhotoLoader->computePhotoFromProperty()
    #3 [internal function]: MStilkerich\\CardDavAddressbook4Roundcube\\DelayedPhotoLoader->__toString()
    #4 /var/www/html/program/steps/addressbook/func.inc(946): preg_match('!^https?://!i', Object(MStilkerich\\CardDavAddressbook4Roundcube\\DelayedPhotoLoader))
    #5 /var/www/html/program/include/rcmail_output_html.php(1359): rcmail_contact_photo(Array)
    #6 [internal function]: rcmail_output_html->xml_command(Array)
  # in /var/www/html/plugins/carddav/src/DelayedPhotoLoader.php on line 272, referer: https://my.domain.tld/?_task=addressbook&_source=carddav_16&_page=2

@mstilkerich
Copy link
Owner

Thanks for the report. It's actually two issues you found there 👍

  1. I did consider the case that PHP has no support for libgd (a graphics library), but not that libgd is available but lacking support for a picture format (here: jpeg). Your photo property contains a big photo of which only a small smart is supposed to be shown (a crop area). E.g. the photo contains an entire person but the crop area defines where the face of the person is in the photo. In this case. rcmcarddav tries to crop the photo for roundcube, because roundcube does not support this.

  2. In the roundcube export of the vcard, the photo is somehow broken (it is a base64 encoding of the addressbook URI, I have no idea how it ends up there). Anyway, I can fix this by providing roundcube with the ready vcard to skip roundcube's own vcard creation process.

@mstilkerich
Copy link
Owner

A fix is available in the issue345_crashOnPhotoCrop branch. It would be great if you could try. I will still have to extend the tests a bit before merging this back to master.

@mstilkerich mstilkerich changed the title wrong photo encoding causes contact display crashing 1) Crash on error during photo cropping, 2) exported VCard contains broken PHOTO property Jun 16, 2021
@mstilkerich mstilkerich self-assigned this Jun 16, 2021
@thopico
Copy link
Author

thopico commented Jun 16, 2021

Thank you for investigating into this. I believe I've understood the two issues you described.

I'm not sure how to test your fix, I simply switched the carddav folder plugin with the new sources. Is this a good way?

The app crashed immediately when I try to access the contacts panel with this error:

[Wed Jun 16 19:58:33.120044 2021] [php7:error] [pid 123] [client 172.18.0.2:49228] PHP Fatal error:  Uncaught TypeError: Return value of carddav::decryptPassword() must be of the type string, bool returned in /var/www/html/plugins/carddav/carddav.php:748
  Stack trace:
    #0 /var/www/html/plugins/carddav/carddav.php(382): carddav->decryptPassword('xxxxxxxxxxxxxxx...')
    #1 /var/www/html/program/lib/Roundcube/rcube_plugin_api.php(469): carddav->getAddressbook(Array)
    #2 /var/www/html/program/include/rcmail.php(226): rcube_plugin_api->exec_hook('addressbook_get', Array)
    #3 /var/www/html/program/steps/addressbook/func.inc(320): rcmail->get_address_book('carddav_12')
    #4 /var/www/html/program/steps/addressbook/func.inc(256): rcmail_contact_groups(Array)
    #5 /var/www/html/program/include/rcmail_output_html.php(1359): rcmail_directory_list(Array)
    #6 [internal function]: rcmail_output_html->xml_command(Array)
    #7 /var/www/html/program/include/rcmail_output_html.php(1209): preg_replace_callback('/<roundcube:([-...', Array, '<roundcube:incl...')
    #8 /var/www/html/program/include/rcmail_output_html.php(781): rcmail_output_html->p in /var/www/html/plugins/carddav/carddav.php on line 748, referer: https://my.domain.tld/?_task=mail&_mbox=INBOX

@mstilkerich
Copy link
Owner

mstilkerich commented Jun 17, 2021

In principal yes, you can drop in the new sources into the plugin directory.

Two things to consider though:

  1. Copy your config.inc.php
  2. The plugin dependencies might be stored inside the plugin (depending on installation method). If your old carddav folder contains a vendor subdirectory, copy that as well.

Concerning your error, I cannot match the line numbers with the corresponding code locations in the issue branch, so be sure to fetch the correct version. I also have no idea how the current code might end up in returning a bool instead of a string.

https://github.com/mstilkerich/rcmcarddav/archive/refs/heads/issue345_crashOnPhotoCrop.zip

And one more info concerning the fix: With your PHP, the cropping still won't work because it lacks support for JPEG processing. But instead of crashing it should now show the uncropped photo instead.

@thopico
Copy link
Author

thopico commented Jun 17, 2021

Hm, sorry.. I believe I started to merge the two versions but stopped somewhere before having finished... Anyway I tried it again and it works fine !

The contacts panel takes some time to load completely, while issuing some php errors, but maybe it is because of my hazardous install. It looks like the carddav sync between RC and NC addressbook is not working anymore (but data is the same so... 🤷‍♂️) :

carddav: <ce91c413> [5 ERR] Errors occurred during the refresh of addressbook 12: Exception: Expected Multistatus HTTP request was not successful (401 Unauthorized): <?xml version="1.0" encoding="utf-8"?>                                                                                                                                                                                                 
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">                                                                                                                                        
  <s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>                                                                                                                                 
  <s:message>No public access to this resource., Username or password was incorrect, No 'Authorization: Bearer' header found. Either the client didn't send one, or the server is mis-configured, Username or password was incorrect</s:message>
</d:error>
in /var/www/html/vendor/mstilkerich/carddavclient/src/CardDavClient.php:488
Stack trace:
  #0 /var/www/html/vendor/mstilkerich/carddavclient/src/CardDavClient.php(504): MStilkerich\CardDavClient\CardDavClient::assertHttpStatus(Object(GuzzleHttp\Psr7\Response), 207, 207, 'Expected Multis...')
  #1 /var/www/html/vendor/mstilkerich/carddavclient/src/CardDavClient.php(434): MStilkerich\CardDavClient\CardDavClient::checkAndParseXMLMultistatus(Object(GuzzleHttp\Psr7\Response), 'MStilkerich\\Car...')
  #2 /var/www/html/vendor/mstilkerich/carddavclient/src/WebDavResource.php(157): MStilkerich\CardDavClient\CardDavClient->findProperties('https://xxx...', Array)
  #3 /var/www/html/vendor/mstilkerich/carddavclient/src/WebDavResource.php(142): MStilkerich\CardDavClient\WebDavResource->refreshProperties()
  #4 /var/www/html/vendor/mstilkerich/carddavclient/src/WebDavCollection.php(124): MStilkerich\CardDavClient\WebDavResource->getProperties()
  #5 /var/www/html/vendor/mstilkerich/carddavclient/src/WebDavCollection.php(72): MStilkerich\CardDavClient\WebDavCollection->supportsReport('{DAV:}sync-coll...')
  #6 /var/www/html/vendor/mstilkerich/carddavclient/src/Services/Sync.php(108): MStilkerich\CardDavClient\WebDavCollection->supportsSyncCollection()
  #7 /var/www/html/vendor/mstilkerich/carddavclient/src/Services/Sync.php(71): MStilkerich\CardDavClient\Service\Sync->synchronizeOneBatch(Object(MStilkerich\CardDavClient\AddressbookCollection), Object(MStilkerich\CardDavAddressbook4Roundcube\SyncHandlerRoundcube), Array, 'http://sabre.io...')
  #8 /var/www/html/plugins/carddav/src/Addressbook.php(1211): MStilkerich\CardDavClient\Services\Sync->synchronize(Object(MStilkerich\CardDavClient\AddressbookCollection), Object(MStilkerich\CardDavAddressbook4Roundcube\SyncHandlerRoundcube), Array, 'http://sabre.io...')
  #9 /var/www/html/plugins/carddav/carddav.php(1381): MStilkerich\CardDavAddressbook4Roundcube\Addressbook->resync()
  #10 /var/www/html/plugins/carddav/carddav.php(396): carddav->resyncAddressbook(Object(MStilkerich\CardDavAddressbook4Roundcube\Addressbook))
  #11 /var/www/html/program/lib/Roundcube/rcube_plugin_api.php(469): carddav->getAddressbook(Array)
  #12 /var/www/html/program/include/rcmail.php(226): rcube_plugin_api->exec_hook('addressbook_get', Array)
  #13 /var/www/html/program/steps/addressbook/func.inc(320): rcmail->get_address_book('carddav_12')
  #14 /var/www/html/program/steps/addressbook/func.inc(256): rcmail_contact_groups(Array)
  #15 /var/www/html/program/include/rcmail_output_html.php(1359): rcmail_directory_list(Array)
  #16 [internal function]: rcmail_output_html->xml_command(Array)
  #17 /var/www/html/program/include/rcmail_output_html.php(1209): preg_replace_callback('/<roundcube:([-...', Array, '<roundcube:incl...')
  #18 /var/www/html/program/include/rcmail_output_html.php(781): rcmail_output_html->parse_xml('<roundcube:incl...')
  #19 /var/www/html/program/include/rcmail_output_html.php(615): rcmail_output_html->parse('addressbook', false)
  #20 /var/www/html/index.php(319): rcmail_output_html->send('addressbook')
  #21 {main}

@thopico
Copy link
Author

thopico commented Jun 17, 2021

When exporting the contact, photo encoding fits in 15306 lines but maybe it is because crop info is not kept?

@mstilkerich
Copy link
Owner

I changed the export code a bit to be simpler. It now provides the card exactly as provided by the carddav server, except for PHOTO where an externally referenced PHOTO is stored inside the card (because the URI normally is not accessible without authentication, and thus limits interoperability of the exported card), and cropping the photo if needed (reduces size of the exported card, plus the cropping feature is not very widely supported outside the Apple ecosystem).

I merged this back to master now.

Concerning your error: The authentication is rejected by the server, like you have the wrong password stored. I don't know how this happened, maybe a side effect of your manual merge? Anyway, I would simply suggest to enter the password again in the settings panel and save the addressbook. Otherwise you will not get updates from the server anymore, and won't be able to change / add / delete cards.

Thanks again for reporting this issue.

@thopico
Copy link
Author

thopico commented Jun 29, 2021

I now use the latest release for production and it works great!
Thank you having solved this so quickly.

@mstilkerich
Copy link
Owner

Thanks for the feedback, happy to hear it works 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants