diff --git a/IconHelper.Test/ArgumentsTests.cs b/IconHelper.Test/ArgumentsTests.cs index 4808371..dee59b1 100644 --- a/IconHelper.Test/ArgumentsTests.cs +++ b/IconHelper.Test/ArgumentsTests.cs @@ -77,6 +77,54 @@ public void ValidateRejectsPaddingEqualToHalfTheSize() Assert.AreEqual("Padding must be less than half the size of the image.", errors[0]); } + [TestMethod] + public void ValidateRejectsNegativePadding() + { + // The upper bound never caught this: -5 >= 64 is false. The run then produced an output + // larger than the documented min(squareSize, size) canvas and exited 0 to say it worked. + using TempDirectory temp = new(); + Arguments args = ValidArguments(temp); + args.Size = 128; + args.Padding = -5; + + bool valid = args.Validate(out Collection errors); + + Assert.IsFalse(valid, "Negative padding grows the canvas instead of insetting content."); + Assert.HasCount(1, errors); + Assert.AreEqual("Padding must not be negative.", errors[0]); + } + + [TestMethod] + public void ValidateRejectsASizeThatIsNotPositive() + { + using TempDirectory temp = new(); + Arguments args = ValidArguments(temp); + args.Size = 0; + args.Padding = 0; + + bool valid = args.Validate(out Collection errors); + + Assert.IsFalse(valid, "A size of zero leaves no canvas at all."); + Assert.HasCount(1, errors, $"Expected only the size error but got: {string.Join(", ", errors)}"); + Assert.AreEqual("Size must be greater than zero.", errors[0]); + } + + [TestMethod] + public void ValidateRejectsANegativeSizeAndPaddingTogether() + { + // The combination the upper bound was least able to catch: -20 >= -10 / 2 is false, so + // both values passed validation untouched and the pair reached the pipeline. + using TempDirectory temp = new(); + Arguments args = ValidArguments(temp); + args.Size = -10; + args.Padding = -20; + + bool valid = args.Validate(out Collection errors); + + Assert.IsFalse(valid, "Neither a negative size nor a negative padding is usable."); + Assert.HasCount(2, errors, $"Expected a size and a padding error but got: {string.Join(", ", errors)}"); + } + [TestMethod] public void ValidateUsesIntegerDivisionForOddSizes() { diff --git a/IconHelper/Arguments.cs b/IconHelper/Arguments.cs index fc7022c..0fc97ca 100644 --- a/IconHelper/Arguments.cs +++ b/IconHelper/Arguments.cs @@ -30,7 +30,27 @@ internal sealed class Arguments internal bool Validate(out Collection errors) { errors = []; - if (Padding >= Size / 2) + + if (Size <= 0) + { + errors.Add("Size must be greater than zero."); + } + + // The upper bound alone was not a bound. A negative padding sails past it -- -5 >= 64 is + // false -- and then EffectivePadding hands it back unchanged, so the content is resized to + // finalSize - padding * 2, which is larger than the canvas. ImageSharp's Pad never shrinks + // an image, so the output comes out bigger than the documented min(squareSize, size), + // quietly and with exit code 0. Reporting it here rather than clamping it downstream keeps + // the documented invariant -- padding insets content, it never changes the canvas -- true + // by construction. + // + // Reported on its own, because "less than half the size" is not the complaint about a + // negative number, and half of an invalid size is not a bound worth quoting either. + if (Padding < 0) + { + errors.Add("Padding must not be negative."); + } + else if (Size > 0 && Padding >= Size / 2) { errors.Add("Padding must be less than half the size of the image."); } diff --git a/README.md b/README.md index b186266..5cad723 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,10 @@ the artwork leaves it alone. `finalSize - padding * 2` and then padded back out to `finalSize`, so the output is always `finalSize` square whatever the padding. +That holds only while the padding is positive, so a negative `--padding` is rejected rather than +clamped: it would resize the content *larger* than the canvas, and padding back out never shrinks +an image. `--size` must likewise be greater than zero. + Because the canvas is `min(squareSize, --size)` rather than `--size`, the padding is measured against that effective canvas and **clamped to `(finalSize - 1) / 2`** so at least one pixel of content survives. This only engages on artwork that trims smaller than `--size`: `--size 100 --padding 45`