Bug #795

Set/Get of PNG comment

Added by Thomas Beutlich almost 6 years ago. Updated over 5 years ago.

Status:ClosedStart date:24 Oct 2011
Priority:NormalDue date:
Assignee:Andreas Huggel% Done:

100%

Category:image format
Target version:0.23

Description

If I set and retrieve the comment of a PNG file using Exiv2 0.22 on WinXPSP3 I get strange characters if the comment string is shorter than 8 characters.
C:\temp>exiv2 -c "PNG" imagemagick.png
C:\temp>exiv2 -pc imagemagick.png
PNG x☺4

Associated revisions

Revision 2634
Added by Andreas Huggel almost 6 years ago

#795: Fixed decoding of PNG comment.

History

#1 Updated by Thomas Beutlich almost 6 years ago

After two weeks of waiting: Can someone please accept this issue. I tried but I failed.

If I run the debug version of exiv2.exe (i.e. with undefined SUPPRESS_WARNINGS) I get a strange error on the original PNG file imagemagick.png which like a strange bug.

C:\temp>exiv2.exe -pc imagemagick.png
Error: Directory NikonPreview with 512 entries considered invalid; not read.

#2 Updated by Andreas Huggel almost 6 years ago

  • Assignee set to Andreas Huggel
  • Target version deleted (0.23)

Hi Thomas, I'll have a look, sorry for the delay.

The error complains about an invalid NikonPreview directory, which typically gets corrupted if the image is edited (converted?) with a software that doesn't handle this part of the makernote correctly.

Andreas

#3 Updated by Thomas Beutlich almost 6 years ago

OK, thanks for looking into this.

By the way, SUPPRESS_WARNINGS is kind of misleading here. If it is undefined, I get an error and exiv2 aborts. If it is defined, there is neither a warning nor a error. I think it is better if SUPPRESS_WARNINGS only suppresses warnings and not changes the error / exception behaviour.

#4 Updated by Andreas Huggel almost 6 years ago

The differentiation between "Warning" and "Error" messages is subjective and I have no well-defined criteria for it (suggestions welcome). Both are equally affected by the SUPPRESS_WARNINGS flag. Exceptions are different. SUPPRESS_WARNINGS should not modify the program flow, in particular it should not affect exceptions. It's a bug if it does, but the code for the error you highlighted looks ok, it doesn't explain your observation, will also check in more detail later.

        // Sanity check with an "unreasonably" large number
        if (n > 256) {
#ifndef SUPPRESS_WARNINGS
            EXV_ERROR << "Directory " << groupName(object->group()) << " with " 
                      << n << " entries considered invalid; not read.\n";
#endif
            return;
        }

#5 Updated by Andreas Huggel almost 6 years ago

  • Status changed from New to Assigned

r2634 fixes the PNG comment decoding issue I can see here, using Valgrind on Mac OSX. Maybe there are memory leaks too, I'll have to also test on Linux. Does it look better on Windows now?

What command did you run to get exiv2 to abort as described in the comment above (with SUPPRESS_WARNINGS undefined)?

-ahu.

#6 Updated by Thomas Beutlich almost 6 years ago

Yes, r2634 solves the primary problem with the PNG comment on Windows using MS Visual Studio C++ 2010 SP1 Express Edition.

I checked in Debugger with Visual Leak Detector Version 2.2 (if compiled with defined SUPPRESS_WARNINGS) and could not detect any memory holes.

The command that aborts (if compiled with undefined SUPPRESS_WARNINGS) is (taken from above):
C:\temp>exiv2.exe -pc imagemagick.png
Error: Directory NikonPreview with 512 entries considered invalid; not read.

This error still occurs with the fix r2634.

#7 Updated by Andreas Huggel almost 6 years ago

C:\temp>exiv2.exe -pc imagemagick.png
Error: Directory NikonPreview with 512 entries considered invalid; not read.
This error still occurs with the fix r2634.

Are you sure it aborts, i.e., throws an exception or crashes? Here all I get is that error message, but that doesn't change the program flow (see the code snippet above). I can't reproduce any exception or crash, if you really observe something like that, can you please try to trace where it comes from? (btw the -q option will turn it off)

Andreas

#8 Updated by Thomas Beutlich almost 6 years ago

Sorry for confusion. It is not a program exception or crash but rather a regular program termination with an error message.

With undefined SUPPRESS_WARNINGS:
C:\temp>exiv2.exe -pc imagemagick.png
Error: Directory NikonPreview with 512 entries considered invalid; not read.

With defined SUPPRESS_WARNINGS:
C:\temp>exiv2 -pc imagemagick.png
PNG

It is just about the different program flow. If SUPPRESS_WARNINGS is not defined I get an error message rather than the expected PNG comment. If SUPPRESS_WARNINGS is defined, the program output is as expected.

#9 Updated by Andreas Huggel almost 6 years ago

  • Status changed from Assigned to Resolved
  • Target version set to 0.23
  • % Done changed from 0 to 100

Marking the issue as resolved, the remaining Valgrind complaints don't seem to be caused by Exiv2.

As for the error message, does this explain what you see: When you first run exiv2 -pc imagemagick.png, you get the error message and no PNG comment, because none was set yet. Running exiv2 -c "PNG" imagemagick sets the comment (and also writes the error message to stderr). Running exiv2 -pc imagemagick.png again after this writes out the comment but not the error message anymore. That's because the Exif block was re-written without the suspicious directory (because that wasn't read it in the first place, as the error message says) when the comment was set.

#10 Updated by Andreas Huggel over 5 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux