From 816aa7aec26a0d149ac065f12455e8679184c68b Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 1 Jun 2021 08:51:40 +0700 Subject: [PATCH] MDL-55243 files: Make is_valid_image support SVG files --- lib/filelib.php | 18 +++- lib/filestorage/file_system.php | 61 +++++++++++--- lib/filestorage/tests/file_system_test.php | 83 +++++++++++++++++++ lib/filestorage/tests/fixtures/testimage.svg | 14 ++++ .../tests/fixtures/testimage_error.svg | 13 +++ .../tests/fixtures/testimage_viewbox.svg | 14 ++++ .../tests/fixtures/testimage_width_height.svg | 14 ++++ 7 files changed, 205 insertions(+), 12 deletions(-) create mode 100644 lib/filestorage/tests/fixtures/testimage.svg create mode 100644 lib/filestorage/tests/fixtures/testimage_error.svg create mode 100644 lib/filestorage/tests/fixtures/testimage_viewbox.svg create mode 100644 lib/filestorage/tests/fixtures/testimage_width_height.svg diff --git a/lib/filelib.php b/lib/filelib.php index 5e6849a9c93..700fc50dc1d 100644 --- a/lib/filelib.php +++ b/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 * diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php index 2339c13bd80..b196c824efd 100644 --- a/lib/filestorage/file_system.php +++ b/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. - * @return array Containing width, height, and mimetype. + * @param string $path The full path of the image file. + * @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)) { - return false; // Nothing to process, the file was not recognised as image by GD. + if (!$issvgimage) { + $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; diff --git a/lib/filestorage/tests/file_system_test.php b/lib/filestorage/tests/file_system_test.php index c85f572c24f..47159a78c90 100644 --- a/lib/filestorage/tests/file_system_test.php +++ b/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. * diff --git a/lib/filestorage/tests/fixtures/testimage.svg b/lib/filestorage/tests/fixtures/testimage.svg new file mode 100644 index 00000000000..52e7af0d493 --- /dev/null +++ b/lib/filestorage/tests/fixtures/testimage.svg @@ -0,0 +1,14 @@ + + ]> + + + + + + + + + + + diff --git a/lib/filestorage/tests/fixtures/testimage_error.svg b/lib/filestorage/tests/fixtures/testimage_error.svg new file mode 100644 index 00000000000..78ef6276e4b --- /dev/null +++ b/lib/filestorage/tests/fixtures/testimage_error.svg @@ -0,0 +1,13 @@ + + ]> + + + + + + + + + + diff --git a/lib/filestorage/tests/fixtures/testimage_viewbox.svg b/lib/filestorage/tests/fixtures/testimage_viewbox.svg new file mode 100644 index 00000000000..6c931cbc606 --- /dev/null +++ b/lib/filestorage/tests/fixtures/testimage_viewbox.svg @@ -0,0 +1,14 @@ + + ]> + + + + + + + + + + + diff --git a/lib/filestorage/tests/fixtures/testimage_width_height.svg b/lib/filestorage/tests/fixtures/testimage_width_height.svg new file mode 100644 index 00000000000..35e5c43093d --- /dev/null +++ b/lib/filestorage/tests/fixtures/testimage_width_height.svg @@ -0,0 +1,14 @@ + + ]> + + + + + + + + + + +