Hi all,
attached to this email is my first source code contribution
to this project: a patch removing unreachable code including
its comment because I think with robustness fixes to the method
the situation described in it really cannot happen. What do you
think?
Please review the patch, and if it is accepted, please apply it
(separately) to the public repository.
Best regards, mabri
unreachable-code and robustness fixes in PdfPagesTree::GetPageNode()
Index: src/doc/PdfPagesTree.cpp
===================================================================
--- src/doc/PdfPagesTree.cpp (revision 1676)
+++ src/doc/PdfPagesTree.cpp (working copy)
@@ -327,9 +327,11 @@
const size_t numDirectKids = rKidsArray.size();
const size_t numKids = GetChildCount(pParent);
- if( static_cast<int>(numKids) < nPageNum )
+ // use <= since nPageNum is 0-based
+ if( static_cast<int>(numKids) <= nPageNum )
{
- PdfError::LogMessage( eLogSeverity_Critical, "Cannot retrieve page %i from a document with only %i pages.",
+ PdfError::LogMessage( eLogSeverity_Critical,
+ "Cannot retrieve page %i from a document with only %i pages.",
nPageNum, static_cast<int>(numKids) );
return NULL;
}
@@ -342,14 +344,6 @@
rLstParents.push_back( pParent );
return GetPageNodeFromArray( nPageNum, rKidsArray, rLstParents );
}
- else if( numDirectKids == numKids && static_cast<size_t>(nPageNum) < numDirectKids )
- {
- // This node has only page nodes as kids,
- // but does not contain our page,
- // skip it - this case should never occur because
- // of handling of childs in the else part below.
- return NULL;
- }
else
{
// We have to traverse the tree
@@ -357,9 +351,12 @@
{
if( (*it).IsArray() )
{
- // Fixes some broken PDFs who have trees with 1 element kids arrays
+ // Fixes some broken PDFs having trees with 1-elem. kids arrays
rLstParents.push_back( pParent );
- return GetPageNodeFromArray( nPageNum, (*it).GetArray(), rLstParents );
+ PdfObject* pPageNode = GetPageNodeFromArray( nPageNum,
+ (*it).GetArray(), rLstParents );
+ if ( pPageNode != NULL )
+ return pPageNode;
}
else if( (*it).IsReference() )
{
@@ -386,7 +383,7 @@
return this->GetPageNode( nPageNum, pChild, rLstParents );
}
}
- else // Type == Page
+ else if( this->IsTypePage(pChild) )
{
if( 0 == nPageNum )
{
@@ -398,6 +395,14 @@
if(nPageNum > 0 )
nPageNum--;
}
+ else
+ {
+ PdfError::LogMessage( eLogSeverity_Critical,
+ "Requesting page index %i. "
+ "Invalid datatype referenced in kids array: %s\n",
+ nPageNum, pChild.GetDataTypeString()
+ ); // TODO: perhaps log reference too?
+ }
}
else
{
------------------------------------------------------------------------------
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
