By default, Bugzilla does not search the list of RESOLVED bugs.
You can force it to do so by putting the upper-case word ALL in front of your search query, e.g.: ALL tdelibs
We recommend searching for bugs this way, as you may discover that your bug has already been resolved and fixed in a later release.
Bug 2625 - tdeabc/vcardparser/vcardparser.cpp accepted encoding
Summary: tdeabc/vcardparser/vcardparser.cpp accepted encoding
Status: RESOLVED FIXED
Alias: None
Product: TDE
Classification: Unclassified
Component: tdelibs (show other bugs)
Version: R14.0.x [Trinity]
Hardware: amd64 Linux
: P5 normal
Assignee: Timothy Pearson
URL:
Depends on:
Blocks: R14.0.6
  Show dependency treegraph
 
Reported: 2016-04-07 15:40 CDT by deloptes
Modified: 2018-10-11 08:37 CDT (History)
5 users (show)

See Also:
Compiler Version:
TDE Version String:
Application Version:
Application Name:


Attachments
patch for tdelibs-tdeabc utf8 support (36.55 KB, patch)
2016-05-13 17:52 CDT, deloptes
Details | Diff
Patch by Slavek to add testing into tdeabc (7.30 KB, patch)
2016-05-14 06:56 CDT, deloptes
Details | Diff
tdepim : Fix utf8 handling in CardDAV (870 bytes, patch)
2016-05-21 05:22 CDT, Slávek Banko
Details | Diff
tdepim : Fix utf8 handling in tdeabc resource cache (1.54 KB, patch)
2016-05-21 05:25 CDT, Slávek Banko
Details | Diff
tdepim : Fix utf8 handling in kaddressbook thumbnailcreator (537 bytes, patch)
2016-05-21 06:35 CDT, Slávek Banko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description deloptes 2016-04-07 15:40:41 CDT
There is an issue passing utf8 to the vCardParser.
I raise it here, so that we do not forget to fix and test it

Fat-Zer was so kind to inspect the code and come up with a proposal to fix it
Here is the relevant part of the discussion

SUBJECT: Re: Re: TQString::fromUtf8 vs TQString::fromLatin1 [possible bug in parseVCard]

Fat-Zer wrote:

> 2016-03-26 11:42 GMT+03:00 deloptes

Yes, It seems you are right there is a bug, haven't tried myself, but
should do the trick:

diff --git a/tdeabc/vcardparser/vcardparser.cpp
b/tdeabc/vcardparser/vcardparser.cpp
index 7ac07ce..db33263 100644
--- a/tdeabc/vcardparser/vcardparser.cpp
+++ b/tdeabc/vcardparser/vcardparser.cpp
@@ -152,7 +152,7 @@ VCard::List VCardParser::parseVCards( const TQString& text )
             KCodecs::quotedPrintableDecode( input, output );
           }
         } else {
-          output = TQCString(value.latin1());
+          output = TQCString(value.utf8());
         }

         if ( params.findIndex( "charset" ) != -1 ) { // have to
convert the data

Note that VCardParser::parseVCards() is generally encoding-unsafe...
Comment 1 deloptes 2016-04-11 16:48:15 CDT
this is easy to test when copy paste data from one resource to another.
I'll test the fix proposed by Fat-Zer next and report back.
If successful we can patch and push.

regards
Comment 2 deloptes 2016-04-15 02:34:24 CDT
I didn't have to do anything to test the proposed patch.
This is another example for theoretical work, that does not work :)

When recompiled and deployed the new tdelib with the fix, after restarting when the addressbook was open it did show double coded characters for all contacts.

Worse it saved couple of copies of the address book in the famous __[n] style

It turns out the function was called when opening the address book and mangled all non ASCII chars.

I've noticed another possible issue with there in the code. When I have some better fix I'll be back.

Thanks to Fat-Zer however for pointing the relevant place

regards
Comment 3 Michele Calgaro 2016-04-15 20:36:29 CDT
If I understand correctly, we need to push Fat-Zer patch as it is, right?

