[poppler] Second opinion for patch

[Albert Astals Cid]

El divendres, 18 de novembre de 2016, a les 2:17:27 CET, William Bader va 
escriure:
> The early return seems to exit when two consecutive objects have the same id
> and gen.

Which early return? I don't see any check for two consecutive objects with the 
same id in that code.

> Is that a normal marker for the end of the list or was that added as a check
> to prevent an infinite loop on a bad file with a circular list? Would it be
> better to replace the test with a better check for an infinite loop?
> Applications that use poppler shouldn't hang if a user opens a corrupt PDF.
> 
> 
> In the example file, is the entry that triggers the early exit a duplicate
> of the previous entry? Would it be better to keep the test but skip the
> entry instead of exiting the loop?

No it's not a duplicate entry, as i said, it's just that the "Last" pointer is 
not pointing to the real Last if you follow the "Next" pointer of the 
siblings.

Cheers,
  Albert

> 
> 
> William
> 
> 
> ________________________________
> From: poppler <poppler-bounces at lists.freedesktop.org> on behalf of Albert
> Astals Cid <aacid at kde.org> Sent: Thursday, November 17, 2016 4:05 PM
> To: poppler at lists.freedesktop.org
> Subject: Re: [poppler] Second opinion for patch
> 
> El dijous, 17 de novembre de 2016, a les 17:48:24 CET, Carlos Garcia Campos
> va
> escriure:
> > Albert Astals Cid <aacid at kde.org> writes:
> > > Can someone have a look at
> > > https://bugs.freedesktop.org/show_bug.cgi?id=98732
> > > 
> > > I'd like a second opinion on the patch, it fixes the problem on that
> > > broken
> > > file for me but before commiting i'd like to know i'm not missing
> > > something.
> > 
> > I guess the file is broken because "Last" is required field in outline
> > dictionary. In that case I would keep using Last if present, and simply
> > remove the early return in OutlineItem::readItemList() otherwise.
> 
> Yes it's broken, the file does have a "Last", it's just that the "Last"
> doesn't really point to the last but to somehwere in the middle (if you use
> the next->next->next).
> 
> Given that Adobe Reader shows all of the items i guess it also ignores it
> and just relies on next.
> 
> Cheers,
>   Albert
> 
> > > Cheers,
> > > 
> > >   Albert
> > > 
> > > _______________________________________________
> > > poppler mailing list
> > > poppler at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/poppler
> 
> poppler Info Page -
> freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/poppler>
> lists.freedesktop.org
> Subscribing to poppler: Subscribe to poppler by filling out the following
> form. You will be sent email requesting confirmation, to prevent others
> from ...
> 
> 
> 
> 
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/poppler
> poppler Info Page -
> freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/poppler>
> lists.freedesktop.org
> Subscribing to poppler: Subscribe to poppler by filling out the following
> form. You will be sent email requesting confirmation, to prevent others
> from ...