Uploading SVG's Causes Error (Depending on imageFileTypes Setting) (#3072)

src/Umbraco.Core/Configuration/UmbracoSettings/IContentSection.cs

@@ -13,7 +13,7 @@ namespace Umbraco.Core.Configuration.UmbracoSettings
         IEnumerable<string> ImageTagAllowedAttributes { get; }
 
         IEnumerable<IImagingAutoFillUploadField> ImageAutoFillProperties { get; }
-
+        
         string ScriptFolderPath { get; }
 
         IEnumerable<string> ScriptFileTypes { get; }

src/Umbraco.Core/Constants-Conventions.cs

@@ -110,6 +110,11 @@ namespace Umbraco.Core
 				/// Property alias for the Media's file extension.
 				/// </summary>
 				public const string Extension = "umbracoExtension";
+
+				/// <summary>
+				/// The default height/width of an image file if the size can't be determined from the metadata
+				/// </summary>
+				public const int DefaultSize = 200;
 			}
 
 			/// <summary>
@@ -354,4 +359,4 @@ namespace Umbraco.Core
             }
 		}
 	}
-}
+}

src/Umbraco.Core/IO/MediaFileSystem.cs

@@ -17,15 +17,15 @@ using Umbraco.Core.Models;
 
 namespace Umbraco.Core.IO
 {
-	/// <summary>
-	/// A custom file system provider for media
-	/// </summary>
-	[FileSystemProvider("media")]
-	public class MediaFileSystem : FileSystemWrapper
-	{
-	    private readonly IContentSection _contentConfig;
+    /// <summary>
+    /// A custom file system provider for media
+    /// </summary>
+    [FileSystemProvider("media")]
+    public class MediaFileSystem : FileSystemWrapper
+    {
+        private readonly IContentSection _contentConfig;
         private readonly UploadAutoFillProperties _uploadAutoFillProperties;
-	    private readonly ILogger _logger;
+        private readonly ILogger _logger;
 
         private readonly object _folderCounterLock = new object();
         private long _folderCounter;
@@ -39,8 +39,8 @@ namespace Umbraco.Core.IO
         };
 
         public MediaFileSystem(IFileSystem wrapped)
-			: this(wrapped, UmbracoConfig.For.UmbracoSettings().Content, ApplicationContext.Current.ProfilingLogger.Logger)
-		{ }
+            : this(wrapped, UmbracoConfig.For.UmbracoSettings().Content, ApplicationContext.Current.ProfilingLogger.Logger)
+        { }
 
         public MediaFileSystem(IFileSystem wrapped, IContentSection contentConfig, ILogger logger)
             : base(wrapped)
@@ -60,13 +60,13 @@ namespace Umbraco.Core.IO
 
         [Obsolete("This low-level method should NOT exist.")]
         public string GetRelativePath(int propertyId, string fileName)
-		{
+        {
             var sep = _contentConfig.UploadAllowDirectories
-				? Path.DirectorySeparatorChar
-				: '-';
+                ? Path.DirectorySeparatorChar
+                : '-';
 
-			return propertyId.ToString(CultureInfo.InvariantCulture) + sep + fileName;
-		}
+            return propertyId.ToString(CultureInfo.InvariantCulture) + sep + fileName;
+        }
 
         [Obsolete("This low-level method should NOT exist.", false)]
         public string GetRelativePath(string subfolder, string fileName)
@@ -264,7 +264,7 @@ namespace Umbraco.Core.IO
             var filename = Path.GetFileName(sourcepath);
             var filepath = GetMediaPath(filename, content.Key, propertyType.Key);
             this.CopyFile(sourcepath, filepath);
-            
+
             return filepath;
         }
 
@@ -321,7 +321,7 @@ namespace Umbraco.Core.IO
         /// <param name="filepath"></param>
         /// <param name="filestream"></param>
         private void SetUploadFile(IContentBase content, Property property, string filepath, Stream filestream)
-        {            
+        {
             // will use filepath for extension, and filestream for length
             _uploadAutoFillProperties.Populate(content, property.Alias, filepath, filestream);
         }
@@ -368,19 +368,20 @@ namespace Umbraco.Core.IO
                         return new Size(width, height);
                     }
                 }
+
+                //we have no choice but to try to read in via GDI
+                using (var image = Image.FromStream(stream))
+                {
+
+                    var fileWidth = image.Width;
+                    var fileHeight = image.Height;
+                    return new Size(fileWidth, fileHeight);
+                }
             }
             catch (Exception)
             {
                 //We will just swallow, just means we can't read exif data, we don't want to log an error either
-            }
-
-            //we have no choice but to try to read in via GDI
-            using (var image = Image.FromStream(stream))
-            {
-
-                var fileWidth = image.Width;
-                var fileHeight = image.Height;
-                return new Size(fileWidth, fileHeight);
+                return new Size(Constants.Conventions.Media.DefaultSize, Constants.Conventions.Media.DefaultSize);
             }
         }
 
@@ -430,8 +431,8 @@ namespace Umbraco.Core.IO
             }
         }
 
-	    public void DeleteMediaFiles(IEnumerable<string> files)
-	    {
+        public void DeleteMediaFiles(IEnumerable<string> files)
+        {
             files = files.Distinct();
 
             Parallel.ForEach(files, file =>

src/Umbraco.Core/Media/Exif/ImageFile.cs

@@ -2,6 +2,7 @@
 using System.Drawing;
 using System.IO;
 using System.Text;
+using Umbraco.Core.Media.TypeDetector;
 
 namespace Umbraco.Core.Media.Exif
 {
@@ -118,22 +119,25 @@ namespace Umbraco.Core.Media.Exif
 		/// <returns>The <see cref="ImageFile"/> created from the file.</returns>
         public static ImageFile FromStream(Stream stream, Encoding encoding)
 		{
-			stream.Seek (0, SeekOrigin.Begin);
-			byte[] header = new byte[8];
-			stream.Seek (0, SeekOrigin.Begin);
-			if (stream.Read (header, 0, header.Length) != header.Length)
-				throw new NotValidImageFileException ();
-			
-			// JPEG
-			if (header[0] == 0xFF && header[1] == 0xD8)
-				return new JPEGFile (stream, encoding);
-			
-			// TIFF
-			string tiffHeader = Encoding.ASCII.GetString (header, 0, 4);
-			if (tiffHeader == "MM\x00\x2a" || tiffHeader == "II\x2a\x00")
-				return new TIFFFile (stream, encoding);
-			
-			throw new NotValidImageFileException ();
+            // JPEG
+            if(JPEGDetector.IsOfType(stream))
+            {
+                return new JPEGFile(stream, encoding);
+            }
+
+            // TIFF
+            if (TIFFDetector.IsOfType(stream))
+            {
+                return new TIFFFile(stream, encoding);
+            }
+
+            // SVG
+            if (SVGDetector.IsOfType(stream))
+            {
+                return new SVGFile(stream);
+            }
+
+            throw new NotValidImageFileException ();
 		}
 		#endregion
 	}

src/Umbraco.Core/Media/Exif/ImageFileFormat.cs

@@ -17,5 +17,9 @@
         /// The file is a TIFF File.
         /// </summary>
         TIFF,
+        /// <summary>
+        /// The file is a SVG File.
+        /// </summary>
+        SVG,
     }
 }

src/Umbraco.Core/Media/Exif/SVGFile.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Drawing;
+using System.IO;
+using System.Linq;
+using System.Xml.Linq;
+
+namespace Umbraco.Core.Media.Exif
+{
+    internal class SVGFile : ImageFile
+    {
+        public SVGFile(Stream fileStream)
+        {
+            fileStream.Position = 0;
+
+            var document = XDocument.Load(fileStream); //if it throws an exception the ugly try catch in MediaFileSystem will catch it
+
+            var width = document.Root?.Attributes().Where(x => x.Name == "width").Select(x => x.Value).FirstOrDefault();
+            var height = document.Root?.Attributes().Where(x => x.Name == "height").Select(x => x.Value).FirstOrDefault();
+
+            Properties.Add(new ExifSInt(ExifTag.PixelYDimension,
+                height == null ? Constants.Conventions.Media.DefaultSize : int.Parse(height)));
+            Properties.Add(new ExifSInt(ExifTag.PixelXDimension,
+                width == null ? Constants.Conventions.Media.DefaultSize : int.Parse(width)));
+            
+            Format = ImageFileFormat.SVG;
+        }
+
+        public override void Save(Stream stream)
+        {
+        }
+
+        public override Image ToImage()
+        {
+            throw new NotImplementedException();
+        }
+    }
+}

src/Umbraco.Core/Media/TypeDetector/JpegDetector.cs

@@ -0,0 +1,14 @@
+using System.IO;
+
+namespace Umbraco.Core.Media.TypeDetector
+{
+    public class JPEGDetector : RasterizedTypeDetector
+    {
+        public static bool IsOfType(Stream fileStream)
+        {
+            var header = GetFileHeader(fileStream);
+
+            return header[0] == 0xff && header[1] == 0xD8;
+        }
+    }
+}

src/Umbraco.Core/Media/TypeDetector/RasterizedTypeDetector.cs

@@ -0,0 +1,16 @@
+using System.IO;
+
+namespace Umbraco.Core.Media.TypeDetector
+{
+    public abstract class RasterizedTypeDetector
+    {
+        public static byte[] GetFileHeader(Stream fileStream)
+        {
+            fileStream.Seek(0, SeekOrigin.Begin);
+            byte[] header = new byte[8];
+            fileStream.Seek(0, SeekOrigin.Begin);
+
+            return header;
+        }
+    }
+}

src/Umbraco.Core/Media/TypeDetector/SVGDetector.cs

@@ -0,0 +1,24 @@
+using System.IO;
+using System.Xml.Linq;
+
+namespace Umbraco.Core.Media.TypeDetector
+{
+    public class SVGDetector
+    {
+        public static bool IsOfType(Stream fileStream)
+        {
+            var document = new XDocument();
+
+            try
+            {
+                document = XDocument.Load(fileStream);
+            }
+            catch (System.Exception ex)
+            {
+                return false;
+            }
+
+            return document.Root?.Name.LocalName == "svg";
+        }
+    }
+}

src/Umbraco.Core/Media/TypeDetector/TIFFDetector.cs

@@ -0,0 +1,24 @@
+using System.IO;
+using System.Text;
+
+namespace Umbraco.Core.Media.TypeDetector
+{
+    public class TIFFDetector
+    {
+        public static bool IsOfType(Stream fileStream)
+        {
+            string tiffHeader = GetFileHeader(fileStream);
+
+            return tiffHeader == "MM\x00\x2a" || tiffHeader == "II\x2a\x00";
+        }
+
+        public static string GetFileHeader(Stream fileStream)
+        {
+            var header = RasterizedTypeDetector.GetFileHeader(fileStream);
+
+            string tiffHeader = Encoding.ASCII.GetString(header, 0, 4);
+
+            return tiffHeader;
+        }
+    }
+}

src/Umbraco.Core/Umbraco.Core.csproj

@@ -365,6 +365,11 @@
     <Compile Include="IEmailSender.cs" />
     <Compile Include="IHttpContextAccessor.cs" />
     <Compile Include="Logging\RollingFileCleanupAppender.cs" />
+    <Compile Include="Media\Exif\SVGFile.cs" />
+    <Compile Include="Media\TypeDetector\JPEGDetector.cs" />
+    <Compile Include="Media\TypeDetector\RasterizedTypeDetector.cs" />
+    <Compile Include="Media\TypeDetector\SVGDetector.cs" />
+    <Compile Include="Media\TypeDetector\TIFFDetector.cs" />
     <Compile Include="Models\AuditEntry.cs" />
     <Compile Include="Models\Consent.cs" />
     <Compile Include="Models\ConsentExtensions.cs" />

src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js

@@ -31,10 +31,10 @@ angular.module("umbraco.directives")
 					//elements
 					var $viewport = element.find(".viewport");
 					var $image = element.find("img");
-					var $overlay = element.find(".overlay");
+                    var $overlay = element.find(".overlay");
 
-					scope.style = function () {
-						if(scope.dimensions.width <= 0){
+				    scope.style = function () {
+                        if (scope.dimensions.width <= 0) {
 							setDimensions();
 						}
 
@@ -45,23 +45,29 @@ angular.module("umbraco.directives")
 					};
 
 					scope.setFocalPoint = function(event) {
+					    scope.$emit("imageFocalPointStart");
 
-						scope.$emit("imageFocalPointStart");
+					    var offsetX = event.offsetX - 10;
+					    var offsetY = event.offsetY - 10;
 
-						var offsetX = event.offsetX - 10;
-						var offsetY = event.offsetY - 10;
-
-						calculateGravity(offsetX, offsetY);
-
-						lazyEndEvent();
+					    calculateGravity(offsetX, offsetY);
 
+					    lazyEndEvent();
 					};
 
-					var setDimensions = function(){
-						scope.dimensions.width = $image.width();
-						scope.dimensions.height = $image.height();
-
-						if(scope.center){
+                    var setDimensions = function () {
+                        if (scope.src.endsWith(".svg")) {
+                            // svg files don't automatically get a size by
+                            // loading them set a default size for now
+                            $image.attr("width", "200");
+                            $image.attr("height", "200");
+                            // can't crop an svg file, don't show the focal point
+                            $overlay.remove();
+                        }
+					    scope.dimensions.width = $image.width();
+					    scope.dimensions.height = $image.height();
+                        
+					    if(scope.center){
 							scope.dimensions.left =  scope.center.left * scope.dimensions.width -10;
 							scope.dimensions.top =  scope.center.top * scope.dimensions.height -10;
 						}else{

src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.html

@@ -46,8 +46,8 @@
                 </umb-image-gravity>
                 <a href class="btn btn-link btn-crop-delete" ng-click="clear()"><i class="icon-delete red"></i> <localize key="content_uploadClear">Remove file</localize></a>
             </div>
-
-            <ul class="umb-sortable-thumbnails cropList clearfix">
+            
+            <ul ng-if="!imageSrc.endsWith('.svg')" class="umb-sortable-thumbnails cropList clearfix">
                 <li ng-repeat=" (key,value) in model.value.crops" ng-class="{'current':currentCrop.alias === value.alias}" ng-click="crop(value)">
 
                     <umb-image-thumbnail center="model.value.focalPoint"

src/Umbraco.Web.UI/config/umbracoSettings.config

@@ -8,7 +8,7 @@
   <content>
     <imaging>
       <!-- what file extension that should cause umbraco to create thumbnails -->
-      <imageFileTypes>jpeg,jpg,gif,bmp,png,tiff,tif</imageFileTypes>
+      <imageFileTypes>jpeg,jpg,gif,bmp,png,tiff,tif,svg</imageFileTypes>
       <!-- what attributes that are allowed in the editor on an img tag -->
       <allowedAttributes>src,alt,border,class,style,align,id,name,onclick,usemap</allowedAttributes>
       <!-- automatically updates dimension, filesize and extension attributes on upload -->
@@ -105,7 +105,7 @@
     <MacroErrors>throw</MacroErrors>
     
     <!-- These file types will not be allowed to be uploaded via the upload control for media and content -->
-    <disallowedUploadFiles>ashx,aspx,ascx,config,cshtml,vbhtml,asmx,air,axd,swf,xml,xhtml,html,htm,svg,php,htaccess</disallowedUploadFiles>
+    <disallowedUploadFiles>ashx,aspx,ascx,config,cshtml,vbhtml,asmx,air,axd,swf,xml,xhtml,html,htm,php,htaccess</disallowedUploadFiles>
 
     <!-- If completed, only the file extensions listed below will be allowed to be uploaded.  If empty, disallowedUploadFiles will apply to prevent upload of specific file extensions. -->
     <allowedUploadFiles></allowedUploadFiles>

src/Umbraco.Web/Editors/MediaController.cs

@@ -942,4 +942,4 @@ namespace Umbraco.Web.Editors
             return hasPathAccess;
         }
     }
-}
+}

src/Umbraco.Web/PropertyEditors/FileUploadPropertyValueEditor.cs

@@ -110,7 +110,7 @@ namespace Umbraco.Web.PropertyEditors
                     _mediaFileSystem.AddFile(filepath, filestream, true); // must overwrite!
 
                     var ext = _mediaFileSystem.GetExtension(filepath);
-                    if (_mediaFileSystem.IsImageFile(ext))
+                    if (_mediaFileSystem.IsImageFile(ext) && ext != ".svg")
                     {
                         var preValues = editorValue.PreValues.FormatAsDictionary();
                         var sizes = preValues.Any() ? preValues.First().Value.Value : string.Empty;
@@ -137,4 +137,4 @@ namespace Umbraco.Web.PropertyEditors
             return string.Join(",", newPaths.Select(x => _mediaFileSystem.GetUrl(x)));
         }
     }
-}
+}

