From fbbe94bfa8caa65fe7e8fb568e4e82d1d8051194 Mon Sep 17 00:00:00 2001 From: Louis Seubert Date: Sun, 12 Jul 2026 18:32:16 +0200 Subject: [PATCH] fix(result): Result hash collision A success result wrapping a value whose GetHashCode is 0 (e.g. Result with 0) collides with a failure result, both hashing to 0. --- CHANGELOG.md | 1 + .../ResultEqualityTests.cs | 70 ++++++++++++++++++- src/request.result/Result.Equality.cs | 7 +- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b08db3..a7f9bdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ To have a consistent experience across all packages, some public interfaces have ### Fixed - **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` ### Removed diff --git a/src/request.result.tests/ResultEqualityTests.cs b/src/request.result.tests/ResultEqualityTests.cs index 67233dd..ad54683 100644 --- a/src/request.result.tests/ResultEqualityTests.cs +++ b/src/request.result.tests/ResultEqualityTests.cs @@ -166,11 +166,28 @@ internal sealed class ResultEqualityTests } [Test] - public async Task I_can_get_hashcode_and_get_zero_for_failure() + public async Task I_can_get_hashcode_and_get_non_zero_for_failure() { var result = Prelude.Failure("error"); - await Assert.That(result.GetHashCode()).IsZero(); + await Assert.That(result.GetHashCode()).IsNotEqualTo(0); + } + + [Test] + public async Task I_can_get_hashcode_and_not_collide_non_generic_success_and_failure() + { + await Assert.That(Prelude.Success().GetHashCode()) + .IsNotEqualTo(Prelude.Failure("error").GetHashCode()); + } + + [Test] + public async Task I_can_get_hashcode_and_not_collide_success_value_zero_with_failure() + { + var success = Prelude.Success(0); + var failure = Prelude.Failure("error"); + + await Assert.That(success.Equals(failure)).IsFalse(); + await Assert.That(success.GetHashCode()).IsNotEqualTo(failure.GetHashCode()); } [Test] @@ -199,4 +216,53 @@ internal sealed class ResultEqualityTests await Assert.That(a.Equals(b)).IsFalse(); } + + [Test] + public async Task I_can_use_equality_operator_and_get_false_for_success_and_failure() + { + var success = Prelude.Success(2); + var failure = Prelude.Failure("error"); + + await Assert.That(success == failure).IsFalse(); + await Assert.That(success != failure).IsTrue(); + } + + [Test] + public async Task I_can_use_equality_operator_and_get_true_for_successes_with_equal_value() + { + await Assert.That(Prelude.Success(2) == Prelude.Success(2)).IsTrue(); + await Assert.That(Prelude.Success(2) != Prelude.Success(3)).IsTrue(); + } + + [Test] + public async Task I_can_use_equality_operator_and_compare_result_to_value() + { + await Assert.That(Prelude.Success(2) == 2).IsTrue(); + await Assert.That(Prelude.Success(2) != 3).IsTrue(); + await Assert.That(Prelude.Failure("x") == 2).IsFalse(); + } + + [Test] + public async Task I_can_equal_object_and_get_false_for_null() + { + object result = Prelude.Success(2); + + await Assert.That(result.Equals(null)).IsFalse(); + } + + [Test] + public async Task I_can_equal_object_and_get_false_for_wrong_type() + { + object result = Prelude.Success(2); + + await Assert.That(result.Equals("not a result")).IsFalse(); + } + + [Test] + public async Task I_can_equal_object_and_get_true_for_boxed_equal_result() + { + object result = Prelude.Success(2); + + await Assert.That(result.Equals(Prelude.Success(2))).IsTrue(); + } } diff --git a/src/request.result/Result.Equality.cs b/src/request.result/Result.Equality.cs index 0253b88..2a8cbbb 100644 --- a/src/request.result/Result.Equality.cs +++ b/src/request.result/Result.Equality.cs @@ -181,7 +181,12 @@ public partial class Result : IEquatable>, IEquatable return comparer.GetHashCode(result.Value); } - return 0; + if (result is { IsSuccess: true, Value: null }) + { + return 0; + } + + return result.Error?.GetHashCode() ?? 0; } }