aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Add equivalent of HttpRuntime.AppDomainAppId available at all times when hosted in IIS

Open yaakov-h opened this issue 3 years ago • 6 comments

Moved from #43631.

Background and Motivation

When an ASP.NET Core application is hosted in IIS, IIS gives it an application ID, the same way it gives every other application an application ID. This contains the site ID (numeric) and virtual directory path, and is already used in ASP.NET Core to initialize ANCM and I believe to determine the value of HttpRequest.PathBase.

The particular scenario I have at the moment is a web application that runs multiple instances on different domains from the same physical path on disk. The only way the application can differentiate between instances e.g. for additional config or cache directories) is the IIS site ID.

In .NET Framework / ASP.NET applications, this value is available via HttpRuntime.AppDomainAppId which reads it from the current AppDomain's data. In ASP.NET Core, this value is currently only available during the lifetime of a request via IServerVariablesFeature, but outside of a single request there is no API for it.

I believe that applications that are hosted in IIS and rely on this value outside of a request lifetime (e.g. during startup or during background work) should be able to be ported from ASP.NET / .NET Framework to ASP.NET Core on modern .NET without needing to completely rework any logic that uses the AppDomainAppId.

The value is, however, already known internally to the ASP.NET Core Module and can be reached via reflection and pointer arithmetic. Making it available to the rest of the application is a question of "how", not "if".

Proposed API

namespace Microsoft.AspNetCore.Hosting;

+public static class IISWebHostDefaults
+{
+    public static readonly string IISConfigPathKey = "iis:configpath";
+}

This API will depend upon the following additional behavioural changes, though they are not direct changes to the public API surface:

  • WebHostBuilderIISExtensions.UseIIS will set this key via IWebHostBuilder.UseSetting.
  • Microsoft.AspNetCore.Server.IIS.Core.IISConfigurationData will get a new string field to transfer this value from the native IIS module to the managed ASP.NET Core runtime.
  • http_get_application_properties will copy the config path from unmanaged code into the struct to be marshalled back to managed code.

Usage Examples

var builder = WebApplication.CreateBuilder(args);
var configPath = builder.Configuration[IISWebHostDefaults.ConfigPathKey]; // "/LM/W3SVC/3/ROOT"
var siteID = configPath.Split('/')[3];
builder.Configuration.AddJsonFile($"MyAppConfig-Site{siteID}.json");

var app = builder.Build();
app.Run();
var builder = WebApplication.CreateBuilder(args);
var configPath = builder.Configuration[IISWebHostDefaults.ConfigPathKey];
builder.Services.AddSomeModule(options => options.Category = $"MyCategory-{configPath}");

var app = builder.Build();
app.Run();
var builder = WebApplication.CreateBuilder(args);
app.AddSomeModule();

var app = builder.Build();
app.UseSomeModuleThatNeedsSiteID(app.Configuration[IISWebHostDefaults.ConfigPathKey]);

app.Run();
class MyDependencyInjectedClass
{
    public MyDependencyInjectedClass(IConfiguration configuration)
    {
        var siteSpecificDirectory = Sanitize(configuration[IISWebHostDefaults.ConfigPathKey]);
        cacheDirectory = Path.Combine(Path.GetTempPath(), "MyApplication", siteSpecificDirectory );
    }

    readonly string cacheDirectory;

    static string Sanitize(string input)
    {
        // imagine there's something cool here, its not relevant
    }
}

Alternative Designs

I did consider a method similar to .NET Framework of exposing a static function/property, as http_get_application_properties can be called from anywhere, however this does not easily allow the consuming code to be tested or for stub/mock values to be provided. Every consumer that wished to do so would immediately create their own interface and implementation to simply wrap this static call.

Risks

This will require changes to ANCM, and I do not know what the forwards-compatibility and backwards-compatibility concerns here are and how to mitigate them.

Looking at the most recent change which added a field to the IISConfigurationData struct, there appear to be no compatibility mitigations / versioning / traditional dwSize fields etc., so this may not be a concern?

Additional Notes

