Copied to clipboard

Flag this post as spam?

This post will be reported to the moderators as potential spam to be looked at


  • jeffrey@umarketingsuite.com 359 posts 1760 karma points MVP 4x c-trib
    Jun 23, 2015 @ 13:29
    jeffrey@umarketingsuite.com
    0

    Adding nosniff-header breaks PNG-images in Internet Explorer

    I've found it always quite "interesting" that when you upload a .png-file in the media-section or richtext editor of Umbraco, Umbraco would create a .jpg-thumbnail.

    This was sometimes a bit annoying because you had to do some filename-rewriting when using in code, but today we added some headers to our website for security reasons. According to OWASP you should add the X-Content-Type-Options: nosniff (https://www.owasp.org/index.php/ListofusefulHTTPheaders) to all your output.

    An example of how this works is shown on these links (see the difference of handling in Chrome and Internet Explorer vs Firefox):

    Some good info about the no-sniff header is found here.

    As far as my knowledge goes it looks at the mimetype that would be expected based on the fileextension and looks at the real mimetype of the file.

    When we added these recommended headers because of a recent security check we had, we've suddenly got complaining customers who use Internet Explorer 11. Half of all pictures weren't shown anymore. After a lot of research it turned out to be the resized images in Umbraco that weren't shown anymore. Apparently the mime-type of the thumbnails isn't correct. I've setup a demo-site, where these headers are still active, and put on an original png-image and the thumbnailed .jpg:

    Also if you try to open the files from disk with a program like Photoshop or IrfanView it will give the message that the file has an incorrect extension.

    Removing these headers is a short-term solution for now, but eventually we have to put in this header because of security regulations.

    1) Seems like the thumbnail-handling isn't entirely correct in Umbraco? 2) Am I doing something wrong? 3) Is there an easy fix? Or does anybody has any experience with putting in these headers?

    I've tried to be as complete as possible, greetings,

    Jeffrey

    p.s.: I think the .jpg-thumbnail-creation started in Umbraco 6.x and further. The example images were thumbnailed by a 7.2 Umbraco install.

  • James Jackson-South 489 posts 1742 karma points c-trib
    Jun 23, 2015 @ 14:10
    James Jackson-South
    0

    Hi Jeffrey,

    Looking of the first 4 bytes of the thumbnail image it is definitely a png file so there is something wrong with the way thumbnails are created. I'm gonna have to have a dig through the source to see exactly what is happening. To me, this is a bug.

    There is a way round your issue though that you could enact in the interim until a fix is written and that is to tap into the media service events and use ImageProcessor to resave the images in the correct format.

    ImageProcessor comes bundled with Umbraco as of version 7.1.4 but they are only using it for certain functionality at the moment.

    I'm gonna drop a whole ton of code at you now that should solve your problem for new images. For old images you will have to go through and re-save to trigger this event.

    namespace Demo
    {
        using System;
        using System.Collections.Concurrent;
        using System.Collections.Generic;
        using System.Drawing;
        using System.IO;
        using System.Linq;
        using System.Web.Helpers;
    
        using ImageProcessor;
        using ImageProcessor.Imaging;
        using ImageProcessor.Imaging.Formats;
    
        using Umbraco.Core;
        using Umbraco.Core.Configuration;
        using Umbraco.Core.Configuration.UmbracoSettings;
        using Umbraco.Core.IO;
        using Umbraco.Core.Logging;
        using Umbraco.Core.Models;
        using Umbraco.Core.Services;
    
        /// <summary>
        /// Attaches ImageProcessor to the Media.Saving event to ensure that uploaded files are saved to the correct format.
        /// </summary>
        public class ImageProcessorPreProcessor : ApplicationEventHandler
        {
            /// <summary>
            /// Allows us to store values over the two events.
            /// </summary>
            private static readonly ConcurrentDictionary<string, long> SizeCache = new ConcurrentDictionary<string, long>();
    
            /// <summary>
            /// Overridable method to execute when All resolvers have been initialized but resolution is not 
            /// frozen so they can be modified in this method
            /// </summary>
            /// <param name="umbracoApplication">The current <see cref="UmbracoApplicationBase"/></param>
            /// <param name="applicationContext">The Umbraco <see cref="ApplicationContext"/> for the current application.</param>
            protected override void ApplicationStarting(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext)
            {
                const string UmbracoFile = Constants.Conventions.Media.File;
    
                // Tap into the Saving event
                MediaService.Saving += (sender, args) =>
                {
                    MediaFileSystem mediaFileSystem = FileSystemProviderManager.Current.GetFileSystemProvider<MediaFileSystem>();
                    IContentSection contentSection = UmbracoConfig.For.UmbracoSettings().Content;
                    IEnumerable<string> supportedTypes = contentSection.ImageFileTypes.ToList();
    
                    foreach (IMedia media in args.SavedEntities)
                    {
                        if (media.HasProperty(UmbracoFile))
                        {
                            string path = media.GetValue<string>(UmbracoFile);
    
                            try
                            {
                                // Make sure it's not empty.
                                if (string.IsNullOrWhiteSpace(path))
                                {
                                    return;
                                }
    
                                // This might happen after changing the data type of umbracoFile from 
                                // Image Cropper back to Upload. 
                                if (path.Contains("{"))
                                {
                                    dynamic cropper = Json.Decode(path);
                                    string src = cropper.Src;
                                    if (!string.IsNullOrWhiteSpace(src))
                                    {
                                        path = src;
                                    }
                                    else
                                    {
                                        return;
                                    }
                                }
    
                                // Make sure it's an image.
                                string extension = Path.GetExtension(path).Substring(1);
                                if (supportedTypes.InvariantContains(extension) && (extension.InvariantEquals("jpeg") || extension.InvariantEquals("jpg")))
                                {
                                    // Resave the image to the correct format
                                    string fullPath = mediaFileSystem.GetFullPath(path);
                                    using (ImageFactory imageFactory = new ImageFactory(true))
                                    {
                                        // Load
                                        imageFactory.Load(fullPath);
    
                                        // Check and set to correct format.
                                        // Apply a background colour here.
                                        if (imageFactory.CurrentImageFormat.GetType() != typeof(JpegFormat))
                                        {
                                            imageFactory.Format(new JpegFormat()).BackgroundColor(Color.White);
                                        }
    
                                        // Save
                                        imageFactory.Save(fullPath);
    
                                        // Add new info to the cache
                                        FileInfo info = new FileInfo(fullPath);
                                        long bytes = info.Length;
                                        SizeCache.AddOrUpdate(path, bytes, (p, b) => bytes);
                                    }
                                }
                            }
                            catch (Exception ex)
                            {
                                // For all unexpected errors we get a hint in the log file.
                                LogHelper.Error<ImageProcessorPreProcessor>(
                                    string.Format("Could not resize image. Path: {0}", path), ex);
                            }
                        }
                    }
                };
    
                // Update the meta information on Saved.
                MediaService.Saved += (sender, args) =>
                {
                    foreach (IMedia media in args.SavedEntities)
                    {
                        if (media.HasProperty(UmbracoFile))
                        {
                            string path = media.GetValue<string>(UmbracoFile);
    
                            if (SizeCache.ContainsKey(path))
                            {
                                long bytes = SizeCache[path];
                                media.SetValue(Constants.Conventions.Media.Bytes, bytes);
                                sender.Save(media, 0, false);
                            }
                        }
                    }
                };
            }
        }
    }
    

    The code is fairly simple (though untested). I'm ensuring I'm dealing with a media file and resaving it using ImageProcessor while ensuring the format for the thumbnail is jpeg. Afterwards I'm updating the file size in the back office to ensure it is accurate.

    Hopefully that should help matters.

    James

  • jeffrey@umarketingsuite.com 359 posts 1760 karma points MVP 4x c-trib
    Jun 24, 2015 @ 08:31
    jeffrey@umarketingsuite.com
    0

    Hi James,

    thanks for your extensive answer! I've added the code to my testproject but still if I upload my .png-file (http://downloads.perplex.eu/nosniffheader/ok-icon.png) it will generate a .jpg with .png header, so the issue still remains.

    If I step through the code this part will return a false:

    if (supportedTypes.InvariantContains(extension) && (extension.InvariantEquals("jpeg") || extension.InvariantEquals("jpg")))
    

    because it is a .png and not a jpeg or jpg.

    I also thought that the thumbnailing in Umbraco was done by ImageProcessor, but it seems like it doesn't? What are they using instead?

    Jeffrey

  • James Jackson-South 489 posts 1742 karma points c-trib
    Jun 24, 2015 @ 09:42
    James Jackson-South
    0

    Hi Jeffrey,

    Is that for the thumbnail only? Looks like we'll have to run some tests to figure out exactly when the thumbnails are saved then and re-save them once we figure that out. The crux of the code is correct though. I'm a bit swamped at the moment so I don't know if I will be able to do it before the weekend.

    ImageProcessor runs the image cropper data type only as far as I know just now. I think thumbnail generation still depends on legacy code. This is most probably because the signature for ImageFactory.Format changed between version 1 and 2 and they are still using the old version which is a shame.

    Cheers

    James

  • jeffrey@umarketingsuite.com 359 posts 1760 karma points MVP 4x c-trib
    Jun 24, 2015 @ 13:55
    jeffrey@umarketingsuite.com
    1

    Hi James,

    this code will only run once (for the actual .png image) and not for a second time for creating the thumbnails.

    I think the main bug in the core is in this class: https://github.com/umbraco/Umbraco-CMS/blob/7c4a189aa3cf583954defd9c43a3e55e325f2c3f/src/Umbraco.Core/Media/ImageHelper.cs. I think that is the class that does the thumbnailing.

    In the function "ResizedImage" (line 122) there's always a .jpg-file as proposed filename for the thumbnail. That could be made more flexible by using the variable "extension" into account. For example:

    Old code:

     var fileNameThumb = String.IsNullOrEmpty(fileNameAddition)
                                                ? string.Format("{0}_UMBRACOSYSTHUMBNAIL.jpg", path.Substring(0, path.LastIndexOf(".")))
                                                : string.Format("{0}_{1}.jpg", path.Substring(0, path.LastIndexOf(".")), fileNameAddition);
    

    Proposed code:

     var fileNameThumb = String.IsNullOrEmpty(fileNameAddition)
                                                ? string.Format("{0}_UMBRACOSYSTHUMBNAIL." + extension, path.Substring(0, path.LastIndexOf(".")))
                                                : string.Format("{0}_{1}." + extension, path.Substring(0, path.LastIndexOf(".")), fileNameAddition);
    

    Or we could put in some code in line 206:

    var codec = extension.ToLower() == "png" || extension.ToLower() == "gif"
                            ? imageEncoders.Single(t => t.MimeType.Equals("image/png"))
                            : imageEncoders.Single(t => t.MimeType.Equals("image/jpeg"));
    

    I did a local test and if you just save the .png-thumbnail as .png file and that works perfectly (http://downloads.perplex.eu/nosniffheader/test_thumbnail.png)

    But I'm not a thumbnailing-expert (and you are ;)) so I don't know if the code should be optimized all together, and if we're breaking other stuff in Umbraco.

    Jeffrey

  • James Jackson-South 489 posts 1742 karma points c-trib
    Jun 25, 2015 @ 08:22
    James Jackson-South
    0

    Hi Jeffrey,

    I'd probably do both.

    I would double check to see that they are not expecting a jpeg further down the line. If that's cool then what you've done should be great. :)

    Except:

    var codec = extension.ToLower() == "png" || extension.ToLower() == "gif"
                        ? imageEncoders.Single(t => t.MimeType.Equals("image/png"))
                        : imageEncoders.Single(t => t.MimeType.Equals("image/jpeg"));
    

    You should probably turn the tertiary condition into a switch statement as you are now setting the wrong encoder for gifs.

    Cheers

    James

  • jeffrey@umarketingsuite.com 359 posts 1760 karma points MVP 4x c-trib
    Jun 26, 2015 @ 14:25
    jeffrey@umarketingsuite.com
    0

    Hi James,

    thanks for the reply again. I was too busy the last couple of days to investigate it further or make a pull request to fix this. I am leaving this evening for a well deserved holiday for two weeks, but when I return I will look deeper into this.

    Adios, Jeffrey

  • jeffrey@umarketingsuite.com 359 posts 1760 karma points MVP 4x c-trib
    Jul 24, 2015 @ 15:11
  • James Jackson-South 489 posts 1742 karma points c-trib
    Jul 24, 2015 @ 15:42
    James Jackson-South
    0

    Great stuff! :)

Please Sign in or register to post replies

Write your reply to:

Draft