Bug 99357

Are you feeding a string directly to this?

In my case yes, but in any case invalid data should not crash it, or?

You're not supposed to do that, it's marked internal for a reason, actually it should be private with friend access. Why are you feeding strings directly to it?

I do that e.g. to handle table of contents, just like okular does it in generator_pdf.cpp, too:

     if (!e.attribute(QStringLiteral("Destination")).isNull())
        {
            Okular::DocumentViewport vp;
            fillViewportFromLinkDestination( vp, Poppler::LinkDestination(e.attribute(QStringLiteral("Destination"))) );
            item.setAttribute( QStringLiteral("Viewport"), vp.toString() );
        }

=> if you have any broken entry there, that will segfault, which IMHO is not nice.

Now you can argue that the toc QDomDocument should contain only wellformed things, still I see no real point in not avoiding the doom in the constructor as that check is really cheap.

Actually my code is here:

https://github.com/AbsInt/FirstAid/blob/master/src/tocdock.cpp

it only contains nice things because it's created by poppler-qt itself so yes, it's only nice things.

If you're going to care about people passing stupidly wrong strings to that function, at least add a warning saying "your string is wrong you fool" and don't silently fail.

It doesn't fail, you will just get a default constructed invalid destination which you can nicely check afterwards e.g. for page not > 0.

It is just the same as if you pass any other garbage string that has enough separators, there you get ATM silently converted garbage values, but no segfault.

Applied, evne if your patch didn't properly apply because the tabs/spaces were messed up

Thanks