ExpressionTextCache is case insensitive: TextboxFor(x=>x.name) and TextboxFor(x=>x.Name) produce the same html

[IGionny]

I think that there is a bug with ExpressionTextCache used to manage expressions into Html.EditorFor...IF i visti a page with Html.EditorFor(x=>x.name) AND after a page with Html.EditorFor(x=>x.Name) (in 2 different model). ..the result will be the same: <input id="name" name="name" ...all in lowercase (like the first lamda expression resolved).

I repro the problem via a test website with 2 model:
public class Product { public string Name { get; set; } }
public class ProductLower { public string name { get; set; } }

A controller with 2 actions:
public IActionResult Product() { var model = new Product(); return View(model); }

public IActionResult ProductLower() { var model = new ProductLower(); return View(model); }

and 2 views:

== [Product] ==

@{
    ViewData["Title"] = "Home Page";
}
@model Example_Models.Models.Product

PRODUCT<br />

@Html.EditorFor(x => x.Name)

== [ProductLower] ==

@{
    ViewData["Title"] = "Home Page";
}
@model Example_Models.Models.ProductLower


LOWER<br />

@Html.EditorFor(x => x.name)

If you visit ProductLower page and AFTER the Product page, the output of the @Html.EditorFor will be the same: <input id='name' name='name'...

I also take the tests for the ExpressionTextCache and put the 'wrong-case' to proof this:

//https://github.com/aspnet/Mvc/blob/2cabd589ac6a2fcd88b3c23aa50541536d2c8b71/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs

edit the NonEquivalentExpressions adding

(Expression<Func<TestLowerModel, string>>) (model => model.name),
(Expression<Func<TestModel, string>>) (model => model.Name)

full code:

//https://github.com/aspnet/Mvc/blob/2cabd589ac6a2fcd88b3c23aa50541536d2c8b71/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ExpressionHelperTest.cs
    public class ExpressionTextCacheTests
    {
        private readonly ExpressionTextCache _expressionTextCache = new ExpressionTextCache();

        public static TheoryData<Expression, Expression> NonEquivalentExpressions
        {
            get
            {
                var value = "test";
                var key = "TestModel";
                var Model = "Test";
                var myModel = new TestModel();

                return new TheoryData<Expression, Expression>
                {

                   //ADDED CASE
                    {
                        (Expression<Func<TestLowerModel, string>>) (model => model.name),
                        (Expression<Func<TestModel, string>>) (model => model.Name)
                    },


                    {
                        (Expression<Func<TestModel, Category>>) (model => model.SelectedCategory),
                        (Expression<Func<TestModel, CategoryName>>) (model => model.SelectedCategory.CategoryName)
                    },
                    {
                        (Expression<Func<TestModel, string>>) (model => model.Model),
                        (Expression<Func<TestModel, string>>) (model => model.Name)
                    },
                    {
                        (Expression<Func<TestModel, CategoryName>>) (model => model.SelectedCategory.CategoryName),
                        (Expression<Func<TestModel, string>>) (model => value)
                    },
                    {
                        (Expression<Func<TestModel, string>>) (testModel => testModel.SelectedCategory.CategoryName
                            .MainCategory),
                        (Expression<Func<TestModel, string>>) (testModel => value)
                    },
                    {
                        (Expression<Func<IList<TestModel>, Category>>) (model => model[2].SelectedCategory),
                        (Expression<Func<TestModel, string>>)
                        (model => model.SelectedCategory.CategoryName.MainCategory)
                    },
                    {
                        (Expression<Func<TestModel, int>>) (testModel => testModel.SelectedCategory.CategoryId),
                        (Expression<Func<TestModel, Category>>) (model => model.SelectedCategory)
                    },
                    {
                        (Expression<Func<IDictionary<string, TestModel>, string>>) (model => model[key].SelectedCategory
                            .CategoryName.MainCategory),
                        (Expression<Func<TestModel, Category>>) (model => model.SelectedCategory)
                    },
                    {
                        (Expression<Func<TestModel, string>>) (m => Model),
                        (Expression<Func<TestModel, string>>) (m => m.Model)
                    },
                    {
                        (Expression<Func<TestModel, TestModel>>) (m => m),
                        (Expression<Func<TestModel, string>>) (m => m.Model)
                    },
                    {
                        (Expression<Func<TestModel, string>>) (m => myModel.Name),
                        (Expression<Func<TestModel, string>>) (m => m.Name)
                    },
                    {
                        (Expression<Func<TestModel, string>>) (m => key),
                        (Expression<Func<TestModel, string>>) (m => value)
                    }
                };
            }
        }

        [Theory]
        [MemberData(nameof(NonEquivalentExpressions))]
        public void GetExpressionText_CheckNonEquivalentExpressions(LambdaExpression expression1,
            LambdaExpression expression2)
        {
            // Act - 1
            var text1 = ExpressionHelper.GetExpressionText(expression1, _expressionTextCache);

            // Act - 2
            var text2 = ExpressionHelper.GetExpressionText(expression2, _expressionTextCache);

            // Assert
            Assert.NotEqual(text1, text2, StringComparer.Ordinal);
            Assert.NotSame(text1, text2);
        }

        private class TestLowerModel
        {
            public string name { get; set; }
        }

        private class TestModel
        {
            public string Name { get; set; }
            public string Model { get; set; }
            public Category SelectedCategory { get; set; }

            public IList<Category> PreferredCategories { get; set; }
        }

        private class Category
        {
            public int CategoryId { get; set; }
            public CategoryName CategoryName { get; set; }
        }

        private class CategoryName
        {
            public string MainCategory { get; set; }
            public string SubCategory { get; set; }
        }
    }

