ResourceLoaderImage: tighten up svg handling
authorChad Horohoe <chadh@wikimedia.org>
Wed, 18 Feb 2015 19:19:38 +0000 (11:19 -0800)
committerChad Horohoe <chadh@wikimedia.org>
Wed, 18 Feb 2015 19:37:43 +0000 (11:37 -0800)
- Check $wgSVGConverter to see if it starts with rsvg, instead
  of just being rsvg. Other things like rsvg-secure are also ok.
- Make sure SvgHandler::rasterize() returned sanely before attempting
  to use the file it produces.
- Clean up temp SVG file if we return early
- Add some debug logging when rasterization fails

Bug: T89505
Change-Id: I9483c8c54a30e328565182b00d50dbf3b83076cd

includes/resourceloader/ResourceLoaderImage.php

index 6788c6f..c6e345f 100644 (file)
@@ -200,18 +200,22 @@ class ResourceLoaderImage {
                        $format = $context->getFormat();
                }
 
+               $path = $this->getPath( $context );
                if ( $this->getExtension() !== 'svg' ) {
-                       return file_get_contents( $this->getPath( $context ) );
+                       return file_get_contents( $path );
                }
 
                if ( $variant && isset( $this->variants[$variant] ) ) {
                        $data = $this->variantize( $this->variants[$variant], $context );
                } else {
-                       $data = file_get_contents( $this->getPath( $context ) );
+                       $data = file_get_contents( $path );
                }
 
                if ( $format === 'rasterized' ) {
                        $data = $this->rasterize( $data );
+                       if ( !$data ) {
+                               wfDebugLog( 'ResourceLoaderImage', __METHOD__  . " failed to rasterize for $path" );
+                       }
                }
 
                return $data;
@@ -307,7 +311,8 @@ class ResourceLoaderImage {
 
                $svg = $this->massageSvgPathdata( $svg );
 
-               if ( $wgSVGConverter === 'rsvg' ) {
+               // Sometimes this might be 'rsvg-secure'. Long as it's rsvg.
+               if ( strpos( $wgSVGConverter, 'rsvg' ) === 0 ) {
                        $command = 'rsvg-convert';
                        if ( $wgSVGConverterPath ) {
                                $command = wfEscapeShellArg( "$wgSVGConverterPath/" ) . $command;
@@ -339,16 +344,19 @@ class ResourceLoaderImage {
 
                        $metadata = SVGMetadataExtractor::getMetadata( $tempFilenameSvg );
                        if ( !isset( $metadata['width'] ) || !isset( $metadata['height'] ) ) {
+                               unlink( $tempFilenameSvg );
                                return false;
                        }
 
                        $handler = new SvgHandler;
-                       $handler->rasterize( $tempFilenameSvg, $tempFilenamePng, $metadata['width'], $metadata['height'] );
-
-                       $png = file_get_contents( $tempFilenamePng );
-
+                       $res = $handler->rasterize( $tempFilenameSvg, $tempFilenamePng, $metadata['width'], $metadata['height'] );
                        unlink( $tempFilenameSvg );
-                       unlink( $tempFilenamePng );
+
+                       $png = null;
+                       if ( $res === true ) {
+                               $png = file_get_contents( $tempFilenamePng );
+                               unlink( $tempFilenamePng );
+                       }
 
                        return $png ?: false;
                }