aspnet / AspNetWebStack

ASP.NET MVC 5.x, Web API 2.x, and Web Pages 3.x (not ASP.NET Core)
Other
858 stars 354 forks source link

MVC 5 | Use of System.IO.Path in DefaultDisplayMode may result in ArgumentException "Illegal characters in path" when HtmlHelper.DisplayFor or EditorFor is used over a sequence #195

Closed rjgotten closed 3 years ago

rjgotten commented 6 years ago

Version: ASP.NET MVC 5.2.6

Behavior encountered

Given the following:

  1. Use either HtmlHelper.DisplayFor or HtmlHelper.EditorFor over a generic type, e.g. using the in-built support to call these methods over an IEnumerable<T> sequence to render display or editor templates for each item in the sequence: @Html.DisplayFor(model => model.Items)
  2. There is a non-default DefaultDisplayMode instance registered with the DisplayModeProvider that is matched. (Note that the out-of-the-box configuration already has additional display modes registered for mobile devices.)

ASP.NET MVC will throw an uncaught [ArgumentException: Illegal characters in path.] originating from System.Web.WebPages.DefaultDisplayMode.TransformPath(String virtualPath, String suffix)

Expected behavior

ASP.NET MVC does not throw an uncaught exception regarding the use of illegal characters in a path.

Root cause analysis

The root cause is that said TransformPath method uses methods from System.IO.Path rather than System.Web.VirtualPathUtility to manipulate a virtual path and will throw above error when a path contains characters not supported by the OS. (:?*<>| on Windows, iirc.). These virtual paths are not guaranteed to only contain safe characters.

Virtual paths processed by the view lookup system contain the view name. For display and editor templates, valid view name candidates are generated by the TemplateHelpers.GetViewNames method. Some of these generated candidates are based on type names, including generic type names which may explain the illegal characters.

Available workarounds

  1. Don't use DisplayFor or EditorFor over generic types, in particular over sequences.
  2. Remove all DefaultDisplayMode instances from the DisplayModeProvider except the instance which has an empty string set as its name. When a DefaultDisplayMode has an empty name, the TransformPath method which contains the bad code is skipped.
  3. Create your own DisplayMode which does not have bad path handling and replace all default registrations with equivalents based on this fixed class.

IMPORTANT: You must always have a fallback display mode registered! Never leave it out, or ASP.NET MVC will no longer be able to find any views at all! (This is a huge, huge design flaw. But then again, the entire display modes feature is kind of an ill-conceived hack; so it's within lines of expectation, I guess?)

dougbu commented 6 years ago

Thanks for reporting this issue. I'll have a look…

mkArtakMSFT commented 3 years ago

Hi. Thanks for contacting us. We're closing this issue as there was not much community interest in this ask for quite a while now.