Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,12 @@ internal async Task<CollectionResult> BindComplexCollectionFromIndexes(

var boundCollection = new List<TElement?>();

// Track explicit index names for normalization when using finite indices.
List<string>? explicitIndexNames = indexNamesIsFinite ? new List<string>() : null;

foreach (var indexName in indexNames)
{
explicitIndexNames?.Add(indexName);
var fullChildName = ModelNames.CreateIndexModelName(bindingContext.ModelName, indexName);

ModelBindingResult? result;
Expand Down Expand Up @@ -394,20 +398,87 @@ internal async Task<CollectionResult> BindComplexCollectionFromIndexes(
bindingContext.ModelMetadata.ElementType));
}

// Normalize ModelState keys from explicit indices to sequential indices so that
// re-rendered views (which iterate collections sequentially) look up the correct
// ModelState values. Without this, non-sequential explicit indices like [0],[10],[1],[2]
// cause data corruption when the page is re-rendered after a validation failure.
if (indexNamesIsFinite && explicitIndexNames is not null)
{
NormalizeCollectionModelStateKeys(bindingContext.ModelState, bindingContext.ModelName, explicitIndexNames);
}

return new CollectionResult(boundCollection)
{
// If we're working with a fixed set of indexes then this is the format like:
//
// ?parameter.index=zero&parameter.index=one&parameter.index=two&parameter[zero]=0&parameter[one]=1&parameter[two]=2...
//
// We need to provide this data to the validation system so it can 'replay' the keys.
// But we can't just set ValidationState here, because it needs the 'real' model.
ValidationStrategy = indexNamesIsFinite ?
new ExplicitIndexCollectionValidationStrategy(indexNames) :
null,
// After normalizing explicit indices to sequential indices, the default
// sequential validation strategy is correct. For non-finite (sequential) indices,
// the default strategy is also used.
ValidationStrategy = null,
};
}

/// <summary>
/// Remaps ModelState entries from explicit (potentially non-sequential) collection indices
/// to sequential indices (0, 1, 2, ...) so that rendered views can correctly look up
/// ModelState values for collection elements.
/// </summary>
private static void NormalizeCollectionModelStateKeys(
ModelStateDictionary modelState,
string modelName,
List<string> explicitIndexNames)
{
// Check if any index differs from its sequential position.
var needsNormalization = false;
for (var i = 0; i < explicitIndexNames.Count; i++)
{
if (!string.Equals(explicitIndexNames[i], i.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal))
{
needsNormalization = true;
break;
}
}

if (!needsNormalization)
{
return;
}

// Collect all entries that need remapping. We snapshot the data before making any
// changes to avoid issues with overlapping indices (e.g., swapping [0] and [1]).
var entriesToRemap = new List<(string OldKey, string NewKey, object? RawValue, string? AttemptedValue)>();

for (var i = 0; i < explicitIndexNames.Count; i++)
{
var explicitIndex = explicitIndexNames[i];
var sequentialIndex = i.ToString(CultureInfo.InvariantCulture);

if (string.Equals(explicitIndex, sequentialIndex, StringComparison.Ordinal))
{
continue;
}

var explicitPrefix = ModelNames.CreateIndexModelName(modelName, explicitIndex);
var sequentialPrefix = ModelNames.CreateIndexModelName(modelName, sequentialIndex);

foreach (var kvp in modelState.FindKeysWithPrefix(explicitPrefix))
{
var oldKey = kvp.Key;
var newKey = string.Concat(sequentialPrefix, oldKey.AsSpan(explicitPrefix.Length));
entriesToRemap.Add((oldKey, newKey, kvp.Value!.RawValue, kvp.Value.AttemptedValue));
}
}

// Remove old entries first, then add new ones to avoid key conflicts.
foreach (var (oldKey, _, _, _) in entriesToRemap)
{
modelState.Remove(oldKey);
}

foreach (var (_, newKey, rawValue, attemptedValue) in entriesToRemap)
{
modelState.SetModelValue(newKey, rawValue, attemptedValue);
}
}

// Internal for testing.
internal sealed class CollectionResult
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,86 @@ public async Task BindComplexCollectionFromIndexes_FiniteIndexes()
// Assert
Assert.Equal(new[] { 42, 0, 200 }, collectionResult.Model.ToArray());