I found a workaround:

public class FixedHtmlHelper<TModel> : HtmlHelper<TModel>
   {
       public FixedHtmlHelper(IHtmlGenerator htmlGenerator, ICompositeViewEngine viewEngine,
           IModelMetadataProvider metadataProvider, IViewBufferScope bufferScope, HtmlEncoder htmlEncoder,
           UrlEncoder urlEncoder, ExpressionTextCache expressionTextCache) : base(htmlGenerator, viewEngine,
           metadataProvider, bufferScope, htmlEncoder, urlEncoder,

//NEW INSTANCE EACH TIME
new FixedExpressionTextCache())
       {
       }
   }

In startup.cs:

 public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc();
            services.AddTransient(typeof(IHtmlHelper<>), typeof(FixedHtmlHelper<>));

[Eilon]

Hmm this does seem to be incorrect. I agree that the casing should be ignored when generating these accessors/caches.

@rynowak / @dougbu - thoughts on this? Just a bug? (Or unintended consequence.)

[dougbu]

@Eilon this looks like a minor bug; at best an incorrect assumption of CLS-compliance in MVC applications. Questionable code at line 73 of ExpressionTextCache.

Keys are treated case-insensitively throughout model binding; property names that differ only in case will have problems in almost every end-to-end scenario. Since that aspect of model binding is intentional, I'm not sure whether fixing this bug would matter much.

[Eilon]

I don't think this has anything to do with CLS compliance because this is a case where two different types have a property that varies by case. CLS compliance only comes into play when the same types has two properties that vary by case.

[dougbu]

Sorry, I missed that.

Thinking aloud: The cache is intended to detect expressions with a similar-enough "shape" that they map to the same text. Could for example confirm the containing types match if the last expression node we check is for a property. But, that seems odd and would diminish the usefulness of the cache e.g. lots of model types have Id properties.

When the property names differ in case, the resulting text is going to be different -- nothing lowercases the generated names. Ordinal string comparisons are also more efficient.

Bottom line, the bug is more important than I thought and there's no real downside to changing that line in ExpressionTextCache.

[IGionny]

Hi,

any news about this issue?
To me it is a huge bug because it can be a show-stopper: we have a json-schema web editor that uses all the properties in lowercase and other edit pages for other models with pascalcase... 'Name' is a common property.
All the client side code (aka javascript/typescript) is case sensitive (example: the jquery id selector #Name != #name): when the application restart, the first edit page works, the other ones don't.

Thank you for your time.

[dougbu]

@IGionny this issue has not yet been triaged. The team is currently focused on stabilizing our builds.

@Eilon fix would be a one-liner plus a few new cases in existing data sets.

[Eilon]

Yeah let's just get this fixed.