Relative Controller View routing inconsistent with 2.0 update.

[chad-ramos]

Issue

Prior to 2.x, scoping to a view like so...

return View("../Search/Index", new List<Post>());

...was valid when publishing assets produced by...

dotnet publish -c Release

Details

• I have been running published assets in production for about a year with relative routing to Views, using a 1.x build, and have had no issues.
  • Of which lives on a Ubuntu server.
• After updating to 2.x, the following behavior became reproducible.
  • Out of Visual Studio...
    • Running a Debug or Release build in IISExpress still produced valid results with relative routing to Views.
  • When producing deployments asset with dotnet publish -c Release and deploying them, relative routing fails and ultimately produces a 404.
  • Upon removing relative routing like so...

return View("~/Views/Search/Index.cshtml", new List<Post>());

• ...and producing deployments asset with dotnet publish -c Release, previously mentioned error(s) are no longer present.

Expected Behaviour

I have zero issues with removing relative pathing to my Views. That being said, there appear to be some inconsistencies that would lead to confusion.

• Is relative pathing something that was intended?
  • If so, based on this Issue, it appears not to work.
  • If not, for some reason it is possible.

CC - @DamianEdwards @pranavkm

Best,

Chad

[pranavkm]

@chad-ramos, here's the convention for view discovery:

• Any name with an extension (.cshtml) is treated as path.
• Paths starting with ~/ (e.g. ~/Views/Home/Index.cshtml) are content root relative.
• Paths without a ~/ are relative to the current executing file path. If no view is currently executing, e.g. if it's a view returned from a controller, the current executing file path is the content root.

If you intended to return a relative path, you have to do return View("../Search/Index.cshtml", new List<Post>());

Further details

When you run your application using dotnet run or in your IDE, the view engine uses PhysicalFileProvider which is essentially an abstraction over System.IO.File. It happily resolves File.Open("Views/{ControllerName}/../Search/Index.cshtml"). On the other hand, EmbeddedFileProvider and precompilation perform dictionary lookups for an entry where the key isViews/Search/Index.cshtml. You can see why that fails.

@Eilon, the view engine does work to resolve path traversals when looking up paths i.e. GetView. Should we

(a) do something similar when doing a FindView lookup where they're meant to be view names?
(b) throw to let the user know that they are passing in the wrong value?
(c) Infer the presence of .. as paths? IMO, this is my least favorite option since the lookup behavior is complicated as it is.

[chad-ramos]

@pranavkm

Makes sense and explains why pre-compilation causes an error.

Few things...

• Based on your statement, are you explicitly stating an extension ( .cshtml) is required for non-content root relative pathing to resolve?
  • If so, this is not the behavior I am seeing as I do not need to provide the extension in the valid cases I have identified.
  • If not, what value if any is there in allowing the extension to be inferred?
  • What not instead always require it and eliminate the guessing game.
• Based on an identified issue with pre-compilation not producing errors/warnings when a non-content root relative path is provided....
  • Why are you allowing non-content root relative paths at all?
    • What value if any does this have?
      • If none, why not require ~/ as the convention in all cases.
      • If there is value, then work needs to be done to, as mentioned, throw an error or account for the case and resolve it.

Best,

Chad

[pranavkm]

@chad-ramos the view engine allows lookup by names for instance when you do return View("Index") or Layout = "_Layout" and due to historic behavior also things that look like paths. For instance return View("Index/NonAdmin") is just as valid and changing it now would be breaking. Throwing for non-deterministic cases might be the least breaking of the behaviors.

[chad-ramos]

@pranavkm Fair enough. Based on that, yes, it would appear throwing for nondeterministic cases would be the desired result.

Thanks,

Chad

[Eilon]

What exactly are the "non-deterministic" cases we're talking about here? What exactly are we proposing that we could change in the behavior?

[pranavkm]

@Eilon return View("../DifferentLocation/Blah") (note the absence of an extension). We treat this as a name rather than as a part even though it's meant to represent a path.

[pranavkm]

@Eilon yet another variant is when you pass do FindView("Home/Index") with a null controller name. This is commonly encountered as part of rendering razor views to strings (https://ppolyzos.com/2016/09/09/asp-net-core-render-view-to-string). The strings the view engine eventually ends up is /Views//Home/Index.cshtml. PhysicalFileProvider ignores the extra path separator, but the dictionary lookups we do for precompiled views and embedded views don't like this. This one's not as cut and dry as the other issue because looking up a view by names containing slashes e.g. return View("SubDir/MyView") is valid and supported (❔). So we couldn't error this out the way we could for ../ slashes.

[Eilon]

@pranavkm - do we have some existing code for doing path canonicalization? It seems that if the system always goes through some sort of canonicalization then both runtime-compiled and pre-compiled views would work.

[pranavkm]

@Eilon we spoke about addressing this by treating things with slashes as paths.

[pranavkm]

We addressed this issue by resolving the value FindView passes in to the page factory. Where we previously would have invoked it with Views/Home/../Search/Index.cshtml, we do path resolution and pass in Views/Search/Index.cshtml. This is close to what we do in GetView and should hide differences in the implementation of the underlying file provider.