Fix validation of image resize with and height to calculate the target

changelog/src/changelog/entries/2024/09/7952.SUP-17267.bugfix

@@ -0,0 +1 @@
+Image Manipulation: The validation of resize width and height have been fixed to also properly handle values set to "auto" in the request.

core/src/main/java/com/gentics/mesh/core/endpoint/node/BinaryTransformHandler.java

@@ -223,7 +223,7 @@ public void handleS3TransformImage(RoutingContext rc, String uuid, String fieldN
 		s3UploadContext.setS3BinaryUuid(UUIDUtil.randomUUID());
 		s3UploadContext.setS3ObjectKey(s3ObjectKey);
 		imageManipulator
-				.handleS3Resize(options.getS3Options().getBucket(), s3ObjectKey, fileName, parameters)
+				.handleS3Resize(options.getS3Options().getBucket(), s3ObjectKey, fileName, s3binaryField.getBinary(), parameters)
 				.flatMap(file -> {
 					// The image was stored and hashed. Now we need to load the stored file again and check the image properties
 					Single<ImageInfo> info = imageManipulator.readImageInfo(file.getName());

core/src/main/java/com/gentics/mesh/core/endpoint/node/S3BinaryFieldResponseHandler.java

@@ -114,7 +114,7 @@ private void resizeAndRespond(RoutingContext rc, S3HibBinaryField s3binaryField,
 		String cacheS3ObjectKey = s3ObjectKey + "/image-" + imageParams.getCacheKey();
 		String fileName = s3binaryField.getBinary().getFileName();
 		imageManipulator
-				.handleS3CacheResize(s3Options.getBucket(), s3Options.getS3CacheOptions().getBucket(), s3ObjectKey, cacheS3ObjectKey, fileName, imageParams)
+				.handleS3CacheResize(s3Options.getBucket(), s3Options.getS3CacheOptions().getBucket(), s3ObjectKey, cacheS3ObjectKey, fileName, s3binaryField.getBinary(), imageParams)
 				.andThen(Single.defer(() -> {
 					Single<S3RestResponse> presignedUrl = s3Binarystorage.createDownloadPresignedUrl(s3Options.getS3CacheOptions().getBucket(), cacheS3ObjectKey, true);
 					return presignedUrl;

mdm/common/src/main/java/com/gentics/mesh/core/image/ImageManipulator.java

@@ -5,11 +5,13 @@
 import java.io.InputStream;
 import java.util.Map;
 
+import com.gentics.mesh.core.data.HibImageDataElement;
 import com.gentics.mesh.core.data.binary.HibBinary;
 import com.gentics.mesh.core.data.node.field.HibBinaryField;
 import com.gentics.mesh.core.rest.node.field.image.FocalPoint;
 import com.gentics.mesh.parameter.ImageManipulationParameters;
 import com.gentics.mesh.parameter.image.ImageManipulation;
+import com.gentics.mesh.parameter.image.ResizeMode;
 
 import io.reactivex.Completable;
 import io.reactivex.Single;
@@ -36,20 +38,22 @@ public interface ImageManipulator {
 	 * @param s3ObjectKey
 	 * @param cacheS3ObjectKey
 	 * @param filename
+	 * @param image
 	 * @param parameters
 	 */
 	Completable handleS3CacheResize(String bucketName, String cacheBucketName, String s3ObjectKey,
-			String cacheS3ObjectKey, String filename, ImageManipulationParameters parameters);
+			String cacheS3ObjectKey, String filename, HibImageDataElement image, ImageManipulationParameters parameters);
 
 	/**
 	 * Resize the given s3 binary data and return the resulting file.
 	 *
 	 * @param bucketName
 	 * @param s3ObjectKey
 	 * @param filename
+	 * @param image
 	 * @param parameters
 	 */
-	Single<File> handleS3Resize(String bucketName, String s3ObjectKey, String filename,
+	Single<File> handleS3Resize(String bucketName, String s3ObjectKey, String filename, HibImageDataElement image,
 			ImageManipulationParameters parameters);
 
 	/**
@@ -116,11 +120,13 @@ static <T extends ImageManipulation> T applyDefaultManipulation(T imageParams, H
 		Integer originalHeight = binary.getImageHeight();
 		Integer originalWidth = binary.getImageWidth();
 
-		if ("auto".equals(imageParams.getHeight())) {
-			imageParams.setHeight(originalHeight);
-		}
-		if ("auto".equals(imageParams.getWidth())) {
-			imageParams.setWidth(originalWidth);
+		if (imageParams.getResizeMode() == ResizeMode.SMART) {
+			if ("auto".equals(imageParams.getHeight())) {
+				imageParams.setHeight(originalHeight);
+			}
+			if ("auto".equals(imageParams.getWidth())) {
+				imageParams.setWidth(originalWidth);
+			}
 		}
 		return imageParams;
 	}

rest-model/src/main/java/com/gentics/mesh/parameter/image/ImageManipulation.java

@@ -6,7 +6,6 @@
 
 import com.gentics.mesh.core.rest.node.field.image.FocalPoint;
 import com.gentics.mesh.core.rest.node.field.image.Point;
-import com.gentics.mesh.etc.config.ImageManipulatorOptions;
 import com.gentics.mesh.util.NumberUtils;
 
 /**
@@ -243,24 +242,6 @@ default ImageManipulation setFocalPoint(float x, float y) {
 	 */
 	ImageManipulation validateFocalPointParameter();
 
-	/**
-	 * Check whether all required crop parameters have been set.
-	 *
-	 * @param options
-	 * @return Fluent API
-	 */
-	default ImageManipulation validateLimits(ImageManipulatorOptions options) {
-		int width = NumberUtils.toInt(getWidth(), 0);
-		int height = NumberUtils.toInt(getHeight(), 0);
-		if (getWidth() != null && options.getMaxWidth() != null && options.getMaxWidth() > 0 && width > options.getMaxWidth()) {
-			throw error(BAD_REQUEST, "image_error_width_limit_exceeded", String.valueOf(options.getMaxWidth()), String.valueOf(getWidth()));
-		}
-		if (getHeight() != null && options.getMaxHeight() != null && options.getMaxHeight() > 0 && height > options.getMaxHeight()) {
-			throw error(BAD_REQUEST, "image_error_height_limit_exceeded", String.valueOf(options.getMaxHeight()), String.valueOf(getHeight()));
-		}
-		return this;
-	}
-
 	/**
 	 * Check whether any resize or crop param has been set.
 	 *

services/image-imgscalr/src/main/java/com/gentics/mesh/image/ImgscalrImageManipulator.java

@@ -37,11 +37,14 @@
 import org.imgscalr.Scalr;
 import org.imgscalr.Scalr.Mode;
 
+import com.gentics.mesh.core.data.HibImageDataElement;
 import com.gentics.mesh.core.data.binary.HibBinary;
 import com.gentics.mesh.core.data.storage.BinaryStorage;
 import com.gentics.mesh.core.data.storage.S3BinaryStorage;
 import com.gentics.mesh.core.db.Supplier;
+import com.gentics.mesh.core.image.ImageManipulator;
 import com.gentics.mesh.core.image.spi.AbstractImageManipulator;
+import com.gentics.mesh.core.rest.node.field.image.Point;
 import com.gentics.mesh.etc.config.ImageManipulationMode;
 import com.gentics.mesh.etc.config.ImageManipulatorOptions;
 import com.gentics.mesh.etc.config.MeshOptions;
@@ -124,13 +127,16 @@ protected BufferedImage crop(BufferedImage originalImage, ImageRect cropArea) {
 	 * @return Resized image or original image if no resize operation was requested
 	 */
 	protected BufferedImage resizeIfRequested(BufferedImage originalImage, ImageManipulation parameters) {
+		Point original = new Point(originalImage.getWidth(), originalImage.getHeight());
+		Point calculated = calculateNewSize(new Point(NumberUtils.toInt(parameters.getWidth(), 0), NumberUtils.toInt(parameters.getHeight(), 0)),
+				new Point(originalImage.getWidth(), originalImage.getHeight()), parameters.getResizeMode());
+
 		int originalHeight = originalImage.getHeight();
 		int originalWidth = originalImage.getWidth();
-		double aspectRatio = (double) originalWidth / (double) originalHeight;
 
 		// Resize if required and calculate missing parameters if needed
-		Integer pHeight = NumberUtils.toInt(parameters.getHeight(), 0);
-		Integer pWidth = NumberUtils.toInt(parameters.getWidth(), 0);
+		int pHeight = calculated.getY();
+		int pWidth = calculated.getX();
 
 		// Resizing is only needed when one of the parameters has been specified
 		if (pHeight != 0 || pWidth != 0) {
@@ -150,26 +156,14 @@ protected BufferedImage resizeIfRequested(BufferedImage originalImage, ImageMani
 				return originalImage;
 			}
 			ResizeMode resizeMode = parameters.getResizeMode();
-			// if the mode used is smart, and one of the dimensions is auto then set this
-			// dimension to the original Value
-			if (resizeMode == ResizeMode.SMART) {
-				if (parameters.getWidth() != null && parameters.getWidth().equals("auto")) {
-					pWidth = originalWidth;
-				}
-				if (parameters.getHeight() != null && parameters.getHeight().equals("auto")) {
-					pHeight = originalHeight;
-				}
-			}
-			int width = pWidth == 0 ? (int) (pHeight * aspectRatio) : pWidth;
-			int height = pHeight == 0 ? (int) (width / aspectRatio) : pHeight;
 
 			// if we want to use smart resizing we need to crop the original image to the
 			// correct format before resizing to avoid distortion
 			if (pWidth != 0 && pHeight != 0 && resizeMode == ResizeMode.SMART) {
 
 				double pAspectRatio = (double) pWidth / (double) pHeight;
-				if (aspectRatio != pAspectRatio) {
-					if (aspectRatio < pAspectRatio) {
+				if (original.getRatio() != pAspectRatio) {
+					if (original.getRatio() < pAspectRatio) {
 						// crop height (top & bottom)
 						int resizeHeight = Math.max(1, (int) (originalWidth / pAspectRatio));
 						int startY = (int) (originalHeight * 0.5 - resizeHeight * 0.5);
@@ -186,27 +180,16 @@ protected BufferedImage resizeIfRequested(BufferedImage originalImage, ImageMani
 			// if we want to use proportional resizing we need to make sure the destination
 			// dimension fits inside the provided dimensions
 			if (pWidth != 0 && pHeight != 0 && resizeMode == ResizeMode.PROP) {
-				double pAspectRatio = (double) pWidth / (double) pHeight;
-				if (aspectRatio < pAspectRatio) {
-					// scale to pHeight
-					width = Math.max(1, (int) (pHeight * aspectRatio));
-					height = Math.max(1, pHeight);
-				} else {
-					// scale to pWidth
-					width = Math.max(1, pWidth);
-					height = Math.max(1, (int) (pWidth / aspectRatio));
-				}
-
 				// Should the resulting format be the same as the original image we do not need
 				// to resize
-				if (width == originalWidth && height == originalHeight) {
+				if (calculated.getX() == originalWidth && calculated.getY() == originalHeight) {
 					return originalImage;
 				}
 			}
 
 			try {
 				BufferedImage image = Scalr.apply(originalImage,
-						new ResampleOp(width, height, options.getResampleFilter().getFilter()));
+						new ResampleOp(calculated.getX(), calculated.getY(), options.getResampleFilter().getFilter()));
 				originalImage.flush();
 				return image;
 			} catch (IllegalArgumentException e) {
@@ -322,14 +305,15 @@ protected BufferedImage cropAndResize(BufferedImage image, ImageManipulation par
 
 	@Override
 	public Single<String> handleResize(HibBinary binary, ImageManipulation parameters) {
+		ImageManipulator.applyDefaultManipulation(parameters, binary);
 		ImageManipulationMode mode = options.getMode();
 
 		if (ImageManipulationMode.OFF == mode) {
 			throw error(BAD_REQUEST, "image_error_reading_failed");
 		}
 		// Validate the resize parameters
 		parameters.validateManipulation();
-		parameters.validateLimits(options);
+		validateLimits(parameters, binary, options);
 
 		String binaryUuid = binary.getUuid();
 
@@ -359,11 +343,11 @@ public Single<String> handleResize(HibBinary binary, ImageManipulation parameter
 	}
 
 	@Override
-	public Single<File> handleS3Resize(String bucketName, String s3ObjectKey, String filename,
+	public Single<File> handleS3Resize(String bucketName, String s3ObjectKey, String filename, HibImageDataElement image,
 			ImageManipulationParameters parameters) {
 		// Validate the resize parameters
 		parameters.validateManipulation();
-		parameters.validateLimits(options);
+		validateLimits(parameters, image, options);
 
 		return s3BinaryStorage.read(bucketName, s3ObjectKey)
 				.flatMapSingle(originalFile -> workerPool.<File>rxExecuteBlocking(bh -> {
@@ -386,10 +370,10 @@ public Single<File> handleS3Resize(String bucketName, String s3ObjectKey, String
 
 	@Override
 	public Completable handleS3CacheResize(String bucketName, String cacheBucketName, String s3ObjectKey,
-			String cacheS3ObjectKey, String filename, ImageManipulationParameters parameters) {
+			String cacheS3ObjectKey, String filename, HibImageDataElement image, ImageManipulationParameters parameters) {
 		// Validate the resize parameters
 		parameters.validate();
-		parameters.validateLimits(options);
+		validateLimits(parameters, image, options);
 
 		return s3BinaryStorage.exists(cacheBucketName, cacheS3ObjectKey)
 				// read from aws and return buffer with data
@@ -453,6 +437,62 @@ public int[] calculateDominantColor(BufferedImage image) {
 		return pixel.getData().getPixel(0, 0, (int[]) null);
 	}
 
+	protected Point calculateNewSize(Point requested, Point original, ResizeMode resizeMode) {
+		int originalHeight = original.getY();
+		int originalWidth = original.getX();
+		double aspectRatio = (double) originalWidth / (double) originalHeight;
+
+		// Resize if required and calculate missing parameters if needed
+		int pHeight = requested.getY();
+		int pWidth = requested.getX();
+
+		// Resizing is only needed when one of the parameters has been specified
+		if (pHeight != 0 || pWidth != 0) {
+
+			// No operation needed when width is the same and no height was set
+			if (pHeight == 0 && pWidth == originalWidth) {
+				return requested;
+			}
+
+			// No operation needed when height is the same and no width was set
+			if (pWidth == 0 && pHeight == originalHeight) {
+				return requested;
+			}
+
+			// No operation needed when width and height match original image
+			if (pWidth != 0 && pWidth == originalWidth && pHeight != 0 && pHeight == originalHeight) {
+				return requested;
+			}
+
+			int width = pWidth == 0 ? (int) (pHeight * aspectRatio) : pWidth;
+			int height = pHeight == 0 ? (int) (width / aspectRatio) : pHeight;
+
+			// if we want to use proportional resizing we need to make sure the destination
+			// dimension fits inside the provided dimensions
+			if (pWidth != 0 && pHeight != 0 && resizeMode == ResizeMode.PROP) {
+				double pAspectRatio = (double) pWidth / (double) pHeight;
+				if (aspectRatio < pAspectRatio) {
+					// scale to pHeight
+					width = Math.max(1, (int) (pHeight * aspectRatio));
+					height = Math.max(1, pHeight);
+				} else {
+					// scale to pWidth
+					width = Math.max(1, pWidth);
+					height = Math.max(1, (int) (pWidth / aspectRatio));
+				}
+
+				// Should the resulting format be the same as the original image we do not need
+				// to resize
+				if (width == originalWidth && height == originalHeight) {
+					return requested;
+				}
+			}
+			return new Point(width, height);
+		} else {
+			return requested;
+		}
+	}
+
 	@Override
 	public Single<Map<String, String>> getMetadata(InputStream ins) {
 		return Single.create(sub -> {
@@ -490,6 +530,29 @@ protected BufferedImage readFromFile(File imageFile) throws IOException {
 		return bufferedImage;
 	}
 
+	/**
+	 * Check whether the resized image will be within the allowed limits
+	 *
+	 * @param parameters
+	 * @param original
+	 * @param options
+	 * @return Fluent API
+	 */
+	protected void validateLimits(ImageManipulation parameters, HibImageDataElement original, ImageManipulatorOptions options) {
+		Point newSize = calculateNewSize(
+				new Point(NumberUtils.toInt(parameters.getWidth(), 0), NumberUtils.toInt(parameters.getHeight(), 0)),
+				new Point(original.getImageWidth(), original.getImageHeight()), parameters.getResizeMode());
+
+		int width = newSize.getX();
+		int height = newSize.getY();
+		if (options.getMaxWidth() != null && options.getMaxWidth() > 0 && width > options.getMaxWidth()) {
+			throw error(BAD_REQUEST, "image_error_width_limit_exceeded", String.valueOf(options.getMaxWidth()), String.valueOf(width));
+		}
+		if (options.getMaxHeight() != null && options.getMaxHeight() > 0 && height > options.getMaxHeight()) {
+			throw error(BAD_REQUEST, "image_error_height_limit_exceeded", String.valueOf(options.getMaxHeight()), String.valueOf(height));
+		}
+	}
+
 	/**
 	 * Resize the image supplied by the given stream supplier. If reading the image
 	 * with ImageIO is not possible (e.g. for webp images), this will fall back to