diff --git a/app/common/src/main/java/stirling/software/common/util/RegexPatternUtils.java b/app/common/src/main/java/stirling/software/common/util/RegexPatternUtils.java index b4bff5d01..5592532b8 100644 --- a/app/common/src/main/java/stirling/software/common/util/RegexPatternUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/RegexPatternUtils.java @@ -14,7 +14,8 @@ public final class RegexPatternUtils { private static final String WHITESPACE_REGEX = "\\s++"; private static final String EXTENSION_REGEX = "\\.(?:[^.]*+)?$"; - private static final Pattern COLOR_PATTERN = Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$"); + private static final Pattern COLOR_PATTERN = + Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{8})$"); private RegexPatternUtils() { super(); diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/WatermarkController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/WatermarkController.java index 6f9ca0830..70612b6c7 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/security/WatermarkController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/security/WatermarkController.java @@ -1,5 +1,7 @@ package stirling.software.SPDF.controller.api.security; +import static stirling.software.common.util.RegexPatternUtils.getColorPattern; + import java.awt.*; import java.awt.image.BufferedImage; import java.beans.PropertyEditorSupport; @@ -44,8 +46,6 @@ import stirling.software.SPDF.model.api.security.AddWatermarkRequest; import stirling.software.common.service.CustomPDFDocumentFactory; import stirling.software.common.util.*; -import static stirling.software.common.util.RegexPatternUtils.getColorPattern; - @Slf4j @RestController @RequestMapping("/api/v1/security") @@ -77,31 +77,39 @@ public class WatermarkController { if (opacity < 0.0f || opacity > 1.0f) { log.error("Opacity must be between 0.0 and 1.0, but got: {}", opacity); throw ExceptionUtils.createIllegalArgumentException( - "error.opacityOutOfRange" , //TODO - "Opacity must be between 0.0 and 1.0, but got: {0}", - opacity); + "error.opacityOutOfRange", // TODO + "Opacity must be between 0.0 and 1.0, but got: {0}", + opacity); } // Validate rotation range: rotationMin <= rotationMax Float rotationMin = request.getRotationMin(); Float rotationMax = request.getRotationMax(); if (rotationMin != null && rotationMax != null && rotationMin > rotationMax) { - log.error("Rotation minimum ({}) must be less than or equal to rotation maximum ({})", rotationMin, rotationMax); + log.error( + "Rotation minimum ({}) must be less than or equal to rotation maximum ({})", + rotationMin, + rotationMax); throw ExceptionUtils.createIllegalArgumentException( - "error.rotationRangeInvalid" , //TODO - "Rotation minimum ({0}) must be less than or equal to rotation maximum ({1})", - rotationMin, rotationMax); + "error.rotationRangeInvalid", // TODO + "Rotation minimum ({0}) must be less than or equal to rotation maximum ({1})", + rotationMin, + rotationMax); } // Validate font size range: fontSizeMin <= fontSizeMax Float fontSizeMin = request.getFontSizeMin(); Float fontSizeMax = request.getFontSizeMax(); if (fontSizeMin != null && fontSizeMax != null && fontSizeMin > fontSizeMax) { - log.error("Font size minimum ({}) must be less than or equal to font size maximum ({})", fontSizeMin, fontSizeMax); + log.error( + "Font size minimum ({}) must be less than or equal to font size maximum ({})", + fontSizeMin, + fontSizeMax); throw ExceptionUtils.createIllegalArgumentException( - "error.fontSizeRangeInvalid" , //TODO - "Font size minimum ({0}) must be less than or equal to font size maximum ({1})", - fontSizeMin, fontSizeMax); + "error.fontSizeRangeInvalid", // TODO + "Font size minimum ({0}) must be less than or equal to font size maximum ({1})", + fontSizeMin, + fontSizeMax); } // Validate color format when not using random color @@ -110,11 +118,13 @@ public class WatermarkController { if (customColor != null && !Boolean.TRUE.equals(randomColor)) { // Check if color is valid hex format (#RRGGBB or #RRGGBBAA) if (!getColorPattern().matcher(customColor).matches()) { - log.error("Invalid color format: {}. Expected hex format like #RRGGBB or #RRGGBBAA", customColor); + log.error( + "Invalid color format: {}. Expected hex format like #RRGGBB or #RRGGBBAA", + customColor); throw ExceptionUtils.createIllegalArgumentException( - "error.invalidColorFormat" , //TODO - "Invalid color format: {0}. Expected hex format like #RRGGBB or #RRGGBBAA", - customColor); + "error.invalidColorFormat", // TODO + "Invalid color format: {0}. Expected hex format like #RRGGBB or #RRGGBBAA", + customColor); } } @@ -122,11 +132,13 @@ public class WatermarkController { Float mirroringProbability = request.getMirroringProbability(); if (mirroringProbability != null && (mirroringProbability < 0.0f || mirroringProbability > 1.0f)) { - log.error("Mirroring probability must be between 0.0 and 1.0, but got: {}", mirroringProbability); + log.error( + "Mirroring probability must be between 0.0 and 1.0, but got: {}", + mirroringProbability); throw ExceptionUtils.createIllegalArgumentException( - "error.mirroringProbabilityOutOfRange" , //TODO - "Mirroring probability must be between 0.0 and 1.0, but got: {0}", - mirroringProbability); + "error.mirroringProbabilityOutOfRange", // TODO + "Mirroring probability must be between 0.0 and 1.0, but got: {0}", + mirroringProbability); } // Validate watermark type @@ -134,11 +146,12 @@ public class WatermarkController { if (watermarkType == null || (!watermarkType.equalsIgnoreCase("text") && !watermarkType.equalsIgnoreCase("image"))) { - log.error( "Watermark type must be 'text' or 'image', but got: {}", watermarkType); + log.error("Watermark type must be 'text' or 'image', but got: {}", watermarkType); throw ExceptionUtils.createIllegalArgumentException( - "error.unsupportedWatermarkType" , //TODO - "Watermark type must be 'text' or 'image', but got: {0}", - watermarkType); + "error.unsupportedWatermarkType", // TODO + "Watermark type must be ''text'' or ''image'', but got: {0}", // single quotes + // must be escaped + watermarkType); } // Validate text watermark has text @@ -147,8 +160,8 @@ public class WatermarkController { if (watermarkText == null || watermarkText.trim().isEmpty()) { log.error("Watermark text is required when watermark type is 'text'"); throw ExceptionUtils.createIllegalArgumentException( - "error.watermarkTextRequired", //TODO - "Watermark text is required when watermark type is 'text'"); + "error.watermarkTextRequired", // TODO + "Watermark text is required when watermark type is 'text'"); } } @@ -158,28 +171,32 @@ public class WatermarkController { if (watermarkImage == null || watermarkImage.isEmpty()) { log.error("Watermark image is required when watermark type is 'image'"); throw ExceptionUtils.createIllegalArgumentException( - "error.watermarkImageRequired", //TODO - "Watermark image is required when watermark type is 'image'"); + "error.watermarkImageRequired", // TODO + "Watermark image is required when watermark type is 'image'"); } // Validate image type - only allow common image formats String contentType = watermarkImage.getContentType(); String originalFilename = watermarkImage.getOriginalFilename(); if (contentType != null && !isSupportedImageType(contentType)) { - log.error("Unsupported image type: {}. Supported types: PNG, JPG, JPEG, GIF, BMP", contentType); + log.error( + "Unsupported image type: {}. Supported types: PNG, JPG, JPEG, GIF, BMP", + contentType); throw ExceptionUtils.createIllegalArgumentException( - "error.unsupportedContentType", //TODO - "Unsupported image type: {0}. Supported types: PNG, JPG, JPEG, GIF, BMP", - contentType); + "error.unsupportedContentType", // TODO + "Unsupported image type: {0}. Supported types: PNG, JPG, JPEG, GIF, BMP", + contentType); } // Additional check based on file extension if (originalFilename != null && !hasSupportedImageExtension(originalFilename)) { - log.error("Unsupported image file extension in: {}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp", originalFilename); + log.error( + "Unsupported image file extension in: {}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp", + originalFilename); throw ExceptionUtils.createIllegalArgumentException( - "error.unsupportedImageFileType", //TODO - "Unsupported image file extension in: {0}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp", - originalFilename); + "error.unsupportedImageFileType", // TODO + "Unsupported image file extension in: {0}. Supported extensions: .png, .jpg, .jpeg, .gif, .bmp", + originalFilename); } } diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/security/WatermarkValidationTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/security/WatermarkValidationTest.java index 65c4a301f..24cf40722 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/security/WatermarkValidationTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/security/WatermarkValidationTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; import java.util.Collections; +import java.util.Set; import org.apache.pdfbox.pdmodel.PDDocument; import org.junit.jupiter.api.BeforeEach; @@ -17,6 +18,11 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.mock.web.MockMultipartFile; import org.springframework.web.multipart.MultipartFile; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; + import stirling.software.SPDF.model.api.security.AddWatermarkRequest; import stirling.software.common.service.CustomPDFDocumentFactory; @@ -29,12 +35,11 @@ class WatermarkValidationTest { @InjectMocks private WatermarkController watermarkController; private AddWatermarkRequest request; - private MockMultipartFile mockPdfFile; @BeforeEach void setUp() throws Exception { request = new AddWatermarkRequest(); - mockPdfFile = + MockMultipartFile mockPdfFile = new MockMultipartFile( "fileInput", "test.pdf", "application/pdf", "test content".getBytes()); request.setFileInput(mockPdfFile); @@ -626,86 +631,431 @@ class WatermarkValidationTest { @DisplayName("Annotation-based Validation Tests") class AnnotationBasedValidationTests { + private Validator validator; + + @BeforeEach + void setUpValidator() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + validator = factory.getValidator(); + } + @Test - @DisplayName("Should enforce count minimum of 1") + @DisplayName("Should reject count below minimum of 1") void testCountMinimum() { - // Note: This tests the @Min annotation on count field - // The actual validation happens at the framework level + // Arrange request.setCount(0); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("count")), + "Should have violation on 'count' field"); } @Test - @DisplayName("Should enforce count maximum of 1000") + @DisplayName("Should accept valid count value") + void testCountValid() { + // Arrange + request.setCount(5); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch(v -> v.getPropertyPath().toString().equals("count")), + "Should have no violations on 'count' field"); + } + + @Test + @DisplayName("Should reject count above maximum of 1000") void testCountMaximum() { - // Note: This tests the @Max annotation on count field + // Arrange request.setCount(1001); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("count")), + "Should have violation on 'count' field"); } @Test - @DisplayName("Should enforce rotation min/max bounds of -360 to 360") - void testRotationBounds() { - // Note: This tests the @DecimalMin/@DecimalMax annotations + @DisplayName("Should reject rotationMin below -360") + void testRotationMinBelowBound() { + // Arrange request.setRotationMin(-361f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("rotationMin")), + "Should have violation on 'rotationMin' field"); + } + + @Test + @DisplayName("Should reject rotationMax above 360") + void testRotationMaxAboveBound() { + // Arrange request.setRotationMax(361f); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("rotationMax")), + "Should have violation on 'rotationMax' field"); } @Test - @DisplayName("Should enforce font size bounds of 1.0 to 500.0") - void testFontSizeBounds() { - // Note: This tests the @DecimalMin/@DecimalMax annotations + @DisplayName("Should accept valid rotation values") + void testRotationValid() { + // Arrange + request.setRotationMin(-180f); + request.setRotationMax(180f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch( + v -> + v.getPropertyPath().toString().equals("rotationMin") + || v.getPropertyPath() + .toString() + .equals("rotationMax")), + "Should have no violations on rotation fields"); + } + + @Test + @DisplayName("Should reject fontSizeMin below 1.0") + void testFontSizeMinBelowBound() { + // Arrange request.setFontSizeMin(0.5f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("fontSizeMin")), + "Should have violation on 'fontSizeMin' field"); + } + + @Test + @DisplayName("Should reject fontSizeMax above 500.0") + void testFontSizeMaxAboveBound() { + // Arrange request.setFontSizeMax(501f); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("fontSizeMax")), + "Should have violation on 'fontSizeMax' field"); } @Test - @DisplayName("Should enforce per-letter font count bounds of 1 to 20") - void testPerLetterFontCountBounds() { - // Note: This tests the @Min/@Max annotations + @DisplayName("Should accept valid font size values") + void testFontSizeValid() { + // Arrange + request.setFontSizeMin(10f); + request.setFontSizeMax(100f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch( + v -> + v.getPropertyPath().toString().equals("fontSizeMin") + || v.getPropertyPath() + .toString() + .equals("fontSizeMax")), + "Should have no violations on font size fields"); + } + + @Test + @DisplayName("Should reject perLetterFontCount below 1") + void testPerLetterFontCountMinimum() { + // Arrange request.setPerLetterFontCount(0); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterFontCount")), + "Should have violation on 'perLetterFontCount' field"); + } + + @Test + @DisplayName("Should reject perLetterFontCount above 20") + void testPerLetterFontCountMaximum() { + // Arrange request.setPerLetterFontCount(21); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterFontCount")), + "Should have violation on 'perLetterFontCount' field"); } @Test - @DisplayName("Should enforce per-letter color count bounds of 1 to 20") - void testPerLetterColorCountBounds() { - // Note: This tests the @Min/@Max annotations + @DisplayName("Should accept valid perLetterFontCount value") + void testPerLetterFontCountValid() { + // Arrange + request.setPerLetterFontCount(5); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterFontCount")), + "Should have no violations on 'perLetterFontCount' field"); + } + + @Test + @DisplayName("Should reject perLetterColorCount below 1") + void testPerLetterColorCountMinimum() { + // Arrange request.setPerLetterColorCount(0); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterColorCount")), + "Should have violation on 'perLetterColorCount' field"); + } + + @Test + @DisplayName("Should reject perLetterColorCount above 20") + void testPerLetterColorCountMaximum() { + // Arrange request.setPerLetterColorCount(21); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterColorCount")), + "Should have violation on 'perLetterColorCount' field"); } @Test - @DisplayName("Should enforce margin bounds of 0.0 to 500.0") - void testMarginBounds() { - // Note: This tests the @DecimalMin/@DecimalMax annotations + @DisplayName("Should accept valid perLetterColorCount value") + void testPerLetterColorCountValid() { + // Arrange + request.setPerLetterColorCount(4); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch( + v -> + v.getPropertyPath() + .toString() + .equals("perLetterColorCount")), + "Should have no violations on 'perLetterColorCount' field"); + } + + @Test + @DisplayName("Should reject margin below 0.0") + void testMarginBelowBound() { + // Arrange request.setMargin(-1f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("margin")), + "Should have violation on 'margin' field"); + } + + @Test + @DisplayName("Should reject margin above 500.0") + void testMarginAboveBound() { + // Arrange request.setMargin(501f); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("margin")), + "Should have violation on 'margin' field"); } @Test - @DisplayName("Should enforce image scale bounds of 0.1 to 10.0") - void testImageScaleBounds() { - // Note: This tests the @DecimalMin/@DecimalMax annotations + @DisplayName("Should accept valid margin value") + void testMarginValid() { + // Arrange + request.setMargin(10f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch(v -> v.getPropertyPath().toString().equals("margin")), + "Should have no violations on 'margin' field"); + } + + @Test + @DisplayName("Should reject imageScale below 0.1") + void testImageScaleBelowBound() { + // Arrange request.setImageScale(0.05f); - request.setImageScale(11f); - // Framework validation would reject this before reaching controller + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("imageScale")), + "Should have violation on 'imageScale' field"); } @Test - @DisplayName("Should validate bounds format pattern") - void testBoundsFormatPattern() { - // Note: This tests the @Pattern annotation on bounds field - request.setBounds("invalid"); - // Framework validation would reject this before reaching controller + @DisplayName("Should reject imageScale above 10.0") + void testImageScaleAboveBound() { + // Arrange + request.setImageScale(11f); - request.setBounds("100,100,200,200"); // Valid format - // Framework validation would accept this + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("imageScale")), + "Should have violation on 'imageScale' field"); + } + + @Test + @DisplayName("Should accept valid imageScale value") + void testImageScaleValid() { + // Arrange + request.setImageScale(1.5f); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch(v -> v.getPropertyPath().toString().equals("imageScale")), + "Should have no violations on 'imageScale' field"); + } + + @Test + @DisplayName("Should reject invalid bounds format") + void testBoundsInvalidFormat() { + // Arrange + request.setBounds("invalid"); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertFalse(violations.isEmpty(), "Should have validation errors"); + assertTrue( + violations.stream() + .anyMatch(v -> v.getPropertyPath().toString().equals("bounds")), + "Should have violation on 'bounds' field"); + } + + @Test + @DisplayName("Should accept valid bounds format") + void testBoundsValidFormat() { + // Arrange + request.setBounds("100,100,200,200"); + + // Act + Set> violations = validator.validate(request); + + // Assert + assertTrue( + violations.stream() + .noneMatch(v -> v.getPropertyPath().toString().equals("bounds")), + "Should have no violations on 'bounds' field"); } } }