diff --git a/tdeabc/vcardparser/vcardparser.cpp
b/tdeabc/vcardparser/vcardparser.cpp
index 7ac07ce..db33263 100644
--- a/tdeabc/vcardparser/vcardparser.cpp
+++ b/tdeabc/vcardparser/vcardparser.cpp
@@ -152,7 +152,7 @@ VCard::List VCardParser::parseVCards( const TQString& text )
             KCodecs::quotedPrintableDecode( input, output );
           }
         } else {
-          output = TQCString(value.latin1());
+          output = TQCString(value.utf8());
         }

         if ( params.findIndex( "charset" ) != -1 ) { // have to
convert the data
Comment 4 Slávek Banko 2016-04-16 03:45:23 CDT
By comment 2, I understood that the proposed patch at the moment brings more complications => double encoding of characters.
Comment 5 Michele Calgaro 2016-04-16 05:53:08 CDT
ah... this morning I must have been sleeping. I understood that exact opposite! :-(
So we wait until deloptes comes up with a working patch. Sorry for the noise.
Comment 6 deloptes 2016-05-13 17:52:14 CDT
Created attachment 2658 [details]
patch for tdelibs-tdeabc utf8 support


Fixed issues with non Utf8 support in the addressbook 
   vcardparser
   tools
   vcardformatplugin
   converter.createVCards
   migrated testing support from the Makefaile.am in to the CMake
   additional test programs
   resourcefile - improved the backup read/write
   
The patch by Fat-Zer also included.
Comment 7 deloptes 2016-05-14 06:56:17 CDT
Created attachment 2659 [details]
Patch by Slavek to add testing into tdeabc

This patch is prereq for 2658 - added here for completeness
Comment 8 Michele Calgaro 2016-05-18 22:20:03 CDT
From Emanoil comments on email:

Proper way to apply is:
1. Slavek patch from comment 7
2. my patch from comment 6
Comment 9 Slávek Banko 2016-05-19 02:01:41 CDT
I will try to test it on my systems before push.
Comment 10 Slávek Banko 2016-05-19 18:38:02 CDT
(In reply to Slávek Banko from comment #9)
> I will try to test it on my systems before push.

I have the first test result: virtually all of my contacts are now broken - it seems to double utf8 transcoding.

Detailed information: I have contacts in sync via CardDAV to our SOGo server. Fortunately broken contacts were not updated back to the SOGo server == contacts on my phone was not destroyed.
Comment 11 Michele Calgaro 2016-05-19 22:09:49 CDT
>I have the first test result: virtually all of my contacts are now broken - it >seems to double utf8 transcoding.
Slavek,
perhaps you can provide some test sample for Emanoil, so it can work on a fix and verify against his test cases and yours. Otherwise we could see this scenario repeat over and over :-)
Comment 12 deloptes 2016-05-20 07:38:38 CDT
This was the case for me as well before digging deeper into it.
It might be you need to update the Dav code - if it was relying on the broken tdeabc and one has "improved" only on the client side. I would guess that it is converting to utf before it goes to the Cal/CardDav.
It could be that I added somewhere something wrong.
The tests are really good for that, but they also rely on assumptions.

Let me know if I can help or learn something.

regards
Comment 13 deloptes 2016-05-20 15:13:56 CDT
The problem is most likely caused in

 tdepim/tderesources/carddav/resource.cpp

by

 Addressee::List addressees = conv.parseVCards( data.utf8() );

at line 430, 459

While it is defined as
 static VCard::List parseVCards( const TQString& text );

so it should be simply

 Addressee::List addressees = conv.parseVCards( data );

utf8() returns TQCString and this breaks it I guess.

You can test this by changing it in testread.cpp and running the test.sh

It is worth checking other resources that use parseVCards. This was part of the big problem.

regards
Comment 14 Slávek Banko 2016-05-21 05:22:23 CDT
Created attachment 2660 [details]
tdepim : Fix utf8 handling in CardDAV

Emanoil, excellent analysis - patch created by your remarks works perfectly! CardDAV is now back in order.
Comment 15 Slávek Banko 2016-05-21 05:25:26 CDT
Created attachment 2661 [details]
tdepim : Fix utf8 handling in tdeabc resource cache

Additional patch related to CardDAV and generally all external resources - fixed utf8 handling for reading vCards from local cache.
Comment 16 deloptes 2016-05-21 06:32:32 CDT
Glad to hear! I think we did a very good work, looking into the patches it all makes sense.
You can not imagine how glad I am to have cleaned up all this. I've been having issues since the time of opensync ~ 2005. Fortunately back then I had only phones using iso8859-15, but since couple of years it became a real pain and this was the only functionality I was desperately missing in TDE. I'm just glad that this year I am not overloaded as before. Lets hope this keeps constant.

Thank you all for helping and guiding.

regards
Comment 17 Slávek Banko 2016-05-21 06:35:02 CDT
Created attachment 2662 [details]
tdepim : Fix utf8 handling in kaddressbook thumbnailcreator

The last patch related to parseVCards - for thumbnails and previews of vCard files.