Skip to content

Commit

Permalink
Port #14546 to V14 + clean up file upload configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
kjac committed Apr 9, 2024
1 parent ac7e4c1 commit fdf2597
Show file tree
Hide file tree
Showing 7 changed files with 277 additions and 49 deletions.
8 changes: 0 additions & 8 deletions src/Umbraco.Core/PropertyEditors/FileExtensionConfigItem.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ namespace Umbraco.Cms.Core.PropertyEditors;
public class FileUploadConfiguration : IFileExtensionsConfig
{
[ConfigurationField("fileExtensions")]
public List<FileExtensionConfigItem> FileExtensions { get; set; } = new();
public IEnumerable<string> FileExtensions { get; set; } = Enumerable.Empty<string>();
}
2 changes: 1 addition & 1 deletion src/Umbraco.Core/PropertyEditors/IFileExtensionConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ namespace Umbraco.Cms.Core.PropertyEditors;
/// </summary>
public interface IFileExtensionsConfig
{
List<FileExtensionConfigItem> FileExtensions { get; set; }
IEnumerable<string> FileExtensions { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ private bool IsAllowedInDataTypeConfiguration(string extension, object? dataType
{
// If FileExtensions is empty and no allowed extensions have been specified, we allow everything.
// If there are any extensions specified, we need to check that the uploaded extension is one of them.
return fileUploadConfiguration.FileExtensions.IsCollectionEmpty() ||
fileUploadConfiguration.FileExtensions.Any(x => x.Value?.InvariantEquals(extension) ?? false);
return fileUploadConfiguration.FileExtensions.Any() is false ||
fileUploadConfiguration.FileExtensions.Contains(extension);
}

return false;
Expand Down
165 changes: 130 additions & 35 deletions src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright (c) Umbraco.

Check warning on line 1 in src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

❌ New issue: Overall Code Complexity

This module has a mean cyclomatic complexity of 4.22 across 9 functions. The mean complexity threshold is 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.

Check notice on line 1 in src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

✅ Getting better: Missing Arguments Abstractions

The average number of function arguments decreases from 6.14 to 5.33, threshold = 4.00. The functions in this file have too many arguments, indicating a lack of encapsulation or too many responsibilities in the same functions. Avoid adding more.
// See LICENSE for more details.

using System.Text;
using System.Text.RegularExpressions;
using System.Web;
using HtmlAgilityPack;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -29,6 +32,8 @@ public sealed class RichTextEditorPastedImages
private const string TemporaryImageDataAttribute = "data-tmpimg";
private readonly IPublishedUrlProvider _publishedUrlProvider;
private readonly IUmbracoContextAccessor _umbracoContextAccessor;
private readonly ILogger<RichTextEditorPastedImages> _logger;
private readonly IShortStringHelper _shortStringHelper;
private readonly ITemporaryFileService _temporaryFileService;
private readonly IScopeProvider _scopeProvider;
private readonly IMediaImportService _mediaImportService;
Expand Down Expand Up @@ -81,12 +86,14 @@ public RichTextEditorPastedImages(
IMediaImportService mediaImportService,
IImageUrlGenerator imageUrlGenerator,
IOptions<ContentSettings> contentSettings)
: this(umbracoContextAccessor, publishedUrlProvider, temporaryFileService, scopeProvider, mediaImportService, imageUrlGenerator, contentSettings)
: this(umbracoContextAccessor, logger, shortStringHelper, publishedUrlProvider, temporaryFileService, scopeProvider, mediaImportService, imageUrlGenerator, contentSettings)
{
}

public RichTextEditorPastedImages(
IUmbracoContextAccessor umbracoContextAccessor,
ILogger<RichTextEditorPastedImages> logger,
IShortStringHelper shortStringHelper,
IPublishedUrlProvider publishedUrlProvider,
ITemporaryFileService temporaryFileService,
IScopeProvider scopeProvider,
Expand All @@ -96,6 +103,8 @@ public RichTextEditorPastedImages(
{
_umbracoContextAccessor =
umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor));
_logger = logger;
_shortStringHelper = shortStringHelper;

Check notice on line 107 in src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Constructor Over-Injection

RichTextEditorPastedImages increases from 7 to 9 arguments, threshold = 5. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
_publishedUrlProvider = publishedUrlProvider;
_temporaryFileService = temporaryFileService;
_scopeProvider = scopeProvider;
Expand All @@ -113,13 +122,87 @@ public RichTextEditorPastedImages(
/// </summary>
/// <param name="html">HTML from the Rich Text Editor property editor.</param>
/// <param name="mediaParentFolder"></param>
/// <param name="userId"></param>
/// <param name="userKey"></param>
/// <returns>Formatted HTML.</returns>
/// <exception cref="NotSupportedException">Thrown if image extension is not allowed</exception>
internal string FindAndPersistEmbeddedImages(string html, Guid mediaParentFolder, Guid userId)
internal async Task<string> FindAndPersistEmbeddedImagesAsync(string html, Guid mediaParentFolder, Guid userKey)
{
// FIXME: the FindAndPersistEmbeddedImages implementation from #14546 must be ported to V14 and added here
return html;
// Find all img's that has data-tmpimg attribute
// Use HTML Agility Pack - https://html-agility-pack.net
var htmlDoc = new HtmlDocument();
htmlDoc.LoadHtml(html);

HtmlNodeCollection? imagesWithDataUris = htmlDoc.DocumentNode.SelectNodes("//img");

if (imagesWithDataUris is null || imagesWithDataUris.Count is 0)
{
return html;
}

foreach (HtmlNode? img in imagesWithDataUris)
{
var srcValue = img.GetAttributeValue("src", string.Empty);

// Ignore src-less images
if (string.IsNullOrEmpty(srcValue))
{
continue;
}

// Take only images that have a "data:image" uri into consideration
if (!srcValue.StartsWith("data:image"))
{
continue;
}

// Create tmp image by scanning the srcValue
// the value will look like "data:image/jpg;base64,abc" where the first part
// is the mimetype and the second (after the comma) is the image blob
Match dataUriInfo = Regex.Match(srcValue, @"^data:\w+\/(?<ext>\w+)[\w\+]*?;(?<encoding>\w+),(?<data>.+)$");

// If it turns up false, it was probably a false-positive and we can't do anything with it
if (dataUriInfo.Success is false)
{
continue;
}

var ext = dataUriInfo.Groups["ext"].Value.ToLowerInvariant();
var encoding = dataUriInfo.Groups["encoding"].Value.ToLowerInvariant();
var imageData = dataUriInfo.Groups["data"].Value;

if (_contentSettings.IsFileAllowedForUpload(ext) is false)
{
// If the image format is not supported we should probably leave it be
// since the user decided to include it.
// If we accepted it anyway, they could technically circumvent the allow list for file types,
// but the user experience would not be very good if we simply failed to save the content.
// Besides, there may be other types of data uri images technically supported by a browser that we cannot handle.
_logger.LogWarning(
"Performance impact: Could not convert embedded image to a Media item because the file extension {Ext} was not allowed. HTML extract: {OuterHtml}",
ext,
img.OuterHtml.Length < 100 ? img.OuterHtml : img.OuterHtml[..100]); // only log the first 100 chars because base64 images can be very long
continue;
}

// convert the encoded image data to bytes
var bytes = encoding.Equals("base64")
? Convert.FromBase64String(imageData)
: Encoding.UTF8.GetBytes(HttpUtility.HtmlDecode(imageData));
GuidUdi udi;
using (var stream = new MemoryStream(bytes))
{
var safeFileName = $"image.{ext}".ToSafeFileName(_shortStringHelper);
var mediaTypeAlias = MediaTypeAlias(safeFileName);

Guid? parentFolderKey = mediaParentFolder == Guid.Empty ? Constants.System.RootKey : mediaParentFolder;
IMedia mediaFile = await _mediaImportService.ImportAsync(safeFileName, stream, parentFolderKey, mediaTypeAlias, userKey);
udi = mediaFile.GetUdi();
}

UpdateImageNode(img, udi);
}

return htmlDoc.DocumentNode.OuterHtml;

Check warning on line 205 in src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

❌ New issue: Complex Method

FindAndPersistEmbeddedImagesAsync has a cyclomatic complexity of 15, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

[Obsolete($"Please use {nameof(FindAndPersistPastedTempImagesAsync)}. Will be removed in V16.")]
Expand Down Expand Up @@ -177,9 +260,11 @@ public async Task<string> FindAndPersistPastedTempImagesAsync(string html, Guid

if (uploadedImages.ContainsKey(temporaryFileKey) == false)
{
var mediaTypeAlias = MediaTypeAlias(temporaryFile.FileName);

using Stream fileStream = temporaryFile.OpenReadStream();
Guid? parentFolderKey = mediaParentFolder == Guid.Empty ? Constants.System.RootKey : mediaParentFolder;
IMedia mediaFile = await _mediaImportService.ImportAsync(temporaryFile.FileName, fileStream, parentFolderKey, Constants.Conventions.MediaTypes.Image, userKey);
IMedia mediaFile = await _mediaImportService.ImportAsync(temporaryFile.FileName, fileStream, parentFolderKey, mediaTypeAlias, userKey);
udi = mediaFile.GetUdi();
}
else
Expand All @@ -191,35 +276,7 @@ public async Task<string> FindAndPersistPastedTempImagesAsync(string html, Guid
scope.Complete();
}

// Add the UDI to the img element as new data attribute
img.SetAttributeValue("data-udi", udi.ToString());

// Get the new persisted image URL
_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext);
IPublishedContent? mediaTyped = umbracoContext?.Media?.GetById(udi.Guid);
if (mediaTyped == null)
{
throw new PanicException(
$"Could not find media by id {udi.Guid} or there was no UmbracoContext available.");
}

var location = mediaTyped.Url(_publishedUrlProvider);

// Find the width & height attributes as we need to set the imageprocessor QueryString
var width = img.GetAttributeValue("width", int.MinValue);
var height = img.GetAttributeValue("height", int.MinValue);

if (width != int.MinValue && height != int.MinValue)
{
location = _imageUrlGenerator.GetImageUrl(new ImageUrlGenerationOptions(location)
{
ImageCropMode = ImageCropMode.Max,
Width = width,
Height = height,
});
}

img.SetAttributeValue("src", location);
UpdateImageNode(img, udi);

Check notice on line 279 in src/Umbraco.Infrastructure/PropertyEditors/RichTextEditorPastedImages.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

✅ Getting better: Complex Method

FindAndPersistPastedTempImagesAsync decreases in cyclomatic complexity from 13 to 10, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

// Remove the data attribute (so we do not re-process this)
img.Attributes.Remove(TemporaryImageDataAttribute);
Expand All @@ -230,4 +287,42 @@ public async Task<string> FindAndPersistPastedTempImagesAsync(string html, Guid

return htmlDoc.DocumentNode.OuterHtml;
}

private string MediaTypeAlias(string fileName)
=> fileName.EndsWith(".svg")
? Constants.Conventions.MediaTypes.VectorGraphicsAlias
: Constants.Conventions.MediaTypes.Image;

private void UpdateImageNode(HtmlNode img, GuidUdi udi)
{
// Add the UDI to the img element as new data attribute
img.SetAttributeValue("data-udi", udi.ToString());

// Get the new persisted image URL
_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext);
IPublishedContent? mediaTyped = umbracoContext?.Media?.GetById(udi.Guid);
if (mediaTyped == null)
{
throw new PanicException(
$"Could not find media by id {udi.Guid} or there was no UmbracoContext available.");
}

var location = mediaTyped.Url(_publishedUrlProvider);

// Find the width & height attributes as we need to set the imageprocessor QueryString
var width = img.GetAttributeValue("width", int.MinValue);
var height = img.GetAttributeValue("height", int.MinValue);

if (width != int.MinValue && height != int.MinValue)
{
location = _imageUrlGenerator.GetImageUrl(new ImageUrlGenerationOptions(location)
{
ImageCropMode = ImageCropMode.Max,
Width = width,
Height = height,
});
}

img.SetAttributeValue("src", location);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,10 @@ public override IEnumerable<ITag> GetTags(object? value, object? dataTypeConfigu
return null;
}

var parseAndSaveBase64Images = _pastedImages.FindAndPersistEmbeddedImages(
richTextEditorValue.Markup, mediaParentId, userKey);
var parseAndSaveBase64Images = _pastedImages
.FindAndPersistEmbeddedImagesAsync(richTextEditorValue.Markup, mediaParentId, userKey)
.GetAwaiter()
.GetResult();
var parseAndSavedTempImages = _pastedImages
.FindAndPersistPastedTempImagesAsync(parseAndSaveBase64Images, mediaParentId, userKey)
.GetAwaiter()
Expand Down
Loading

0 comments on commit fdf2597

Please sign in to comment.