-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Media: Set width and height for uploaded SVGs #22244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
AndyButland
merged 27 commits into
umbraco:main
from
enkelmedia:v17/feature/22114-svg-width-height
Mar 26, 2026
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
179581e
Added migration for SVG width/height
markusobviuse 354a751
#22114 worked on SVG width height implementation
markusobviuse d50d141
#22244 Code style fixes
enkelmedia 8a0dd86
#22244 XmlReaderSettings and using
enkelmedia fe50b97
#22244 Cleanup
enkelmedia 7c1671c
#22244 Correction if statement
enkelmedia deef6d6
#22244 Refactor log message
enkelmedia 22135be
#22244 Correction if statment
enkelmedia 2ce4b26
#22244 Cleanup
enkelmedia 5528991
#22244 Cleanup
enkelmedia 185659f
#22244 Code style adjustments
enkelmedia 4c226a5
#22244 Adjust if statement
enkelmedia c35dc48
#22244 Adjust documentation comments
enkelmedia 361f3c6
#22244 Fix log comment
markusobviuse 50dfc18
#22244 Fallback to viewbox if width height attribute has other unit t…
markusobviuse 5dbf0bd
#22244 Refactoring SVG parser, no support for decimals
markusobviuse fdf109c
#22244 Migration, consistent logging
markusobviuse ac3d587
#22244 Create vector umbracoWidth and umbracoHeight during clean install
markusobviuse 221dd93
#22244 Remove SupportedImageType from ISvgDimensionsExtractor
markusobviuse 880f9d2
#22244 pass culture and segment to SetValue
markusobviuse c34f620
Merge branch 'main' into v17/feature/22114-svg-width-height
AndyButland 780373d
Add DtdProcessing.Prohibit security hardening to SvgDimensionExtractor.
AndyButland 2027b08
Addressed some code styling and robustness of the migration and extra…
AndyButland e872f79
Add further unit tests.
AndyButland a39dbf7
Add logging to notification handler. Skip when properties don't exist…
AndyButland f174d1d
Add unit tests for media saving handler.
AndyButland 70ce78e
Move the dimensions extractor implementation into infrastructure.
AndyButland File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using System.Drawing; | ||
|
|
||
| namespace Umbraco.Cms.Core.Media; | ||
|
|
||
| /// <summary> | ||
| /// Extracts image dimensions from an SVG stream. | ||
| /// </summary> | ||
| public interface ISvgDimensionExtractor | ||
| { | ||
| /// <summary> | ||
| /// Gets the dimensions. | ||
| /// </summary> | ||
| /// <param name="stream">The stream.</param> | ||
| /// <returns> | ||
| /// The dimensions of the image if the stream was parsable; otherwise, <c>null</c>. | ||
| /// </returns> | ||
| public Size? GetDimensions(Stream stream); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
164 changes: 164 additions & 0 deletions
164
src/Umbraco.Infrastructure/Media/SvgDimensionExtractor.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| using System.Drawing; | ||
| using System.Globalization; | ||
| using System.Xml; | ||
| using System.Xml.Linq; | ||
| using Microsoft.Extensions.Logging; | ||
| using Umbraco.Cms.Core.Media; | ||
|
|
||
| namespace Umbraco.Cms.Infrastructure.Media; | ||
|
|
||
| /// <inheritdoc /> | ||
| public class SvgDimensionExtractor : ISvgDimensionExtractor | ||
| { | ||
| private readonly ILogger<SvgDimensionExtractor> _logger; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="SvgDimensionExtractor"/> class. | ||
| /// </summary> | ||
| /// <param name="logger">The logger.</param> | ||
| public SvgDimensionExtractor(ILogger<SvgDimensionExtractor> logger) | ||
| => _logger = logger; | ||
|
|
||
| /// <inheritdoc /> | ||
| public Size? GetDimensions(Stream stream) | ||
| { | ||
| if (stream.CanRead is false) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| long? originalPosition = null; | ||
|
|
||
| if (stream.CanSeek) | ||
| { | ||
| originalPosition = stream.Position; | ||
| stream.Position = 0; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| return ReadDimensions(stream); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Failed to extract dimensions from SVG stream."); | ||
| return null; | ||
| } | ||
| finally | ||
| { | ||
| if (originalPosition.HasValue) | ||
| { | ||
| stream.Position = originalPosition.Value; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static Size? ReadDimensions(Stream stream) | ||
| { | ||
| var settings = new XmlReaderSettings | ||
| { | ||
| DtdProcessing = DtdProcessing.Prohibit, | ||
| XmlResolver = null, | ||
| }; | ||
| using var reader = XmlReader.Create(stream, settings); | ||
| var document = XDocument.Load(reader); | ||
|
|
||
| XElement? root = document.Root; | ||
|
|
||
| if (root is null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var widthAttributeValue = root.Attribute("width")?.Value; | ||
| var heightAttributeValue = root.Attribute("height")?.Value; | ||
|
|
||
| Size? size = null; | ||
|
|
||
| if (widthAttributeValue is not null && heightAttributeValue is not null) | ||
| { | ||
| size = ParseWidthHeightAttributes(widthAttributeValue, heightAttributeValue); | ||
| } | ||
|
|
||
| // Fall back to viewbox. | ||
| size ??= ParseViewBox(root); | ||
|
|
||
| return size; | ||
| } | ||
|
|
||
| private static Size? ParseViewBox(XElement root) | ||
| { | ||
| var viewBox = root.Attribute("viewBox")?.Value; | ||
|
|
||
| if (string.IsNullOrWhiteSpace(viewBox)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var parts = viewBox.Split([' ', ',', '\t', '\r', '\n'], StringSplitOptions.RemoveEmptyEntries); | ||
|
|
||
| if (parts.Length != 4) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (double.TryParse(parts[2], NumberStyles.Float, CultureInfo.InvariantCulture, out var width) is false) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (double.TryParse(parts[3], NumberStyles.Float, CultureInfo.InvariantCulture, out var height) is false) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (width < 0 || height < 0) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| return new Size( | ||
| (int)Math.Round(width), | ||
| (int)Math.Round(height)); | ||
| } | ||
|
|
||
| private static Size? ParseWidthHeightAttributes(string widthAttributeValue, string heightAttributeValue) | ||
| { | ||
| if (TryExtractNumericFromValue(widthAttributeValue, out var widthValue) | ||
| && TryExtractNumericFromValue(heightAttributeValue, out var heightValue)) | ||
| { | ||
| return new Size(widthValue, heightValue); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Extract a "pixel" value from the width / height attributes. | ||
| /// </summary> | ||
| private static bool TryExtractNumericFromValue(string attributeValue, out int value) | ||
| { | ||
| if (int.TryParse(attributeValue, out int onlyNumbersValue) && onlyNumbersValue > 0) | ||
| { | ||
| value = onlyNumbersValue; | ||
| return true; | ||
| } | ||
|
|
||
| value = 0; | ||
|
|
||
| var input = attributeValue.Trim(); | ||
|
|
||
| if (input.EndsWith("px", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| input = input[..^2].Trim(); | ||
| } | ||
|
|
||
| if (int.TryParse(input, out var numericValue) && numericValue > 0) | ||
| { | ||
| value = numericValue; | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
101 changes: 101 additions & 0 deletions
101
src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_4_0/AddDimensionsToSvg.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| using Microsoft.Extensions.Logging; | ||
| using Umbraco.Cms.Core; | ||
| using Umbraco.Cms.Core.Models; | ||
| using Umbraco.Cms.Core.Services; | ||
| using Umbraco.Cms.Core.Services.OperationStatus; | ||
| using Umbraco.Cms.Core.Strings; | ||
|
|
||
| namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_17_4_0; | ||
|
|
||
| /// <summary> | ||
| /// Migration that adds umbracoWidth and umbracoHeight to Vector Graphics Media Type. | ||
| /// </summary> | ||
| public class AddDimensionsToSvg : AsyncMigrationBase | ||
| { | ||
| private readonly ILogger<AddDimensionsToSvg> _logger; | ||
| private readonly IMediaTypeService _mediaTypeService; | ||
| private readonly IDataTypeService _dataTypeService; | ||
| private readonly IShortStringHelper _shortStringHelper; | ||
|
|
||
| private readonly Guid _labelPixelsDataTypeKey = new(Constants.DataTypes.Guids.LabelPixels); | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="AddDimensionsToSvg"/> class. | ||
| /// </summary> | ||
| /// <param name="context">The <see cref="IMigrationContext"/> for the migration operation.</param> | ||
| /// <param name="logger">The <see cref="ILogger{AddDimensionsToSvg}"/> instance for logging.</param> | ||
| /// <param name="mediaTypeService">The <see cref="IMediaTypeService"/> for managing media types.</param> | ||
| /// <param name="dataTypeService">The <see cref="IDataTypeService"/> for managing data types.</param> | ||
| /// <param name="shortStringHelper">The <see cref="IShortStringHelper"/> for working with strings.</param> | ||
| public AddDimensionsToSvg( | ||
| IMigrationContext context, | ||
| ILogger<AddDimensionsToSvg> logger, | ||
| IMediaTypeService mediaTypeService, | ||
| IDataTypeService dataTypeService, | ||
| IShortStringHelper shortStringHelper) | ||
| : base(context) | ||
| { | ||
| _logger = logger; | ||
| _mediaTypeService = mediaTypeService; | ||
| _dataTypeService = dataTypeService; | ||
| _shortStringHelper = shortStringHelper; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| protected override async Task MigrateAsync() | ||
| { | ||
| IMediaType? vectorGraphicsMediaType = _mediaTypeService.Get(Constants.Conventions.MediaTypes.VectorGraphicsAlias); | ||
|
|
||
| if (vectorGraphicsMediaType is null) | ||
| { | ||
| _logger.LogInformation("No standard Vector Graphics Media Type configured, ignore adding width/height."); | ||
| return; | ||
| } | ||
|
|
||
| IDataType? labelPixelDataType = await _dataTypeService.GetAsync(_labelPixelsDataTypeKey); | ||
|
|
||
| if (labelPixelDataType is null) | ||
| { | ||
| _logger.LogInformation("No Label Pixel Data Type configured, ignore adding width/height."); | ||
| return; | ||
| } | ||
|
|
||
| PropertyGroup? propertyGroup = vectorGraphicsMediaType.PropertyGroups.FirstOrDefault(); | ||
| if (propertyGroup is null) | ||
| { | ||
| _logger.LogWarning("Vector Graphics Media Type has no property groups, skipping adding width/height."); | ||
| return; | ||
| } | ||
|
|
||
| int highestSort = vectorGraphicsMediaType.PropertyTypes.Any() | ||
| ? vectorGraphicsMediaType.PropertyTypes.Max(x => x.SortOrder) | ||
| : 0; | ||
|
|
||
| // Add new properties (AddPropertyType handles duplicates, so the migration is idempotent). | ||
| vectorGraphicsMediaType.AddPropertyType(new PropertyType(_shortStringHelper, labelPixelDataType, Constants.Conventions.Media.Width) | ||
| { | ||
| Name = "Width", | ||
| SortOrder = highestSort + 1, | ||
| PropertyGroupId = new Lazy<int>(()=> propertyGroup.Id), | ||
| }); | ||
| vectorGraphicsMediaType.AddPropertyType(new PropertyType(_shortStringHelper, labelPixelDataType, Constants.Conventions.Media.Height) | ||
| { | ||
| Name = "Height", | ||
| SortOrder = highestSort + 2, | ||
| PropertyGroupId = new Lazy<int>(()=> propertyGroup.Id), | ||
| }); | ||
|
|
||
| Attempt<ContentTypeOperationStatus> attempt = await _mediaTypeService.UpdateAsync(vectorGraphicsMediaType, Constants.Security.SuperUserKey); | ||
| if (attempt.Success is false) | ||
| { | ||
| if (attempt.Exception is not null) | ||
| { | ||
| _logger.LogError(attempt.Exception, "Failed to update media type '{Alias}' during migration.", vectorGraphicsMediaType.Alias); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogWarning("Failed to update media type '{Alias}' during migration. Status: {ResultStatus}", vectorGraphicsMediaType.Alias, attempt.Result); | ||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.