Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Fix for #1681 - Model state errors are wrong with attributes like [Fr…
Browse files Browse the repository at this point in the history
…omHeader]

The issue here is that a model state error is added with the model name
'doubled'. This is on a fairly obscure code path and the code dates back
to the original WSR git checkin of webapi. There are no wsr tests that
verify this behavior.

The cause here is that our 'greedy' model binders (like
FromHeaderModelBinder) return 'true' whether or not they successfully
found a model, because they don't want other model binders to run.

This also has an effect on the validation system. That means that
validators will run and attempt to validate the model (which may be null).
That's that rare case where we get to this code path.
  • Loading branch information
rynowak committed Dec 10, 2014
1 parent b1b5414 commit 1631ca1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,9 @@ private void ValidateThis(ModelValidationContext validationContext, ModelValidat
// If the Model at the current node is null and there is no parent, we cannot validate, and the
// DataAnnotationsModelValidator will throw. So we intercept here to provide a catch-all value-required
// validation error
var modelStateKey = ModelBindingHelper.CreatePropertyModelName(ModelStateKey,
ModelMetadata.GetDisplayName());
if (parentNode == null && ModelMetadata.Model == null)
{
modelState.TryAddModelError(modelStateKey, Resources.Validation_ValueNotFound);
modelState.TryAddModelError(ModelStateKey, Resources.Validation_ValueNotFound);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,9 @@ public async Task FromHeader_BindHeader_ToString_OnParameter_NoValues(string hea

Assert.Null(result.HeaderValue);
Assert.Null(result.HeaderValues);

// This is a bug - the model state error key is wrong here.

var error = Assert.Single(result.ModelStateErrors);
Assert.Equal("transactionId.transactionId", error);
Assert.Equal("transactionId", error);
}

// The action that this test hits will echo back the model-bound values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#if ASPNET50
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
Expand Down Expand Up @@ -245,6 +246,27 @@ public void Validate_SkipsValidationIfSuppressed()
Assert.Empty(log);
}

[Fact]
public void Validate_ValidatesIfModelIsNull()
{
// Arrange
var modelMetadata = GetModelMetadata(typeof(ValidateAllPropertiesModel));
var node = new ModelValidationNode(modelMetadata, "theKey");

var context = CreateContext(modelMetadata);

// Act
node.Validate(context);

// Assert
var modelState = Assert.Single(context.ModelState);
Assert.Equal("theKey", modelState.Key);
Assert.Equal(ModelValidationState.Invalid, modelState.Value.ValidationState);

var error = Assert.Single(modelState.Value.Errors);
Assert.Equal("A value is required but was not present in the request.", error.ErrorMessage);
}

[Fact]
[ReplaceCulture]
public void Validate_ValidateAllProperties_AddsValidationErrors()
Expand Down Expand Up @@ -321,6 +343,11 @@ private static ModelMetadata GetModelMetadata(object o)
return new DataAnnotationsModelMetadataProvider().GetMetadataForType(() => o, o.GetType());
}

private static ModelMetadata GetModelMetadata(Type type)
{
return new DataAnnotationsModelMetadataProvider().GetMetadataForType(modelAccessor: null, modelType: type);
}

private static ModelValidationContext CreateContext(ModelMetadata metadata = null)
{
var providers = new IModelValidatorProvider[]
Expand Down

0 comments on commit 1631ca1

Please sign in to comment.