dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.23k stars 9.95k forks source link

Connections.Abstractions broken with non-6.0 ASP.NETCore Apps #38699

Open BrennanConroy opened 2 years ago

BrennanConroy commented 2 years ago

In 6.0 we removed the Microsoft.AspNetCore.Http.Features package and added a new package (Microsoft.Extensions.Features) with some types moved from the old package to the new one. https://github.com/aspnet/Announcements/issues/462

The problem is that Microsoft.AspNetCore.Http.Features is in the shared framework so when a reference to the new Microsoft.Extensions.Features package comes in you now have types defined in both Microsoft.Extensions.Features and Microsoft.AspNetCore.Http.Features.

You can easily repro this by creating a net461 or netcoreapp3.1 project and adding a reference to Microsoft.AspNetCore.Connections.Abstractions 6.0. A more likely scenario and where the real issue can be seen; I maintain a library that targets netstandard2.0 (so that it runs everywhere, and because all my dependencies like Microsoft.AspNetCore.Connections.Abstractions claim to run on netstandard2.0), and I see that 6.0 came out, I update my dependencies, maybe use some new features etc. unit test my library and ship it. Now someone comes along, tries to use my library in a net461, netcoreapp3.1, or net5.0 app and it breaks. My library now only works in net6.0 or a console app even though I wrote it so it runs everywhere.

BrennanConroy commented 2 years ago

Triage: We're going to add back the netstandard2.0 TFM for Microsoft.AspNetCore.Http.Features and start shipping it as a package again (still in the shared framework, that's not changing). We'll #ifdef the new parts that don't work in netstandard2.0 like DIMs.

People who bring in Extensions.Features to a net461/net3.1/etc. app will still hit issues, but will now have a workaround of referencing the 6.0 Microsoft.AspNetCore.Http.Features package. We will need to update the announcement/discussion and any breaking change docs.

We will keep not shipping Microsoft.AspNetCore.Http.Features in 7.0 and have until next year to decide if we want to ship it again or not before 7.0 RTM.

BrennanConroy commented 2 years ago

The above fixes 3.1 but full framework is still broken because of other breaking changes that have happened, for example InplaceStringBuilder was removed from Microsoft.Extensions.Primitives but used in Microsoft.Net.Http.Headers which will break Kestrel in full fx. We'll need to re-think our strategy and consider if it's worth making the above change if it only fixes 3.1.

jahorne commented 2 years ago

My issue has been marked as a dup. I'm not sure it is. If you want to use Azure's SignalR in an ASP.NET 4.8 app, you have code that looks like this:

        app.UseCookieAuthentication(new()
        {
            AuthenticationType = DefaultAuthenticationTypes.ApplicationCookie,
            LoginPath = new("/Account/Login.aspx"),
            CookieSecure = CookieSecureOption.SameAsRequest
        });

        app.MapAzureSignalR(GetType().FullName);

If the authentication code is before MapAzureSignalR as it is above, you get a 500 error on the connection.

If the authentication code is moved below MapAzureSIgnalR, then Context.User in the SignalR hub is null and you can't get the user name, etc.

This still works fine as long as I leave Microsoft.AspNetCore.Connections.Abstractions and Microsoft.ApsNetCore.Http.Connections.Client and Microsoft.AspNetCore.HttpCommon at the 5.0.11 versions. As soon as I upgrade to (now) 6.0.2, it breaks.