This can be done already, albeit extremely hackily, by reflection and pointer arithmetic to access values stored within the IIS integration memory:

var builder = WebApplication.CreateBuilder(args);
var nativeApplicationType = Type.GetType("Microsoft.AspNetCore.Server.IIS.Core.IISNativeApplication, Microsoft.AspNetCore.Server.IIS");
var iisNativeApplication = builder.Services.Where(s => s.ServiceType == nativeApplicationType)
    .Select(x => x.ImplementationInstance)
    .FirstOrDefault();

if (nativeApplicationType?.GetField("_nativeApplication", BindingFlags.NonPublic | BindingFlags.Instance) is { } field &&
    field.GetValue(iisNativeApplication) is SafeHandle nativeApplicationHandle)
{
    var nativeApplicationPointer = nativeApplicationHandle.DangerousGetHandle();

    var objectPointer = IntPtr.Add(nativeApplicationPointer, 136); // Assuming x64, recalculate for x86
    var configPathPointer = Marshal.ReadIntPtr(objectPointer);

    // Got the config path!
    var configPath = Marshal.PtrToStringUni(configPathPointer);
}

I would much rather this be exposed as an API as this relies heavily on not just ASP.NET internals but also the memory layout of underlying native types.

I am happy to drive the implementation myself with a PR, once a design is approved.

yaakov-h avatar Aug 30 '22 05:08 yaakov-h

@davidfowl I've re-filed here.

yaakov-h avatar Aug 30 '22 06:08 yaakov-h

We've run into similar problems (IIS site name in our case), and the way we managed to work around this limitation, was to introduce a well-known environment variable via system.webServer/aspNetCore/environmentVariables in applicationHost.config for each site manually.

Getting the application virtual path during startup can currently be done by reading the ASPNETCORE_APPL_PATH environment variable; although that does not appear to be documented. But .NET 7 and below depend on it too, so it's not likely to go away soon (just plan for contingencies for when it does, like juggling aspNetCore/environmentVariables manually).

rubenprins avatar Aug 30 '22 20:08 rubenprins

Triage: we can consider adding something like this, though we'd need to think about the API design more. Once we've agreed on the design, we would take a PR to add this.

adityamandaleeka avatar Sep 02 '22 20:09 adityamandaleeka

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

ghost avatar Sep 02 '22 20:09 ghost

Marking ready for review so we remember to discuss it.

adityamandaleeka avatar Sep 02 '22 20:09 adityamandaleeka

For reference - this might be something that would overlap with the work being done in https://github.com/dotnet/systemweb-adapters and specifically HttpRuntimeFactory which allows mocking/testing the properties.

CZEMacLeod avatar Sep 21 '22 08:09 CZEMacLeod

API review notes:

  1. If we do this, we probably want the configuration path to be a static property or server feature.

halter73 avatar Oct 03 '22 18:10 halter73

@halter73 are there any further details from API review?

Personally a static property (like in ASP.NET / .NET Framework) would also work, and I could then wrap it up in something more DI-able with private code or a NuGet package.

I don't think a server feature would be appropriate as (AFAIK) that cannot be accessed before calling WebApplicationBuilder.Build()/WebHostBuilder.Build().

yaakov-h avatar Oct 05 '22 22:10 yaakov-h

The main takeaway is that we want to have someone look at this closely and try implementing a static API and see if there are any implementation issues before we approve the final API shape.

halter73 avatar Oct 06 '22 01:10 halter73

@halter73 Can you clarify? This API should not exist on anything that isn't IIS specific and it would couple the code in a way that was undesirable. This is why it's a configuration property:

  • That makes it available very early in the pipeline
  • That makes doesn't tightly couple your code to IIS
  • A unit test can provide this configuration value.

Let's try avoid the static API here please.

EDIT: I see, the configuration API as a property. That's fine 😄

davidfowl avatar Oct 06 '22 03:10 davidfowl

It was @Tratcher's idea to make this a static API, but it makes sense. This is environmental data that's either available immediately when the process starts or it isn't. .NET also ships static APIs to read environment variables. I see no reason to make you initialize the host before reading it

