Bug 139766 - Crash when displaying EXIF metadata with Unicode comment
Summary: Crash when displaying EXIF metadata with Unicode comment
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Metadata-Exif (show other bugs)
Version: 0.9.0
Platform: Fedora RPMs Linux
: NOR crash
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-08 11:37 UTC by Leonid Zeitlin
Modified: 2017-08-13 07:26 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 0.9.1
Sentry Crash Report:


Attachments
Proposed patch to fix the crash described in this report (556 bytes, patch)
2007-01-08 11:38 UTC, Leonid Zeitlin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Leonid Zeitlin 2007-01-08 11:37:33 UTC
Version:           0.9.0 (using KDE KDE 3.5.3)
Installed from:    Fedora RPMs
Compiler:          gcc 4.1.0 
OS:                Linux

When displaying EXIF metadata (in the Metadata sidebar) with "User Comment" field in Unicode, Digikam crashes sporadically. When it doesn't crash, sometimes garbage characters are seen appended to the User Comment.

The crash happens in dmetadata.cpp, function convertCommentValue, line 1147:

return QString::fromUcs2((unsigned short *)comment.data());

It appears that comment.data() is not null-terminated, which causes QString::fromUcs2 to crash. I feel that the line above:

comment += "\0\0";

doesn't achieve its purpose. It may depend on the implementation of STL, but I think this code doesn't really do anything: the right-hand side is interpreted as null-terminated string, and thus is empty, so appending it has no effect. The following code:

comment.resize(comment.length()+2, 0);

does indeed append two null characters to comment, and this change eliminates the crash. Digikam developers, please consider making this change. I am attaching the patch. And thanks for the great application!
Comment 1 Leonid Zeitlin 2007-01-08 11:38:29 UTC
Created attachment 19188 [details]
Proposed patch to fix the crash described in this report
Comment 2 caulier.gilles 2007-01-08 11:59:45 UTC
Marcel,

This is another Comments encoding stuff... What do you think about the patch ?

Gilles
Comment 3 Leonid Zeitlin 2007-01-21 12:54:32 UTC
Hi Gilles, Marcel,
Just curious is the patch is going to be reviewed
Comment 4 Marcel Wiesweg 2007-01-22 15:31:04 UTC
SVN commit 626219 by mwiesweg:

Apply patch from "lz@europe.com" (thanks for your help!):
Append terminating "\0\0" sequence to UCS2 string properly.
Adding with operator+= did nothing because it treats the sequence as a then empty C string.

BUG: 139766


 M  +2 -2      dmetadata.cpp  


--- trunk/extragear/graphics/digikam/libs/dmetadata/dmetadata.cpp #626218:626219
@@ -1146,8 +1146,8 @@
         if (charset == "\"Unicode\"")
         {
             // QString expects a null-terminated UCS-2 string.
-            // Is it already null terminated? In any case, add termination for safety.
-            comment += "\0\0";
+            // Is it already null terminated? In any case, add termination "\0\0" for safety.
+            comment.resize(comment.length() + 2, '\0');
             return QString::fromUcs2((unsigned short *)comment.data());
         }
         else if (charset == "\"Jis\"")
Comment 5 Leonid Zeitlin 2007-01-27 11:28:41 UTC
Thanks a lot!