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.

Sign up for GitHub

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

Jump to bottom

Fix issue #306 #316

Merged
merged 6 commits into from
May 23, 2018
Merged

Fix issue #306 #316

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
88ec00b
Replace '\n' by endl that flush content. Useful for debugging
piponazo May 19, 2018
bf79396
style: Apply formatting guidelines to one-line ifs
piponazo May 19, 2018
4bab67e
debug: Add new debug message to print the PNG chunk being processed
piponazo May 19, 2018
758a814
Add tests for #306
piponazo May 20, 2018
838dee7
Analyze minimum needed number of null separators in PngChunk::parseTX…
piponazo May 19, 2018
4fbfdbb
Check validity of compressionFlag & compressionMethod
piponazo May 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/pngchunk_int.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,22 +165,28 @@ namespace Exiv2 {
else if(type == iTXt_Chunk)
{
const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_-1], '\0');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I think this should be &data.pData_[data.size_] as the end iterator should point to the first element outside of the range.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahg, you are right. I'll create a mini PR fixing that.

enforce(nullSeparators >= 2, Exiv2::kerCorruptedMetadata);
enforce(nullSeparators >= 2, Exiv2::kerCorruptedMetadata, "iTXt chunk: not enough null separators");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message you added will not be displayed, as kerCorruptedMetadata does not include it in the output via e.what().

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the message was not printed, but I thought it could be a good idea to have that string there in case we can add unit tests in the future around this code. I did not check though if we could get the string when catching the exception in the unit tests code.

Anyways, I think it does not hurt to have a small string in the enforcecalls giving some details of why we are placing the enforce calls in the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will hurt the performance slightly in case the exception gets thrown, as then the Exiv2::Error constructor will search for a place where to insert the string but won't be able to find one. Not really a big deal though. In case you want the error message printed, we you can use kerErrorMessage (albeit that is a quite generic error).


// Extract a deflate compressed or uncompressed UTF-8 text chunk

// we get the compression flag after the key
const byte* compressionFlag = data.pData_ + keysize + 1;
const byte compressionFlag = data.pData_[keysize + 1];
// we get the compression method after the compression flag
const byte* compressionMethod = data.pData_ + keysize + 2;
const byte compressionMethod = data.pData_[keysize + 2];

enforce(compressionFlag == 0x00 || compressionFlag == 0x01, Exiv2::kerCorruptedMetadata,
"iTXt chunk: not valid value in compressionFlag");
enforce(compressionMethod == 0x00, Exiv2::kerCorruptedMetadata,
"iTXt chunk: not valid value in compressionMethod");

// language description string after the compression technique spec
std::string languageText((const char*)(data.pData_ + keysize + 3));
unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
// translated keyword string after the language description
std::string translatedKeyText((const char*)(data.pData_ + keysize + 3 + languageTextSize +1));
unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());

if ( compressionFlag[0] == 0x00 )
if ( compressionFlag == 0x00 )
{
// then it's an uncompressed iTXt chunk
#ifdef DEBUG
Expand All @@ -194,7 +200,7 @@ namespace Exiv2 {
arr.alloc(textsize);
arr = DataBuf(text, textsize);
}
else if ( compressionFlag[0] == 0x01 && compressionMethod[0] == 0x00 )
else if ( compressionFlag == 0x01 && compressionMethod == 0x00 )
{
// then it's a zlib compressed iTXt chunk
#ifdef DEBUG
Expand Down