Bug 52488

Attachment is too big.
I've uploaded it here:
http://trippelsdorf.de/cud10nebsiaol.pdf

The following patch fixes the issue for me:

diff --git a/splash/Splash.cc b/splash/Splash.cc
index b927e5e..76fdd8f 100644
--- a/splash/Splash.cc
+++ b/splash/Splash.cc
@@ -3492,8 +3492,10 @@ SplashError Splash::drawImage(SplashImageSource src, void *srcData,
       }
       scaledImg = scaleImage(src, srcData, srcMode, nComps, srcAlpha, w, h,
                             scaledWidth, scaledHeight);
-      vertFlipImage(scaledImg, scaledWidth, scaledHeight, nComps);
-      blitImage(scaledImg, srcAlpha, x0, y0, clipRes);
+      if (scaledImg) {
+        vertFlipImage(scaledImg, scaledWidth, scaledHeight, nComps);
+        blitImage(scaledImg, srcAlpha, x0, y0, clipRes);
+      }
       delete scaledImg;
     }

Created attachment 64677 [details] [review]
Guard against scaleImage returning NULL

This patch guards all callers of scaleImage against a possible NULL return value.

(In reply to comment #1)
> Attachment is too big.
> I've uploaded it here:
> http://trippelsdorf.de/cud10nebsiaol.pdf

The link is not correct, got a 404.

(In reply to comment #4)
> (In reply to comment #1)
> > Attachment is too big.
> > I've uploaded it here:
> > http://trippelsdorf.de/cud10nebsiaol.pdf
> 
> The link is not correct, got a 404.

Yes, sorry I've deleted that file, because it is copyrighted
and I don't want to get into trouble with Elsevier...

If you google for "cud10nebsiaol.pdf" you'll will find it.

Seems as if You get the problem on page 38, don't You?
As far as I can see Your patch the segfault, but it doesn't repair the problem itself: My implementation of tilingPatternFill runs into a problem with this page: it results in trying to create a huge bitmap with a size of more than 100.000 pixel in height and width. 
After Albert will commit my blend mode patch, I'll create a patch repairs that, too.

(In reply to comment #6)
> Seems as if You get the problem on page 38, don't You?

Yes, see comment 0: »This happens when I view page 38 of the attached pdf with Okular.«

> As far as I can see Your patch the segfault, but it doesn't repair the problem
> itself: My implementation of tilingPatternFill runs into a problem with this
> page: it results in trying to create a huge bitmap with a size of more than
> 100.000 pixel in height and width. 
> After Albert will commit my blend mode patch, I'll create a patch repairs that,
> too.

Thanks Thomas!

Thomas: I think i commited the patch you mention.

Octopoid's changes make sense anyway, scaledImage can return null, so we have to protect against them.

Octoploid, before i can commit your patch i need your name so i can give you proper copyright attribution.

(In reply to comment #9)
> Octopoid's changes make sense anyway, scaledImage can return null, so we have
> to protect against them.
> 
> Octoploid, before i can commit your patch i need your name so i can give you
> proper copyright attribution.

Markus Trippelsdorf <markus@trippelsdorf.de>

Thanks.

(In reply to comment #9)
> Octopoid's changes make sense anyway, scaledImage can return null, so we have
> to protect against them.
> 
> Octoploid, before i can commit your patch i need your name so i can give you
> proper copyright attribution.

Yes, like always You're true. And I tested a change in tilingPattern last weekend, but I wasn't complete satisfied with my changes, so probabbly it will take a while.
But it would be helpful if Markus can change his patch a little bit: 

      if (scaledImg == NULL) {
        return splashErrBadArg;
      }
in case that scaleImg is null. This would be more compatible to Splash::arbitraryTransformImage, where You, Albert, already made this change.

Created attachment 65059 [details] [review]
Updated patch

Updated patch with Thomas' suggestion.

Commited the protection patch, waiting for Thomas' "better" one

Created attachment 65477 [details] [review]
Avoid bogus memory error for tilingPattern

I think, I now find a solution. TilingPatterns are quite sensitive, so this solution is more a practical approach than really a theoretical solution. But I'm satisfied with the actual regtest results, and even if we get this problem under other cirumstances in futur, it now fall back to the Gfx implementation of tiling patterns.

Thomas, I'm a bit confused, it seems that rendering the file with and without this patch have the same exact output?

Am i doing something wrong?

(In reply to comment #15)
> Thomas, I'm a bit confused, it seems that rendering the file with and without
> this patch have the same exact output?
> 
> Am i doing something wrong?

Welcome to the club :-). I was confused, too. And I tried it with returning gFalse in tilingPatternFill always, so let Gfx do the stuff: No effects either. But because I also saw no differences in what Acrobat Reader shows, I didn't care anymore. So the patch just removes the "bogus memory allocation" and doesn't change other output too much (and hopefully will bring better results in other cases)

Created attachment 66003 [details]
tiling pattern

It bothers me that we get the exact same output and therefore I digged a little bit deeper into it in my vacation: The attached png is the tiling pattern only, and due to the clipping path only a part of the white square remains and therefore it paints white on white. I guess that it was a former approach to fill the frogs, but then the graphic was moved without moving the tiling pattern, and the filling of the frogs was repaired without removing the tiling pattern itself.

But with my patch everything is now moved outside the clipping path. It doesn't matter in this case because it still changes nothing, but this could cause problems with other tiling patterns, therefore I now upload a revised patch, which still doesn't change anything in this case but is more cowardly.

Created attachment 66005 [details] [review]
Avoid bogus memory error for tilingPattern

If You have a look into this new patch, I just return gFalse if the resulting tiling pattern bitmap reaches a memory limit and so the Gfx implemention of tiling patterns will be used. I think that this is an acceptable approach also concerning performance, because it would take also a while to paint such a huge bitmap and then draw it to splash.

Pushed, only to master as it did not seem critical.