Stop image endpoints from upscaling beyond the source resolution
ImageHelper.GetNewImageSize passed the caller-supplied width/height straight through to SkiaEncoder.EncodeImage, which allocates an SKImageInfo of exactly that size. Nothing bounded those values against the source image, so a request like Items/<id>/Images/Primary?width=23100&height=23100 made the server allocate and resample a 23100x23100 surface from, say, a 600x336 poster: the reporter measured 100% of a core for 10-15 minutes and 6-12 GB resident per request. The item images endpoints do not require authentication, so any caller who knows an item id can trigger this, and varying the size by one pixel misses the cache every time. Add DrawingUtils.ScaleDownToFit, which scales a size down uniformly until it fits inside a bounding box and returns it unchanged if it already does, and apply it in GetNewImageSize against the original image dimensions. Requests that ask for more pixels than the source now get the source resolution back, scaled to the requested aspect ratio. Downscaling paths are untouched, and DrawingUtils.Resize keeps its existing behaviour for the transcoding callers in EncodingJobInfo and StreamInfo, which legitimately size video output. ResizeFill already refused to upscale; this makes width/height consistent with fillWidth/fillHeight. Fixes #17056.
This commit is contained in:
@@ -11,7 +11,11 @@ namespace MediaBrowser.Controller.Drawing
|
||||
// Determine the output size based on incoming parameters
|
||||
var newSize = DrawingUtils.Resize(originalImageSize, options.Width ?? 0, options.Height ?? 0, options.MaxWidth ?? 0, options.MaxHeight ?? 0);
|
||||
newSize = DrawingUtils.ResizeFill(newSize, options.FillWidth, options.FillHeight);
|
||||
return newSize;
|
||||
|
||||
// Never encode larger than the source. Upscaling adds no detail, and the requested
|
||||
// width/height are caller-controlled, so without this an unauthenticated request can
|
||||
// pin a CPU and allocate several GB encoding a single image.
|
||||
return DrawingUtils.ScaleDownToFit(newSize, originalImageSize);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -103,6 +103,35 @@ namespace MediaBrowser.Model.Drawing
|
||||
return new ImageDimensions(newWidth, newHeight);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Scales a size down uniformly until it fits inside a bounding box.
|
||||
/// Returns the original size if it already fits, so this never upscales.
|
||||
/// </summary>
|
||||
/// <param name="size">The size object.</param>
|
||||
/// <param name="boundingBox">The box the result has to fit inside.</param>
|
||||
/// <returns>A new size object, or <paramref name="size"/> if it already fits.</returns>
|
||||
public static ImageDimensions ScaleDownToFit(ImageDimensions size, ImageDimensions boundingBox)
|
||||
{
|
||||
if (size.Width <= 0 || size.Height <= 0 || boundingBox.Width <= 0 || boundingBox.Height <= 0)
|
||||
{
|
||||
return size;
|
||||
}
|
||||
|
||||
double widthRatio = size.Width / (double)boundingBox.Width;
|
||||
double heightRatio = size.Height / (double)boundingBox.Height;
|
||||
double scaleRatio = Math.Max(widthRatio, heightRatio);
|
||||
|
||||
if (scaleRatio <= 1)
|
||||
{
|
||||
return size;
|
||||
}
|
||||
|
||||
var newWidth = Math.Clamp(Convert.ToInt32(Math.Round(size.Width / scaleRatio)), 1, boundingBox.Width);
|
||||
var newHeight = Math.Clamp(Convert.ToInt32(Math.Round(size.Height / scaleRatio)), 1, boundingBox.Height);
|
||||
|
||||
return new ImageDimensions(newWidth, newHeight);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gets the new width.
|
||||
/// </summary>
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
using MediaBrowser.Controller.Drawing;
|
||||
using MediaBrowser.Model.Drawing;
|
||||
using Xunit;
|
||||
|
||||
namespace Jellyfin.Controller.Tests.Drawing;
|
||||
|
||||
public static class ImageHelperTests
|
||||
{
|
||||
[Fact]
|
||||
public static void GetNewImageSize_ExplicitSizeLargerThanSource_ClampsToSource()
|
||||
{
|
||||
// Regression test for https://github.com/jellyfin/jellyfin/issues/17056: the caller-supplied
|
||||
// width/height were used verbatim, so a single request could ask for a 23100x23100 encode.
|
||||
var options = new ImageProcessingOptions { Width = 23100, Height = 23100 };
|
||||
|
||||
var newSize = ImageHelper.GetNewImageSize(options, new ImageDimensions(600, 336));
|
||||
|
||||
Assert.Equal(336, newSize.Width);
|
||||
Assert.Equal(336, newSize.Height);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public static void GetNewImageSize_WidthLargerThanSource_ClampsToSource()
|
||||
{
|
||||
var options = new ImageProcessingOptions { Width = 10000 };
|
||||
|
||||
var newSize = ImageHelper.GetNewImageSize(options, new ImageDimensions(600, 336));
|
||||
|
||||
Assert.Equal(600, newSize.Width);
|
||||
Assert.Equal(336, newSize.Height);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public static void GetNewImageSize_FillLargerThanSource_ClampsToSource()
|
||||
{
|
||||
// ResizeFill already refused to upscale; this pins that behaviour.
|
||||
var options = new ImageProcessingOptions { FillWidth = 23100, FillHeight = 23100 };
|
||||
|
||||
var newSize = ImageHelper.GetNewImageSize(options, new ImageDimensions(600, 336));
|
||||
|
||||
Assert.Equal(600, newSize.Width);
|
||||
Assert.Equal(336, newSize.Height);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public static void GetNewImageSize_SmallerThanSource_StillDownscales()
|
||||
{
|
||||
var options = new ImageProcessingOptions { MaxWidth = 300 };
|
||||
|
||||
var newSize = ImageHelper.GetNewImageSize(options, new ImageDimensions(600, 336));
|
||||
|
||||
Assert.Equal(300, newSize.Width);
|
||||
Assert.Equal(168, newSize.Height);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public static void GetNewImageSize_NoSizeRequested_ReturnsSource()
|
||||
{
|
||||
var newSize = ImageHelper.GetNewImageSize(new ImageProcessingOptions(), new ImageDimensions(600, 336));
|
||||
|
||||
Assert.Equal(600, newSize.Width);
|
||||
Assert.Equal(336, newSize.Height);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,28 @@
|
||||
using MediaBrowser.Model.Drawing;
|
||||
using Xunit;
|
||||
|
||||
namespace Jellyfin.Model.Drawing;
|
||||
|
||||
public static class DrawingUtilsTests
|
||||
{
|
||||
[Theory]
|
||||
// Already inside the box, returned untouched.
|
||||
[InlineData(600, 336, 1920, 1080, 600, 336)]
|
||||
[InlineData(1920, 1080, 1920, 1080, 1920, 1080)]
|
||||
// Scaled down uniformly, requested aspect ratio preserved.
|
||||
[InlineData(23100, 23100, 1920, 1080, 1080, 1080)]
|
||||
[InlineData(3840, 2160, 1920, 1080, 1920, 1080)]
|
||||
[InlineData(1200, 400, 600, 336, 600, 200)]
|
||||
// Extreme ratios still produce at least one pixel per axis.
|
||||
[InlineData(10000, 1, 100, 100, 100, 1)]
|
||||
// Degenerate inputs are passed through rather than dividing by zero.
|
||||
[InlineData(600, 336, 0, 0, 600, 336)]
|
||||
[InlineData(0, 0, 1920, 1080, 0, 0)]
|
||||
public static void ScaleDownToFit_Bounds_WithoutUpscaling(int width, int height, int boxWidth, int boxHeight, int expectedWidth, int expectedHeight)
|
||||
{
|
||||
var scaled = DrawingUtils.ScaleDownToFit(new ImageDimensions(width, height), new ImageDimensions(boxWidth, boxHeight));
|
||||
|
||||
Assert.Equal(expectedWidth, scaled.Width);
|
||||
Assert.Equal(expectedHeight, scaled.Height);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user