halter73 avatar Oct 06 '22 04:10 halter73

If you care about testing it, abstract it yourself, just like you would for environment variables. I suppose we could also put it in config like we do with environment variables, but it should come from a static source.

halter73 avatar Oct 06 '22 04:10 halter73

I think it's a bad idea to expose it as a static API. We don't do anything else like this in ASP.NET Core (it's one of the original design principles). Because this is so special, and IIS specific, it should not exists as a first class static API. If it wasn't tied to IIS I'd put couple it to the IWebHostEnvironment. Maybe we could add an IFeatureCollection or property bag there to expose static state like this in a non-static way.

davidfowl avatar Oct 06 '22 05:10 davidfowl

Let me try to propose some alternatives. We could have an IISWebHostEnvironment that exposes all of the static state understood by IIS as first class properties:

class IISWebHostEnvironment : IWebHostEnvironment
{
    public string ConfigurationPath { get; }

    // More properties from IWebHostEnvironment
}

When you call the IIS configuration logic would replace the IHostEnvironment with this implementation (probably with a wrapper) and code in the application can do this:

var builder = WebApplication.CreateBuilder(args);
if (builder.Environment is IISWebHostEnvironment env)
{
   var configPath = env.ConfigurationPath; // "/LM/W3SVC/3/ROOT"
   var siteID = configPath.Split('/')[3];
   builder.Configuration.AddJsonFile($"MyAppConfig-Site{siteID}.json");
}

var app = builder.Build();
app.Run();

Problems:

  • We would need to make sure that the instance of IWebHostEnvironment flows via the DI Container after having replaced it on the WebHostBuilderContext (does that work today?)
  • What happens if something else replaces the IWebHostEnvironment? This isn't composable.

davidfowl avatar Oct 06 '22 05:10 davidfowl

Another approach would be a feature based one:

interface IISEnvironmentFeature
{
    string ConfigurationPath { get; }
}

We add an IFeatureCollection to IWebHostEnvironment:

interface IWebHostEnvironment
{
+    IFeatureCollection Features { get; }
}

When you call the IIS configuration logic would add the feature to the collection.

var builder = WebApplication.CreateBuilder(args);
if (builder.Environment.Features.Get<IISEnvironmentFeature>() is { } env)
{
   var configPath = env.ConfigurationPath; // "/LM/W3SVC/3/ROOT"
   var siteID = configPath.Split('/')[3];
   builder.Configuration.AddJsonFile($"MyAppConfig-Site{siteID}.json");
}

var app = builder.Build();
app.Run();

This approach follows the same pattern as Server features and even Http features.

davidfowl avatar Oct 06 '22 05:10 davidfowl

When originally trying to find an API shape for this feature in #43631, I also considered a special subclass of IWebHostEnvironment but when looking at the implementation of that today, replacing it with a subclass seemed like a particularly invasive change just to pass around a single, small, rarely-used string. It also has the same problem @davidfowl mentioned of not being able to have multiple features each follow the same pattern and replace the host environment.

A builder-level or environment-level feature collection is interesting. (I assume you meant builder**.Environment**.Features in the example snippet?)

yaakov-h avatar Oct 06 '22 09:10 yaakov-h

Just on this comment, because it's sticking in the back of my head:

If you care about testing it, abstract it yourself, just like you would for environment variables.

TBH nowadays I mostly use environment variables that map to IConfiguration/IOption[Monitor|Snapshot]<T> anyway.

The out-of-box abstractions are flexible and powerful enough that I haven't had to write my own abstractions for ASP.NET Core applications in quite a long time, and I'd expect that to continue for fairly standard things.

yaakov-h avatar Oct 06 '22 10:10 yaakov-h

Not to hijack this at all, but it might be nice to surface a couple of other IIS properties in the IISEnvironmentFeature AppPoolId, AppPoolConfig, AppDomainAppId, IISVersion, InstanceID, and InstanceMetaPath would be nice. The first two can be got via environment variables - APP_POOL_ID and APP_POOL_CONFIG, and the others can be found via server variables - APPL_MD_PATH, SERVER_SOFTWARE, INSTANCE_ID, and INSTANCE_META_PATH, but only during a request. I think most of these can be surfaced via the existing interface, perhaps with the exception of the version number. I couldn't find any reference source for webengine4.dll to see how it does it and perhaps have a go at adding it to ANCM.

[DllImport(_IIS_NATIVE_DLL)]
        internal static extern void MgdGetIISVersionInformation(
            [Out] out uint pdwVersion,
            [Out] out bool pfIsIntegratedMode);

CZEMacLeod avatar Oct 06 '22 10:10 CZEMacLeod

For properties that are only available during a request, the existing API should be able to retrieve them, and I would expect code being ported from ASP.NET to ASP.NET Core to already only be handling them during a request.

(If they can also be fetched outside of a request then I'd be happy to scope-creep this somewhat to include them, if doing so is reasonable.)

The issue I have with the IIS config path specifically is that in server variables it is only available during a request, but it is available in ASP.NET from other sources outside of the request and for the entire application lifetime, and I have existing code and design patterns that rely on this.

INSTANCE_META_PATH looks like it's exactly the same thing as HttpRuntime.AppDomainAppId, as does APPL_MD_PATH.

INSTANCE_ID looks like its the same derived value that I'm ultimately after (the site ID).

Perhaps this proposal can solve all four at once.

yaakov-h avatar Oct 06 '22 10:10 yaakov-h

We already have a FeatureCollection on IServer and IApplicationBuilder to convey server specific information. Is that available early enough?

Tratcher avatar Oct 06 '22 15:10 Tratcher

Nope, it's not early enough. That's the problem.

davidfowl avatar Oct 06 '22 15:10 davidfowl

@CZEMacLeod I think it makes sense to expose a bunch of properties as well.

davidfowl avatar Oct 06 '22 15:10 davidfowl

Coming back to this, I had a closer look again and I seem to have mixed up Configuration Path and Application ID. Oops

Application ID is what I'm after here, Configuration Path is slightly different, but we could expose that as well if neccesary.

I think this would be the best alternative to a static API:

namespace Microsoft.AspNetCore.Hosting;

public interface IWebHostEnvironment : IHostEnvironment
{
+   IFeatureCollection Features { get; }
    string WebRootPath { get; set; }
    IFileProvider WebRootFileProvider { get; set; }
}
namespace Microsoft.AspNetCore.Server.IIS;

+public class IISApplicationFeature
+{
+    string ApplicationId { get; }
+}

However it comes with the caveat that we're adding a new member to an interface, which is a breaking change for any implementers.

Is that acceptable to do in a major version (hopefully .NET 8), or would we need to mitigate it somehow (IWebHostEnvironment2, Default Method Implementation, new interface, etc.)?

yaakov-h avatar Oct 25 '22 05:10 yaakov-h

@adityamandaleeka Can the above be presented for API review or is it a non-starter to make that kind of breaking change?

yaakov-h avatar Nov 23 '22 05:11 yaakov-h

Some of the APIs have been requested in the dotnet/systemweb-adapters project and it would be great to enable them here (not sure how to do some without access to the native module). I have a prototype of what this would look like here.

As @yaakov-h mentioned, it does require changes to native code side. For the most part, it can be isolated to changes in InProcessRequestHandler which probably has a smaller compat bar. However, to retrieve the version information, a change must be made to ANCM itself (the version is passed into the initial RegisterModule function export).

I added all the properties I found mentioned above and came up with the following:

namespace Microsoft.AspNetCore.Hosting
{
  public interface IWebHostEnvironment : IHostEnvironment
  {
+     IFeatureCollection Features { get; }
      string WebRootPath { get; set; }
      IFileProvider WebRootFileProvider { get; set; }
  }
}

namespace Microsoft.AspNetCore.Server.IIS
+{
+    /// <summary>
+    /// This feature provides access to IIS application information
+    /// </summary>
+    public interface IISEnvironmentFeature
+    {
+        /// <summary>
+        /// Gets the version of IIS that is being used.
+        /// </summary>
+        Version IISVersion { get; }
+
+        /// <summary>
+        /// Gets the AppPool Id that is currently running
+        /// </summary>
+        string AppPoolId { get; }
+
+        /// <summary>
+        /// Gets path to the AppPool configuration that is currently running
+        /// </summary>
+        string AppPoolConfig { get; }
+
+        /// <summary>
+        /// Gets the path of the application.
+        /// </summary>
+        string ApplicationPath { get; }
+
+        /// <summary>
+        /// Gets the virtual path of the application.
+        /// </summary>
+        string ApplicationVirtualPath { get; }
+
+        /// <summary>
+        /// Gets ID of the current application.
+        /// </summary>
+        string ApplicationId { get; }
+
+        /// <summary>
+        /// Gets the name of the current site.
+        /// </summary>
+        string SiteName { get; }
+
+        /// <summary>
+        /// Gets the id of the current site.
+        /// </summary>
+        uint SiteId { get; }
+    }
}

Example:

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Server.IIS;

var builder = WebApplication.CreateBuilder();

// Can access it once the builder is created
var feature = builder.Environment.Features.Get<IISEnvironmentFeature>();

builder.Build().Run();

High level overview of changes:

  • I added fields to the IISConfigurationData struct to transfer the data
  • Added an environment variable for now to transfer the version info; not sure what the servicing/compat requirements of the module installed on IIS is
  • Flowed the required data in InProcessRequestHandler so the existing managed exports can be used
  • Add a configuration call in UseIIS to add the IISEnvironmentFeature to IWebHostEnvironment.Features

Questions:

  • What is the compat requirements of the ANCM module and the InProcessRequestHandler? Are they different?
  • If we can't rely on the ANCM module to be updated to set the version somehow, we'd probably want to have the IISEnvironmentFeature.IISVersion property be nullable
  • Should the IWebHostEnvironment.Features be accessible via the IServer.Features? i.e. IServerFeatures.Features = new FeatureCollection(IWebHostEnvironment.Features)

twsouthwick avatar Jul 28 '23 19:07 twsouthwick

Could this be simplified by making IISEnvironmentFeature available as a DI service instead of a feature? We wouldn't need to change IWebHostEnvironment.

Tratcher avatar Jul 28 '23 20:07 Tratcher

I think the main driving force for the IWebHostEnvironment change was that it should be available before IServiceProvider is available. One of the example scenarios is using one of the values (such as the SiteId) as a key for which config file should be added.

twsouthwick avatar Jul 28 '23 20:07 twsouthwick

It still wouldn't be available until some time after UseIIS is called, and UseIIS doesn't have direct access to IWebHostEnvironment. It would have to register for one of the Configure callbacks, where it would still run after most other registrations. https://github.com/dotnet/aspnetcore/blob/bf52af43b0f77e1ee66096097554c3c36c04c6e6/src/Servers/IIS/IIS/src/WebHostBuilderIISExtensions.cs#L23

Tratcher avatar Jul 28 '23 22:07 Tratcher

Yes, on the branch I linked I registered a configure callback, and it's available for use in this pattern (which covers the scenarios described originally):

using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Server.IIS;

var builder = WebApplication.CreateBuilder();

// Can access it once the builder is created
var feature = builder.Environment.Features.Get<IISEnvironmentFeature>();

var app = builder.Build();

app.Run();

There may be two issues at play here that we may address separately:

  1. Surfacing the information (i.e. adding it to the IServer.Features)
  2. Ensuring it's available early on (i.e. adding IWebHostEnvironment.Features)

twsouthwick avatar Jul 28 '23 22:07 twsouthwick

Thanks, @twsouthwick!

Just to note that I've been bitten in the past by using environment variables in this manner (particularly by MSBuild). If one ASP.NET Core site in IIS launches another in a subprocess, then it would by default inherit this piece of version data...

yaakov-h avatar Jul 29 '23 08:07 yaakov-h