Bug 139766

Summary:	Crash when displaying EXIF metadata with Unicode comment
Product:	[Applications] digikam	Reporter:	Leonid Zeitlin <lz>
Component:	Metadata-Exif	Assignee:	Digikam Developers <digikam-bugs-null>
Status:	RESOLVED FIXED
Severity:	crash	CC:	caulier.gilles
Priority:	NOR
Version:	0.9.0
Target Milestone:	---
Platform:	Fedora RPMs
OS:	Linux
Latest Commit:		Version Fixed In:	0.9.1
Sentry Crash Report:
Attachments:	Proposed patch to fix the crash described in this report

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!