IOHelper: Why not use HostingEnvironment when there is no HttpContext?

[enkelmedia]

Umbraco Verison: 8.0.0 -> 8.11.1

We've had issues with IIS virtual paths not being respected by Umbraco in some cases. It all boils down to IOHelper.MapPath (

Umbraco-CMS/src/Umbraco.Core/IO/IOHelper.cs

Line 81 in 0bd4dce

public static string MapPath(string path, bool useHttpContext)

) that will build the path based on the assembly path when not HttpContext is available.

There is a check here to see if there is a HttpContext available:

Umbraco-CMS/src/Umbraco.Core/IO/IOHelper.cs

Line 95 in 0bd4dce

if (useHttpContext && HttpContext.Current != null)

if there is no context the method uses "GetRootDirectorySafe()" which figures out the path based on the executing assembly.

It makes sense not to call HttpContext.Server.MapPath() when there i no HttpContext available but from my research HostingEnvironment.MapPath() will work even in scenarios when there is no HttpContext.

I've tried HostingEnvironment.MapPath() in the following scenarios and it resolves our virtual paths in all of them:

• Inside a web request (when HttpContext is available)
• Inside UmbracoApplication.Ctor (during startup)
• Inside UmbracoApplication.Init() (during startup)
• Inside a Composers Compose()-method (during startup)
• Inside a Components Initialize()-method (during startup)
• Inside a new thread (like Task.Run(()=> {}}). (during startup)

In each of my test I also execute the same check in a new thread (different Thread.CurrentThread.ManagedThreadId) and HostingEnvironment.MapPath resolves the paths in all these cases.

This is the output from my tests where I have a Virtual Path (~/cache) configured, this points to a network drive \web.shared.se\site\Cache.

Inside a web request

This code is triggered inside a MVC-controller

----------- Inside a web request thread -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: \\web.shared.se\site\Cache\
UmbracoIOHelper: \\web.shared.se\site\Cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

-----------Inside new thread/task from this web request thread -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

Inside UmbracoApplication.Ctor (during startup)

-----------Inside current thread 153 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

-----------Inside task thread 214 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

Inside UmbracoApplication.Init() (during startup)

-----------Inside current thread 153 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: \\web.shared.se\site\Cache\
UmbracoIOHelper: \\web.shared.se\site\Cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

-----------Inside task thread 297 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

Inside Composers Compose() (during startup)

-----------Inside current thread 162 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: \\web.shared.se\site\Cache\
UmbracoIOHelper: \\web.shared.se\site\Cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

-----------Inside task thread 153 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

Inside Components Initialize() (during startup)

-----------Inside current thread 162 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: \\web.shared.se\site\Cache\
UmbracoIOHelper: \\web.shared.se\site\Cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\
-----------Inside task thread 290 -------------
Relative Path: ~/cache/
HostingEnvironmentMapPath: \\web.shared.se\site\Cache\
ServerMapPath: n/a = exception
UmbracoIOHelper: C:\Development\site\Site.Web.UI\cache\
ByAssemblyPath: C:\Development\site\Site.Web.UI\cache\

My conclusion is that HostingEnvironment.MapPath() will work in all scenarios that I've tested, the check for the HttpContext inside IOHelper.MapPath() should not be needed - removing this an always use HostingEnvironement.MapPath would solve a lot of the usses we have related to virtual paths.

Maybe the comment in the code is some kind o legacy that was there when/if the code uses HttpContext.Current.Server.MapPath? Then this check would make sense but not it does not.

I'm proposing: Remove the check for HttpContext and resolve paths with HostingEnvironment.MapPath all the time.

I'm I missing something here? Any scenario that I have not tested that I should test?

Maybe this was added as a fallback for unit tests? There have to be a way to get around that and resolve Virtual Directories in IIS in the right way when Umbraco runs.

Cheers!

[Shazwazza]

HostingEnvironment isn't a silver bullet. In a lot of cases the Umbraco DLLs are being run outside of a web environment (i.e. command line and unit tests). There is no HostingEnvironment there. This could be mitigated by a check with HostingEnvironment.IsHosted.

If you have time to submit a PR to deal with all the cases please do!

[umbrabot]

Hi @enkelmedia,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[enkelmedia]

@Shazwazza Sounds fair! I’m thinking that the check for the HttpContext could be switch with a check for HostingEnvironment.IsHosted and we’ll keep the fallback to GetRootDirectorySafe().

Not sure about the “useHttpContext” parameter, it’s not really using the context today at all so the name of the setting doesn’t really make sense? What should we do with it?

[Shazwazza]

Just having a look at this method now, it seems its already doing all the things you want isn't it? https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/IO/IOHelper.cs#L81

Typically everything should just be using the single param overload here: https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/IO/IOHelper.cs#L111 public static string MapPath(string path) which passes in true for useHttpContext which then checks if there's an HttpContext.Current or if IsHosted (which checks HostingEnvironment.IsHosted) and then proceeds to use HostingEnvironment.MapPath if it can, else fallsback to manually putting together the path.

So this is pretty much doing exactly what you want, it's always going to use HostingEnvironment.MapPath if it's hosted.

[Shazwazza]

Ah, I think the only check that needs to change which seems like a bug is this one: if (useHttpContext && HttpContext.Current != null) which should just be if (useHttpContext) since useHttpContext is set to useHttpContext && IsHosted; which checks if there is a HttpContext.Current or if it's hosted.

[enkelmedia]

@Shazwazza That's what I was thinking as well, I verified this in a local install, applied the change and and executed the same tests as I did when posting the issue and the virtual path was resolved correctly in all the tests.