Fix FieldIdentifier.GetHashCode() NullReferenceException for default struct#65573
Fix FieldIdentifier.GetHashCode() NullReferenceException for default struct#65573kubaflo wants to merge 1 commit intodotnet:mainfrom
Conversation
|
Thanks for your PR, @@kubaflo. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
🤖 AI Fix Summary📋 Pre-FlightIssue: #45096 — 🧪 Tests (3 added)
🔬 Multi-Model Exploration (6 attempts, all passing)
Cross-pollination: 2 models reviewed — no new ideas. Exhausted: Yes. ✅ Fix Applied1 line changed in var fieldHash = FieldName is null ? 0 : StringComparer.Ordinal.GetHashCode(FieldName);81/81 tests pass — zero regressions. |
There was a problem hiding this comment.
Pull request overview
Fixes a crash in Microsoft.AspNetCore.Components.Forms.FieldIdentifier when default(FieldIdentifier) is used by making hashing behavior null-safe, and adds regression coverage in the Forms test suite.
Changes:
- Make
FieldIdentifier.GetHashCode()null-safe forFieldNamewhen the struct is default-initialized. - Add tests covering
default(FieldIdentifier)hashing and equality scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Components/Forms/src/FieldIdentifier.cs | Adjusts GetHashCode() to avoid exceptions when FieldName is null on default structs. |
| src/Components/Forms/test/FieldIdentifierTest.cs | Adds tests to validate default(FieldIdentifier) behavior (hashing and equality). |
| @@ -75,7 +75,7 @@ public override int GetHashCode() | |||
| { | |||
| // We want to compare Model instances by reference. RuntimeHelpers.GetHashCode returns identical hashes for equal object references (ignoring any `Equals`/`GetHashCode` overrides) which is what we want. | |||
| var modelHash = RuntimeHelpers.GetHashCode(Model); | |||
There was a problem hiding this comment.
GetHashCode() still calls RuntimeHelpers.GetHashCode(Model) without guarding against Model being null when the struct is default(FieldIdentifier). RuntimeHelpers.GetHashCode(null) throws, so default(FieldIdentifier).GetHashCode() will still throw despite the FieldName null-check. Consider also handling a null Model (e.g., using 0 or another sentinel hash) so the new default-struct tests pass and the method aligns with the PR’s intent.
| var modelHash = RuntimeHelpers.GetHashCode(Model); | |
| var modelHash = Model is null ? 0 : RuntimeHelpers.GetHashCode(Model); |
…struct default(FieldIdentifier) bypasses the constructor, leaving FieldName as null. StringComparer.Ordinal.GetHashCode(null) then throws NullReferenceException. Add null check in GetHashCode to handle default struct gracefully. Add tests for default(FieldIdentifier) GetHashCode, Equals, and inequality. Fixes dotnet#45096 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6f1b501 to
ba6cec5
Compare
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & ValidationIssue: #45096 - Blazor Value cannot be null. (Parameter 'obj') Key Findings
Test Command
Fix Candidates
🧪 Test — Bug ReproductionTest Result: ✅ TESTS CREATEDTest Command: Tests Written
ConclusionThe first test directly reproduces the bug: 🚦 Gate — Test Verification & RegressionGate Result: ✅ PASSEDTest Command: New Tests vs Buggy Code
Regression Check
ConclusionTests properly catch the bug. Fix applied: null-check FieldName in GetHashCode. All 81 tests pass. 🔧 Fix — Analysis & Comparison (✅ 6 passed)Fix Exploration SummaryTotal Attempts: 6 Attempt Results
Cross-Pollination
Exhausted: Yes Comparison
RecommendationAttempt 0 is best: minimal 1-line change, preserves existing StringComparer.Ordinal hash algorithm, clear intent, zero risk of behavioral changes for valid FieldIdentifiers. ✅ Attempt 0: PASSAttempt 0: Initial Fix (manual)Approach: Null-conditional check on FieldName in GetHashCode 📄 Diffdiff --git a/src/Components/Forms/src/FieldIdentifier.cs b/src/Components/Forms/src/FieldIdentifier.cs
index 5764b018e4..05d6430593 100644
--- a/src/Components/Forms/src/FieldIdentifier.cs
+++ b/src/Components/Forms/src/FieldIdentifier.cs
@@ -75,7 +75,7 @@ public readonly struct FieldIdentifier : IEquatable<FieldIdentifier>
{
// We want to compare Model instances by reference. RuntimeHelpers.GetHashCode returns identical hashes for equal object references (ignoring any `Equals`/`GetHashCode` overrides) which is what we want.
var modelHash = RuntimeHelpers.GetHashCode(Model);
- var fieldHash = StringComparer.Ordinal.GetHashCode(FieldName);
+ var fieldHash = FieldName is null ? 0 : StringComparer.Ordinal.GetHashCode(FieldName);
return (
modelHash,
fieldHash✅ Attempt 1: PASSApproach: Use
|
Summary
Fixes #45096 —
default(FieldIdentifier).GetHashCode()throwsNullReferenceExceptionRoot Cause
FieldIdentifieris areadonly struct. Usingdefault(FieldIdentifier)bypasses the constructor, leavingFieldNameasnull. TheGetHashCode()method callsStringComparer.Ordinal.GetHashCode(FieldName)which throwsNullReferenceExceptionwhenFieldNameisnull.Fix
Add a null check in
GetHashCode():Changes
src/Components/Forms/src/FieldIdentifier.cs— null-safeGetHashCode()(1 line change)src/Components/Forms/test/FieldIdentifierTest.cs— 3 new tests fordefault(FieldIdentifier)scenariosMulti-Model Exploration
is nullcheckFieldName?.GetHashCode(StringComparison.Ordinal) ?? 0FieldName ?? string.Emptyin GetHashCode+EqualsEqualityComparer<string>.Default.GetHashCode()HashCode.Combinewith null-coalescingSelected Attempt 0 — minimal 1-line change, preserves existing
StringComparer.Ordinalhash algorithm, clear intent, zero risk of behavioral changes for validFieldIdentifierinstances.Test Results
All 81 tests in
Microsoft.AspNetCore.Components.Forms.Testspass with zero regressions.