From 68194d3cb3c33038a457a1fb36304fcf08914a8b Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:42:07 +0200 Subject: [PATCH] fix(validation): guard null comparison bounds in comparison builders Passing a null bound to GreaterThan/LessThan/Between etc. called value.CompareTo(null), throwing NullReferenceException for non-null reference-type values. Short-circuit when the bound is null so the rule is satisfied instead of crashing. --- CHANGELOG.md | 1 + .../RuleBuilderExtensionsTests.cs | 65 +++++++++++++++++++ .../_fixtures/ComparableModel.cs | 19 ++++++ .../RuleBuilderExtensions.cs | 12 ++-- 4 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 src/request.validation.tests/_fixtures/ComparableModel.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9e9c8..65a9a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ To have a consistent experience across all packages, some public interfaces have - **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) - **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` +- **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) ### Removed diff --git a/src/request.validation.tests/RuleBuilderExtensionsTests.cs b/src/request.validation.tests/RuleBuilderExtensionsTests.cs index 5b5271a..c0d2cae 100644 --- a/src/request.validation.tests/RuleBuilderExtensionsTests.cs +++ b/src/request.validation.tests/RuleBuilderExtensionsTests.cs @@ -314,6 +314,71 @@ internal sealed class RuleBuilderExtensionsTests .Throws(); } + [Test] + public async Task I_can_validate_greater_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_greater_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.GreaterThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThan(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_less_than_or_equal_to_with_null_bound_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.LessThanOrEqualTo(null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + + [Test] + public async Task I_can_validate_between_with_null_bounds_without_throwing() + { + var validator = new PropertyValidator(model + => model.Value, rule => rule.Between(null, null)); + + var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); + var ignoredNull = validator.Validate(new ComparableModel()); + + await Assert.That(valid.IsValid).IsTrue(); + await Assert.That(ignoredNull.IsValid).IsTrue(); + } + [Test] public async Task I_can_see_it_throw_for_invalid_between_configuration() { diff --git a/src/request.validation.tests/_fixtures/ComparableModel.cs b/src/request.validation.tests/_fixtures/ComparableModel.cs new file mode 100644 index 0000000..9178228 --- /dev/null +++ b/src/request.validation.tests/_fixtures/ComparableModel.cs @@ -0,0 +1,19 @@ +// Copyright (c) The Geekeey Authors +// SPDX-License-Identifier: EUPL-1.2 + +namespace Geekeey.Request.Validation.Tests; + +internal sealed class ComparableModel +{ + public ComparableValue? Value { get; init; } +} + +internal sealed class ComparableValue : IComparable +{ + public int Number { get; init; } + + public int CompareTo(ComparableValue? other) + { + return Number.CompareTo(other!.Number); + } +} diff --git a/src/request.validation/RuleBuilderExtensions.cs b/src/request.validation/RuleBuilderExtensions.cs index 528b387..7e1a5db 100644 --- a/src/request.validation/RuleBuilderExtensions.cs +++ b/src/request.validation/RuleBuilderExtensions.cs @@ -170,7 +170,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) > 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) > 0, $"Value must be greater than {comparisonValue}."); } @@ -189,7 +189,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) >= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) >= 0, $"Value must be greater than or equal to {comparisonValue}."); } @@ -208,7 +208,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) < 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) < 0, $"Value must be less than {comparisonValue}."); } @@ -227,7 +227,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) <= 0, + return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) <= 0, $"Value must be less than or equal to {comparisonValue}."); } @@ -254,7 +254,9 @@ public static class RuleBuilderExtensions "Maximum value must be greater than or equal to minimum value."); } - return rule.Must(value => IsNull(value) || (value.CompareTo(minValue) >= 0 && value.CompareTo(maxValue) <= 0), + return rule.Must(value => IsNull(value) + || ((minValue is null || value.CompareTo(minValue) >= 0) + && (maxValue is null || value.CompareTo(maxValue) <= 0)), $"Value must be between {minValue} and {maxValue}."); }