// This requires a non-default IValidationStrategy
var strategy = Assert.IsType<ExplicitIndexCollectionValidationStrategy>(collectionResult.ValidationStrategy);
Assert.Equal(new[] { "foo", "bar", "baz" }, strategy.ElementKeys);
// After normalization, explicit indices use the default sequential validation strategy
Assert.Null(collectionResult.ValidationStrategy);
}

[Fact]
public async Task BindComplexCollectionFromIndexes_NonSequentialNumericIndexes_NormalizesModelStateKeys()
{
// Arrange - reproduces https://github.com/dotnet/aspnetcore/issues/26217
// When explicit indices are non-sequential (e.g., [0],[10],[1],[2]), the ModelState keys
// must be normalized to sequential indices so that re-rendered views display correct values.
var valueProvider = new SimpleValueProvider
{
{ "someName[0]", "100" },
{ "someName[10]", "200" },
{ "someName[1]", "300" },
{ "someName[2]", "400" }
};
var bindingContext = GetModelBindingContext(valueProvider);
var modelState = bindingContext.ModelState;
var binder = new CollectionModelBinder<int>(CreateIntBinderWithModelState(), NullLoggerFactory.Instance);

// Act
var collectionResult = await binder.BindComplexCollectionFromIndexes(
bindingContext,
new[] { "0", "10", "1", "2" });

// Assert - collection is bound in explicit index order
Assert.Equal(new[] { 100, 200, 300, 400 }, collectionResult.Model.ToArray());

// ModelState keys should be normalized to sequential indices
Assert.True(modelState.TryGetValue("someName[0]", out var entry0));
Assert.Equal("100", entry0!.AttemptedValue);

Assert.True(modelState.TryGetValue("someName[1]", out var entry1));
Assert.Equal("200", entry1!.AttemptedValue);

Assert.True(modelState.TryGetValue("someName[2]", out var entry2));
Assert.Equal("300", entry2!.AttemptedValue);

Assert.True(modelState.TryGetValue("someName[3]", out var entry3));
Assert.Equal("400", entry3!.AttemptedValue);

// Old non-sequential keys should no longer exist
Assert.False(modelState.ContainsKey("someName[10]"));

// Uses default sequential validation strategy after normalization
Assert.Null(collectionResult.ValidationStrategy);
}

[Fact]
public async Task BindComplexCollectionFromIndexes_SequentialIndexes_NoNormalizationNeeded()
{
// Arrange - sequential indices should not trigger normalization
var valueProvider = new SimpleValueProvider
{
{ "someName[0]", "100" },
{ "someName[1]", "200" },
{ "someName[2]", "300" }
};
var bindingContext = GetModelBindingContext(valueProvider);
var modelState = bindingContext.ModelState;
var binder = new CollectionModelBinder<int>(CreateIntBinderWithModelState(), NullLoggerFactory.Instance);

// Act
var collectionResult = await binder.BindComplexCollectionFromIndexes(
bindingContext,
new[] { "0", "1", "2" });

// Assert
Assert.Equal(new[] { 100, 200, 300 }, collectionResult.Model.ToArray());

// ModelState keys should remain unchanged
Assert.True(modelState.TryGetValue("someName[0]", out var entry0));
Assert.Equal("100", entry0!.AttemptedValue);

Assert.True(modelState.TryGetValue("someName[1]", out var entry1));
Assert.Equal("200", entry1!.AttemptedValue);

Assert.True(modelState.TryGetValue("someName[2]", out var entry2));
Assert.Equal("300", entry2!.AttemptedValue);
}

[Fact]
Expand Down Expand Up @@ -526,6 +603,40 @@ private static IModelBinder CreateIntBinder()
});
}

private static IModelBinder CreateIntBinderWithModelState()
{
return new StubModelBinder(context =>
{
var value = context.ValueProvider.GetValue(context.ModelName);
if (value == ValueProviderResult.None)
{
return ModelBindingResult.Failed();
}

context.ModelState.SetModelValue(context.ModelName, value);

object valueToConvert = null;
if (value.Values.Count == 1)
{
valueToConvert = value.Values[0];
}
else if (value.Values.Count > 1)
{
valueToConvert = value.Values.ToArray();
}

var model = ModelBindingHelper.ConvertTo(valueToConvert, context.ModelType, value.Culture);
if (model == null)
{
return ModelBindingResult.Failed();
}
else
{
return ModelBindingResult.Success(model);
}
});
}

private static DefaultModelBindingContext CreateContext()
{
var actionContext = new ActionContext()
Expand Down
Loading