src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs

@@ -154,7 +154,7 @@ namespace Umbraco.Web.PropertyEditors
                 _mediaFileSystem.AddFile(filepath, filestream, true); // must overwrite!
 
                 var ext = _mediaFileSystem.GetExtension(filepath);
-                if (_mediaFileSystem.IsImageFile(ext))
+                if (_mediaFileSystem.IsImageFile(ext) && ext != ".svg")
                 {
                     var preValues = editorValue.PreValues.FormatAsDictionary();
                     var sizes = preValues.Any() ? preValues.First().Value.Value : string.Empty;

src/umbraco.businesslogic/IO/IOHelper.cs

@@ -13,9 +13,9 @@ using umbraco.businesslogic.Exceptions;
 
 namespace umbraco.IO
 {
-	[Obsolete("Use Umbraco.Core.IO.IOHelper instead")]
+    [Obsolete("Use Umbraco.Core.IO.IOHelper instead")]
     public static class IOHelper
-    {        
+    {
         public static char DirSepChar
         {
             get
@@ -27,42 +27,42 @@ namespace umbraco.IO
         //helper to try and match the old path to a new virtual one
         public static string FindFile(string virtualPath)
         {
-        	return Umbraco.Core.IO.IOHelper.FindFile(virtualPath);
+            return Umbraco.Core.IO.IOHelper.FindFile(virtualPath);
         }
 
         //Replaces tildes with the root dir
         public static string ResolveUrl(string virtualPath)
         {
-			return Umbraco.Core.IO.IOHelper.ResolveUrl(virtualPath); 
+            return Umbraco.Core.IO.IOHelper.ResolveUrl(virtualPath);
         }
 
-		[Obsolete("Use Umbraco.Web.Templates.TemplateUtilities.ResolveUrlsFromTextString instead, this method on this class will be removed in future versions")]
+        [Obsolete("Use Umbraco.Web.Templates.TemplateUtilities.ResolveUrlsFromTextString instead, this method on this class will be removed in future versions")]
         public static string ResolveUrlsFromTextString(string text)
         {
-			return Umbraco.Core.IO.IOHelper.ResolveUrlsFromTextString(text); 
+            return Umbraco.Core.IO.IOHelper.ResolveUrlsFromTextString(text);
         }
 
         public static string MapPath(string path, bool useHttpContext)
         {
-			return Umbraco.Core.IO.IOHelper.MapPath(path, useHttpContext); 
+            return Umbraco.Core.IO.IOHelper.MapPath(path, useHttpContext);
         }
 
         public static string MapPath(string path)
         {
-			return Umbraco.Core.IO.IOHelper.MapPath(path); 
+            return Umbraco.Core.IO.IOHelper.MapPath(path);
         }
 
         //use a tilde character instead of the complete path
-		[Obsolete("This method is no longer in use and will be removed in future versions")]
+        [Obsolete("This method is no longer in use and will be removed in future versions")]
         public static string returnPath(string settingsKey, string standardPath, bool useTilde)
         {
-			return Umbraco.Core.IO.IOHelper.ReturnPath(settingsKey, standardPath, useTilde); 
+            return Umbraco.Core.IO.IOHelper.ReturnPath(settingsKey, standardPath, useTilde);
         }
 
-		[Obsolete("This method is no longer in use and will be removed in future versions")]
+        [Obsolete("This method is no longer in use and will be removed in future versions")]
         public static string returnPath(string settingsKey, string standardPath)
         {
-			return Umbraco.Core.IO.IOHelper.ReturnPath(settingsKey, standardPath); 
+            return Umbraco.Core.IO.IOHelper.ReturnPath(settingsKey, standardPath);
 
         }
 
@@ -75,12 +75,12 @@ namespace umbraco.IO
         /// <returns>true if valid, throws a FileSecurityException if not</returns>
         public static bool ValidateEditPath(string filePath, string validDir)
         {
-			return Umbraco.Core.IO.IOHelper.ValidateEditPath(filePath, validDir); 
+            return Umbraco.Core.IO.IOHelper.ValidateEditPath(filePath, validDir);
         }
 
-	    public static bool ValidateFileExtension(string filePath, List<string> validFileExtensions)
+        public static bool ValidateFileExtension(string filePath, List<string> validFileExtensions)
         {
-			return Umbraco.Core.IO.IOHelper.ValidateFileExtension(filePath, validFileExtensions); 
+            return Umbraco.Core.IO.IOHelper.ValidateFileExtension(filePath, validFileExtensions);
         }
 
 
@@ -92,7 +92,7 @@ namespace umbraco.IO
         /// <returns></returns>
         private static string getRootDirectorySafe()
         {
-			return Umbraco.Core.IO.IOHelper.GetRootDirectorySafe(); 
+            return Umbraco.Core.IO.IOHelper.GetRootDirectorySafe();
         }
 
     }