Hi all committers of PoDoFo,
>From: Nenad Novak <nenad.no...@gmail.com>>To:
>podofo-users@lists.sourceforge.net
>Sent: 15:02 Saturday, 3 October 2015
>Subject: Re: [Podofo-users] unreachable-code and robustness fixes in
>PdfPagesTree::GetPageNode()
>
hello Nenad Novak,
> Please, consider other users which just try to create a new PDF. It's
> nothing CRITICAL here. You can not retrieve a page 0
> when it does not exist yet.
yes, therefore it is invalid to call the (private, nonetheless) method
GetPageNode() with a page index even of 0, because that means (it's
0-based!) the first page, which doesn't exist in the document yet.
You don't rely on outcome of a method call which has invalid parameters,
do you?
I have attached a patch which should fix these calls when you create a
new page in a document which hasn't got any yet. Committers of PoDoFo,
could you please review it and if accepted, apply/commit it (separately)
to the public repository, otherwise please tell me why not?
> Part of patch is wrong. It should be:
> if( static_cast<int>(numKids) < nPageNum )
No, it's not, because with your form, if nPageNum == numKids, it would
point one past the end of the Kids array, so be invalid. If there are
no Kids yet, then the index 0 is *not* valid either (it means "first page").
> http://sourceforge.net/p/podofo/code/1683
> // 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.",
> nPageNum, static_cast<int>(numKids) );
> return NULL;
> }
>
>
>
> E:\devel\podofo-trunk\build\examples\helloworld-base14>helloworld-base14.exe
> sample.pdf
> CRITICAL: Cannot retrieve page 0 from a document with only 0
> pages.Courier Width = 309.6 Height = 9.432
To try to access an array past their end which is at index
static_cast<int>(numKids)-1, in a private method to boot, is critical (IMHO).
The GetPageNode patch is my first source code contribution to this project,
in which I limited myself to just that method, because I didn't want to mess
with other parts of the code yet, and to the revised patch I added a program
to generate the test PDF document attached to that e-mail which could have
been built and run by some reviewer after applying and before committing the
change. I'm so sorry that I didn't do it myself (I was in a hurry to get ready
to depart from where I was on vacation then).
> Courier-Bold Width = 345.6 Height = 9.432
> Courier-Oblique Width = 367.2 Height = 9.432
> Courier-BoldOblique Width = 396 Height = 9.432
> Helvetica Width = 275.304 Height = 11.1
> Helvetica-Bold Width = 315.444 Height = 11.1
> Helvetica-Oblique Width = 320.652 Height = 11.1
> Helvetica-BoldOblique Width = 360.12 Height = 11.1
> Times-Roman Width = 279.12 Height = 10.8
> Times-Bold Width = 277.992 Height = 10.8
> Times-Italic Width = 269.112 Height = 10.8
> Times-BoldItalic Width = 296.16 Height = 10.8
> Symbol Width = 256.548 Height = 10.8
> ZapfDingbats Width = 405.996 Height = 10.8
> Arial Width = 249.973 Height = 13.7988
> Times New Roman Width = 303.768 Height = 13.7988
> Verdana Width = 304.57 Height = 14.584
>
> Created a PDF file containing the line "Hello World!": sample.pdf
Best regards, mabri
----- Original Message -----
From: zyx <z...@litepdf.cz>
To: podofo-users@lists.sourceforge.net
CC:
Sent: 19:54 Friday, 2 October 2015
Subject: Re: [Podofo-users] unreachable-code and robustness fixes in
PdfPagesTree::GetPageNode()
On Sat, 2015-09-19 at 00:05 +0000, Matthew Brincke wrote: So I hope that the
patch now makes the method so robust that you'll
accept it, please review it, and if accepted, please apply
(separately)
to the public repository.
>Hi,
I gave a little testing to your patch and it seems to work, I didn't
notice any regression with the PDF files I tried. Interestingly, the
produced PDF seems not to be too nice, at least evince (3.16.1) is
quite unhappy about it, not being able to show other than the first
page. It can be an issue in the evince, I do not know. I committed
your change as revision 1683: http://sourceforge.net/p/podofo/code/1683
Bye, zyx
------------------------------------------------------------------------------Index: src/doc/PdfPagesTree.cpp
===================================================================
--- src/doc/PdfPagesTree.cpp (revision 1683)
+++ src/doc/PdfPagesTree.cpp (working copy)
@@ -140,9 +140,13 @@ void PdfPagesTree::InsertPage( int nAfterPageIndex
//printf("Fetching page node: %i\n", nAfterPageIndex);
PdfObjectList lstParents;
+ PdfObject* pPageBefore = NULL;
//printf("Searching page=%i\n", nAfterPageIndex );
- PdfObject* pPageBefore = this->GetPageNode( nAfterPageIndex, this->GetRoot(), lstParents );
-
+ if( this->GetTotalNumberOfPages() != 0 ) // no GetPageNode call w/o pages
+ {
+ pPageBefore = this->GetPageNode( nAfterPageIndex, this->GetRoot(),
+ lstParents );
+ }
//printf("pPageBefore=%p lstParents=%i\n", pPageBefore,lstParents.size() );
if( !pPageBefore || lstParents.size() == 0 )
{
@@ -193,8 +197,12 @@ void PdfPagesTree::InsertPages( int nAfterPageInde
}
PdfObjectList lstParents;
- PdfObject* pPageBefore = this->GetPageNode( nAfterPageIndex, this->GetRoot(), lstParents );
-
+ PdfObject* pPageBefore = NULL;
+ if( this->GetTotalNumberOfPages() != 0 ) // no GetPageNode call w/o pages
+ {
+ pPageBefore = this->GetPageNode( nAfterPageIndex, this->GetRoot(),
+ lstParents );
+ }
if( !pPageBefore || lstParents.size() == 0 )
{
if( this->GetTotalNumberOfPages() != 0 )
------------------------------------------------------------------------------
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
