ASPNETCORE_ENVIRONMENT issues with Startup classes and local IIS profiles

[echuck66]

Describe the bug

When running, debugging or deploying Asp.Net Core applications, environment variables are added to the web.config file, which ultimately concatenates this specific environment variable, rendering it absolutely worthless. For example, if you set the environment variable to 'Development', running or debugging the application causes the environment variable to be rendered as 'Development;Staging', 'Development;Development', 'Staging;Production', or some variation thereof. This makes determining which Startup class, Config class and appSettings.json file impossible to determine at run time.

To Reproduce

Steps to reproduce the behavior:

1. Start with a normal Asp.Net core application running in IISExpress.
2. Set a system environment variable for ASPNETCORE_ENVIRONMENT.
3. Add a profile to allow running the application through the local machine's IIS.
4. Run or debug the project. <-- at this point, you can see the web.config has been mondified
5. Remove the local IIS profile and run/debug/deploy again.
6. Condition persists.
7. Remove the IIS application definitions from the local IIS and repeat.
8. Condition persists.

The web.config file will be modified to include the environment variable ASPNETCORE_ENVIRONMENT.

Expected behavior

With so many different ways to set the environment variable, I would expect the web.config file to NOT be modified with the environment variable. This completely breaks the application.

Visual Studio Version

Visual Studio 2019 version 16.2.1

[javiercn]

@echuck66 Thanks for contacting us.

@vijayrkn I believe this is a tooling issue. Could you have someone take a look?

[echuck66]

If anyone has a workaround and can share it, that would be greatly appreciated. I'm not sure how I can debug the application right now because of this.

This was not an issue before I added the local IIS profile. I've removed it, but the problem persists.

I'm not sure if I should remove IIS, uninstall/re-install Visual Studio, etc. Hate to do any of those things, but will do so if that's the only way around this. I'll restart my machine to see if that clears out anything.

[echuck66]

Workaround for Debugging Found:

In Program.cs, I added:

#if DEBUG 
      Environment.SetEnvironmentVariable("ASPNETCORE_ENVIRONMENT", "Development");
#endif

This seems to work fine for debugging, but is only a workaround.

[jkotalik]

@anurse this is an issue with IIS inprocess concatenating environment variables in the web.config with ones that are already set on the process. A few other people have hit this issue as well. It would be a breaking change, but a light one as it's rare to conflict between web.config and iisexpress process environment variables.

[analogrelay]

Let's change the behavior early in 5.0 to not append and solicit feedback. We'll also add an ANCM property that can be used to quirk the behavior back to the old logic.

If it's reasonable, we could do replacement with expansion (PATH=%PATH%;C:\Foo;C:\Bar) to allow users to choose if they want append, prepend, etc.

[jkotalik]

ANCM already expands the environment variables today: https://github.com/aspnet/AspNetCore/blob/3477daf3c4f530dff80f197e0642cb39a26fb07b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/environmentvariablehelpers.h#L83.

I think we should remove the appending behavior. I believe out of proc is also not appending: https://github.com/aspnet/AspNetCore/blob/2e89aa4aa6546d11aafe815928886d4439df8c2f/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp#L847. It calls CreateProcess with inherit handles and passes in environment variables. I believe if there are duplicates in the process, those are ingored/overwritten by the ones in environment variables.

@anurse I'd say we make the breaking change now.

[analogrelay]

Do we have tests that use embedded env vars to reproduce append behavior? If so, or if we can add one, I'm totally in favor of making this change ASAP. We'll need to announce a breaking change properly, but I think this makes sense. I think I'd still add a property to the property bag to quirk it off even though it should be possible to "rewrite" your logic. Just in case.

[jkotalik]

I don't think we have a dedicated test for it, but we can add one as part of this change.