aspnetcore: HttpContextAccessor.HttpContext is null when using TestServer and .net 2.2.2

I can do one better and get a full-on workaround by adding an HttpMessageHandler that abandons the ExecutionContext for you. Then you just have to wrap the ClientHandler up in that and it seems to work:

This workaround unblocked us. Our scenario is this: We have multiple microservices. We use JWT Bearer auth and Open ID Connect using our own IdentityServer4 IdP. In order for our tests to work we need to set up two TestServers: the microservice we are testing and the IdP. Then we delegate the JWT verification from our microservice to our IdP.

public class ApiServiceFactory : WebApplicationFactory<Api.Startup>
{
  private WebApplicationFactory<Identity.Startup> _identityServerFactory;

  protected override void ConfigureWebHost(IWebHostBuilder builder)
  {
    var backChannelHandler = _identityServerFactory.CreateHandler();
    builder.ConfigureServices(
      s => s.AddSingleton<IPostConfigureOptions<IdentityServerAuthenticationOptions>>(
        new PostConfigureOptions<IdentityServerAuthenticationOptions>(
            "AuthScheme",
          opt => { opt.JwtBackChannelHandler = new SuppressExecutionContextHandler(backChannelHandler); })));
  }
}

This regression is unfortunate, but glad we’re unblocked now. I’m not fully understanding the reasoning why this is an edge case. It seems like a common case for anyone testing with authentication. Thanks for the workaround @anurse .

Thanks @parekhkb, that’s a more concrete example and it’s not even recursive, it’s multi-tier. It makes sense we’d need to prevent cross contamination between the server instances.

I can do one better and get a full-on workaround by adding an HttpMessageHandler that abandons the ExecutionContext for you. Then you just have to wrap the ClientHandler up in that and it seems to work:

› dotnet exec .\bin\Debug\netcoreapp2.2\UnitTests.dll
Runtime Path: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.2.4\System.Private.CoreLib.dll
ASP.NET Path: C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\2.2.4\Microsoft.AspNetCore.Http.Abstractions.dll
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/outer
[OUTER] (before) HttpContextAccessor.HttpContext.Request.Path = /outer
[INNER] HttpContextAccessor.HttpContext.Request.Path = /inner
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/inner
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 7.6767ms 200
HttpContextAccessor NON-NULL after nested!
[OUTER] (after) HttpContextAccessor.HttpContext.Request.Path = /outer

My main concern is the unintended consequences of abandoning the EC here. Perhaps this would live best as a known-issue workaround rather than a fix.

Looks like there is indeed a regression here. The problem lies in the nested HTTP calls. When you make a call back to the application using the HttpClient produced by TestServer.CreateClient() (or handlers produced by TestServer.CreateHandler()) the HttpContextAccessor gets cleared out after the nested call is made and isn’t restored to it’s previous content.

This behavior does not appear to occur in 2.2.0. After a brief binary search it appears to have been introduces in 2.2.2. I do see a change to HttpContextAccessor in 2.2.2 (https://github.com/aspnet/AspNetCore/pull/6036 by @JunTaoLuo), which definitely seems suspiciously relevant 😃. I think something in that PR regressed this TestServer scenario where there are nested HTTP calls.

Repro details below. cc @Tratcher @davidfowl @JunTaoLuo

The following program repros the issue

class Program
{
    public static async Task Main(string[] args)
    {
        HttpClient client = null;
        var builder = WebHost.CreateDefaultBuilder()
            .Configure(app =>
            {
                app.Use(async (context, next) =>
                {
                    var accessor = context.RequestServices.GetRequiredService<IHttpContextAccessor>();
                    if (context.Request.Path.StartsWithSegments("/inner"))
                    {
                        if (accessor.HttpContext == null)
                        {
                            throw new System.Exception("Invalid During Nested Call!");
                        }
                    }
                    else if (context.Request.Path.StartsWithSegments("/outer"))
                    {
                        var nestedResp = await client.GetAsync("/inner");
                        if (accessor.HttpContext == null)
                        {
                            Console.WriteLine("HttpContextAccessor NULL after nested!");
                        }
                        else
                        {
                            Console.WriteLine("HttpContextAccessor NON-NULL after nested!");
                        }
                    }
                    else
                    {
                        await next();
                    }
                });
            })
            .ConfigureServices(services =>
            {
                services.AddHttpContextAccessor();
            });
        Console.WriteLine($"Runtime Path: {typeof(string).Assembly.Location}");
        Console.WriteLine($"ASP.NET Path: {typeof(HttpContext).Assembly.Location}");
        var server = new TestServer(builder);
        client = server.CreateClient();
        var resp = await client.GetAsync("/outer");
        resp.EnsureSuccessStatusCode();
    }
}

If I build this and then run it against 2.2.0, I get:

> dotnet exec --fx-version 2.2.0 .\bin\Debug\netcoreapp2.2\UnitTests.dll
Runtime Path: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.2.4\System.Private.CoreLib.dll
ASP.NET Path: C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\2.2.0\Microsoft.AspNetCore.Http.Abstractions.dll
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/outer
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/inner
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 6.7586ms 200
HttpContextAccessor NON-NULL after nested!

If I run against 2.2.2, I get:

> dotnet exec --fx-version 2.2.2 .\bin\Debug\netcoreapp2.2\UnitTests.dll
Runtime Path: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.2.4\System.Private.CoreLib.dll
ASP.NET Path: C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\2.2.2\Microsoft.AspNetCore.Http.Abstractions.dll
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/outer
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[1]
      Request starting HTTP/2.0 GET http://localhost/inner
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 6.3915ms 200
HttpContextAccessor NULL after nested!

I’ll look at making this change and adding a setting to turn it back off. Technically a breaking change which will need some announcement.

The workaround provided by @anurse is here https://github.com/aspnet/AspNetCore/issues/7975#issuecomment-481536061

What else would we loose?

The thing we lose is fairly simple, it’s the flow of ExecutionContext between the “client” and the “server”. That means AsyncLocal and things like current culture. Since TestServer is designed to emulate a real client/server interaction, and a real client/server interaction has a machine boundary, this seems reasonable. The fact that we flow the EC at all is entirely misleading, and unlike suppressing 500s I think it reduces the value and usefulness of TestServer rather than improving it. I agree that where it adds value, we can compromise the “emulation” but this does not add value, it reduces it.

there’s no first class way to set up this recursive scenario in the test server

Of course there is. We literally give you the ability to create HttpMessageHandlers that can “invoke” the server. Regardless of if it was intentional, we definitely gave users the ability to make recursive calls.

A user trying to flow AsyncLocals between the client and server in this scenario seems like the edge case and is not the intended design. I don’t think we should optimize for the edge case (that doesn’t work in a real server) here.

To me, disabling EC flow by default seems like the best option here and in 3.0 it can be relatively easily justified as a potential breaking change. We can annoucement it, do it in an early preview and provide an opt-out switch.

Feels like we should have a clean ExecutionContext per request no?

💯 agree. Just want to make sure we have an escape hatch because it will break some code (arguably wrong code 😉). An option to “not abandon the EC” seems reasonable to me.

Just use UnsafeQueueUserWorkItem

Returning a value from UnsafeQueueUserWorkItem seems to introduce more complexity than just suppressing the flow from Task.Run, since you have to dance with a TCS, but whatever. As long as you abandon the EC somehow 😃

Ok, I’ll throw this in to preview 6 as low pri. We’ll see what we can pull out here. This seems like another area I might be able to prove I can still write code ;P.

@brandonpollett if you want a workaround for now, see my comment above.