MDL-55243 files: Make is_valid_image support SVG files

lib/filelib.php

@@ -2529,6 +2529,12 @@ function send_file($path, $filename, $lifetime = null , $filter=0, $pathisstring
         $filename = rawurlencode($filename);
     }
 
+    // We need to force download and force filter the file content for the SVG file.
+    if (file_is_svg_image_from_mimetype($mimetype)) {
+        $forcedownload = true;
+        $filter = 1;
+    }
+
     if ($forcedownload) {
         header('Content-Disposition: attachment; filename="'.$filename.'"');
 
@@ -2589,7 +2595,7 @@ function send_file($path, $filename, $lifetime = null , $filter=0, $pathisstring
 
     } else {
         // Try to put the file through filters
-        if ($mimetype == 'text/html' || $mimetype == 'application/xhtml+xml') {
+        if ($mimetype == 'text/html' || $mimetype == 'application/xhtml+xml' || file_is_svg_image_from_mimetype($mimetype)) {
             $options = new stdClass();
             $options->noclean = true;
             $options->nocache = true; // temporary workaround for MDL-5136
@@ -3018,6 +3024,16 @@ function file_merge_draft_area_into_draft_area($getfromdraftid, $mergeintodrafti
     }
 }
 
+/**
+ * Attempt to determine whether the specified mime-type is an SVG image or not.
+ *
+ * @param string $mimetype Mime-type
+ * @return bool True if it is an SVG file
+ */
+function file_is_svg_image_from_mimetype(string $mimetype): bool {
+    return preg_match('|^image/svg|', $mimetype);
+}
+
 /**
  * RESTful cURL class
  *

lib/filestorage/file_system.php

@@ -398,22 +398,61 @@ abstract class file_system {
     /**
      * Returns image information relating to the specified path or URL.
      *
-     * @param string $path The path to pass to getimagesize.
+     * @param string $path The full path of the image file.
-     * @return array Containing width, height, and mimetype.
+     * @return array|bool array that containing width, height, and mimetype or false if cannot get the image info.
      */
     protected function get_imageinfo_from_path($path) {
-        $imageinfo = getimagesize($path);
+        $imagemimetype = file_storage::mimetype_from_file($path);
+        $issvgimage = file_is_svg_image_from_mimetype($imagemimetype);
 
-        if (!is_array($imageinfo)) {
+        if (!$issvgimage) {
-            return false; // Nothing to process, the file was not recognised as image by GD.
+            $imageinfo = getimagesize($path);
+            if (!is_array($imageinfo)) {
+                return false; // Nothing to process, the file was not recognised as image by GD.
+            }
+            $image = [
+                    'width' => $imageinfo[0],
+                    'height' => $imageinfo[1],
+                    'mimetype' => image_type_to_mime_type($imageinfo[2]),
+            ];
+        } else {
+            // Since SVG file is actually an XML file, GD cannot handle.
+            $svgcontent = @simplexml_load_file($path);
+            if (!$svgcontent) {
+                // Cannot parse the file.
+                return false;
+            }
+            $svgattrs = $svgcontent->attributes();
+
+            if (!empty($svgattrs->viewBox)) {
+                // We have viewBox.
+                $viewboxval = explode(' ', $svgattrs->viewBox);
+                $width = intval($viewboxval[2]);
+                $height = intval($viewboxval[3]);
+            } else {
+                // Get the width.
+                if (!empty($svgattrs->width) && intval($svgattrs->width) > 0) {
+                    $width = intval($svgattrs->width);
+                } else {
+                    // Default width.
+                    $width = 800;
+                }
+                // Get the height.
+                if (!empty($svgattrs->height) && intval($svgattrs->height) > 0) {
+                    $height = intval($svgattrs->height);
+                } else {
+                    // Default width.
+                    $height = 600;
+                }
+            }
+
+            $image = [
+                    'width' => $width,
+                    'height' => $height,
+                    'mimetype' => $imagemimetype,
+            ];
         }
 
-        $image = array(
-                'width'     => $imageinfo[0],
-                'height'    => $imageinfo[1],
-                'mimetype'  => image_type_to_mime_type($imageinfo[2]),
-            );
-
         if (empty($image['width']) or empty($image['height']) or empty($image['mimetype'])) {
             // GD can not parse it, sorry.
             return false;

lib/filestorage/tests/file_system_test.php

@@ -940,6 +940,89 @@ class core_files_file_system_testcase extends advanced_testcase {
         $this->assertFalse($result);
     }
 
+    /**
+     * Test that get_imageinfo_from_path returns an appropriate response
+     * for an svg image with viewbox attribute.
+     */
+    public function test_get_imageinfo_from_path_svg_viewbox() {
+        $filepath = __DIR__ . '/fixtures/testimage_viewbox.svg';
+
+        // Get the filesystem mock.
+        $fs = $this->get_testable_mock();
+
+        $method = new ReflectionMethod(file_system::class, 'get_imageinfo_from_path');
+        $method->setAccessible(true);
+        $result = $method->invokeArgs($fs, [$filepath]);
+
+        $this->assertArrayHasKey('width', $result);
+        $this->assertArrayHasKey('height', $result);
+        $this->assertArrayHasKey('mimetype', $result);
+        $this->assertEquals(100, $result['width']);
+        $this->assertEquals(100, $result['height']);
+        $this->assertStringContainsString('image/svg', $result['mimetype']);
+    }
+
+    /**
+     * Test that get_imageinfo_from_path returns an appropriate response
+     * for an svg image with width and height attributes.
+     */
+    public function test_get_imageinfo_from_path_svg_with_width_height() {
+        $filepath = __DIR__ . '/fixtures/testimage_width_height.svg';
+
+        // Get the filesystem mock.
+        $fs = $this->get_testable_mock();
+
+        $method = new ReflectionMethod(file_system::class, 'get_imageinfo_from_path');
+        $method->setAccessible(true);
+        $result = $method->invokeArgs($fs, [$filepath]);
+
+        $this->assertArrayHasKey('width', $result);
+        $this->assertArrayHasKey('height', $result);
+        $this->assertArrayHasKey('mimetype', $result);
+        $this->assertEquals(100, $result['width']);
+        $this->assertEquals(100, $result['height']);
+        $this->assertStringContainsString('image/svg', $result['mimetype']);
+    }
+
+    /**
+     * Test that get_imageinfo_from_path returns an appropriate response
+     * for an svg image without attributes.
+     */
+    public function test_get_imageinfo_from_path_svg_without_attribute() {
+        $filepath = __DIR__ . '/fixtures/testimage.svg';
+
+        // Get the filesystem mock.
+        $fs = $this->get_testable_mock();
+
+        $method = new ReflectionMethod(file_system::class, 'get_imageinfo_from_path');
+        $method->setAccessible(true);
+        $result = $method->invokeArgs($fs, [$filepath]);
+
+        $this->assertArrayHasKey('width', $result);
+        $this->assertArrayHasKey('height', $result);
+        $this->assertArrayHasKey('mimetype', $result);
+        $this->assertEquals(800, $result['width']);
+        $this->assertEquals(600, $result['height']);
+        $this->assertStringContainsString('image/svg', $result['mimetype']);
+    }
+
+    /**
+     * Test that get_imageinfo_from_path returns an appropriate response
+     * for a file which is not an correct svg.
+     */
+    public function test_get_imageinfo_from_path_svg_invalid() {
+        $filepath = __DIR__ . '/fixtures/testimage_error.svg';
+
+        // Get the filesystem mock.
+        $fs = $this->get_testable_mock();
+
+        $method = new ReflectionMethod(file_system::class, 'get_imageinfo_from_path');
+        $method->setAccessible(true);
+        $result = $method->invokeArgs($fs, [$filepath]);
+
+        $this->assertFalse($result);
+    }
+
     /**
      * Ensure that get_content_file_handle returns a valid file handle.
      *

lib/filestorage/tests/fixtures/testimage.svg

@@ -0,0 +1,14 @@
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
+        <!ENTITY ns_flows "http://ns.adobe.com/Flows/1.0/">
+        ]>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g id='gtop' stroke-width="12" stroke="#000">
+        <g id="svgstar" transform="translate(50,50)">
+            <path id="svgbar" d="M-27-5a7,7,0,1,0,0,10h54a7,7,0,1,0,0-10z"/>
+            <use id='use1' xlink:href="#svgbar" transform="rotate(45)"/>
+            <use id='use2' xlink:href="#svgbar" transform="rotate(90)"/>
+            <use id='use3' xlink:href="#svgbar" transform="rotate(135)"/>
+        </g>
+    </g>
+    <use id="usetop" xlink:href="#svgstar" fill="#FB4"/>
+</svg>

lib/filestorage/tests/fixtures/testimage_error.svg

@@ -0,0 +1,13 @@
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
+        <!ENTITY ns_flows "http://ns.adobe.com/Flows/1.0/">
+        ]>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g id='gtop' stroke-width="12" stroke="#000">
+        <g id="svgstar" transform="translate(50,50)">
+            <path id="svgbar" d="M-27-5a7,7,0,1,0,0,10h54a7,7,0,1,0,0-10z"/>
+            <use id='use1' xlink:href="#svgbar" transform="rotate(45)"/>
+            <use id='use2' xlink:href="#svgbar" transform="rotate(90)"/>
+            <use id='use3' xlink:href="#svgbar" transform="rotate(135)"/>
+    </g>
+    <use id="usetop" xlink:href="#svgstar" fill="#FB4"/>
+</svg>

lib/filestorage/tests/fixtures/testimage_viewbox.svg

@@ -0,0 +1,14 @@
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
+        <!ENTITY ns_flows "http://ns.adobe.com/Flows/1.0/">
+        ]>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 100 100">
+    <g id='gtop' stroke-width="12" stroke="#000">
+        <g id="svgstar" transform="translate(50,50)">
+            <path id="svgbar" d="M-27-5a7,7,0,1,0,0,10h54a7,7,0,1,0,0-10z"/>
+            <use id='use1' xlink:href="#svgbar" transform="rotate(45)"/>
+            <use id='use2' xlink:href="#svgbar" transform="rotate(90)"/>
+            <use id='use3' xlink:href="#svgbar" transform="rotate(135)"/>
+        </g>
+    </g>
+    <use id="usetop" xlink:href="#svgstar" fill="#FB4"/>
+</svg>

lib/filestorage/tests/fixtures/testimage_width_height.svg

@@ -0,0 +1,14 @@
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [
+        <!ENTITY ns_flows "http://ns.adobe.com/Flows/1.0/">
+        ]>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="100px" height="100px">
+    <g id='gtop' stroke-width="12" stroke="#000">
+        <g id="svgstar" transform="translate(50,50)">
+            <path id="svgbar" d="M-27-5a7,7,0,1,0,0,10h54a7,7,0,1,0,0-10z"/>
+            <use id='use1' xlink:href="#svgbar" transform="rotate(45)"/>
+            <use id='use2' xlink:href="#svgbar" transform="rotate(90)"/>
+            <use id='use3' xlink:href="#svgbar" transform="rotate(135)"/>
+        </g>
+    </g>
+    <use id="usetop" xlink:href="#svgstar" fill="#FB4"/>
+</svg>