Expand System.Web facade to support lightup code migrating from .NET Framework

[bartonjs]

While working with a team converting from .NET Framework to .NET Core, we ran across two lightup dependency patterns involving classic ASP.NET:

1) HttpContext.Current

if (HttpContext.Current != null) {
   ...
}

return null;

2) HostingEnvironment.SiteName

string siteName = HostingEnvironment.SiteName;

if (!string.IsNullOrEmpty(siteName)) {
   ...
}

3) (Theoretical) HostingEnvironment.IsHosted

---

Pattern 2 is used by the Microsoft.WindowsAzure.ServiceRuntime library, which prevents projects interacting with Azure's classic hosted service offering from using .NET Core (at least, if they want to interact with the service host).

We currently have a System.Web facade in the shared runtime, and I propose we increase it from one type forward to a PlatformNotSupportedException auto-generated assembly, with the following exceptions:

• HttpContext.Current should return null, rather than PNSE.
• HostingEnvironment.IsHosted should return false, rather than PNSE.
• HostingEnvironment.SiteName should return null, rather than PNSE (perhaps, generally, HostingEnvironment should behave as the "!IsHosted" case in all members that can avoid allocating new objects)

This would allow ASP.NET contextual lightup code to just execute in the "not ASP.NET" path, rather than fail with a TypeLoadException.

This extra support will not be in any ref.cs, it's only intended for the .NET Framework assembly running on .NET Core shim.

cc: @ericstj

[ericstj]

@terrajobst what is your opinion on this? It would mean we grow System.Web.dll substantially. I did a quick measurement and found it would be 900KB compared to about 4KB today. Turns out System.Web is very big.

[terrajobst]

@bartonjs and I chatted about this earlier. In general, I'm supportive of making more existing code work slash portable as that's very much in the spirit of the platform. That being said, Jeremy mentioned providing APIs to make light-up code work. I'm a bit surprised this has a 900KB footprint... Is all of that really necessary?

(System.Web is part of Microsoft.NETCore.App. Normally we try to make more things portable without inflating the platform for everyone.)

[ericstj]

900KB is what I get if I include everything from System.Web, excluding only dependencies on EnterpriseServices. Could probably get smaller if you slice and dice. Here's my generated source: https://gist.github.com/ericstj/0796d775ec8460daede2f45159191a7c

[terrajobst]

I see, so 900KB is a "worst case" benchmark of sorts. I think @bartonjs seems to have something more targeted in mind.

[ericstj]

one type forward to a PlatformNotSupportedException auto-generated assembly, with the following exceptions

This implies it is full, only omitting a couple API so that we can implement them with default behavior.

[bartonjs]

We could start it at HostingEnvironment and HttpContext as roots, and complete the type closure (so that code like

if (HttpContext.Current != null)
{
    return HttpContext.Current.SomeProp.GetSomething(5)[2].OtherProp;
}

return null;

doesn't get a different missing type/member exception).

HostingEnvironment had like a 14 type closure. HttpContext eventually got to Style, which pulled in WebControl, and that's when I stopped the manual port. (https://github.com/bartonjs/corefx/blob/system.web_shim/src/shims/System.Web.forwards.cs snipped that link, and I came out to 101,376 bytes... but that was all by-hand work)

[davidfowl]

This is about updating just the runtime and not the ref correct?

[bartonjs]

This is about updating just the runtime and not the ref correct?

Correct. From the initial post: This extra support will not be in any ref.cs, it's only intended for the .NET Framework assembly running on .NET Core shim.

[ericstj]

Rather than growing System.Web.dll in the shared framework itself I'd recommend we tackle this by building an out-of-band only package. That package could be part of the Microsoft.Windows.Compatibility package. We could optionally include a version of System.Web.dll in this package, or make the inbox one support it via dangling typeforwards (like we do for other things in the compat pack). The downside of the latter approach is that it would only work on latest.

Having it be out-of-band mitigates the size growth of the shared framework. The package would only be exposing runtime assets and not references.

I'm still not sure if its worth doing this. @bartonjs do you have any data on how hard it is for our folks who are porting to .NETCore to just remove their calls to this System.Web API?

[bartonjs]

For the team I was working with, it wasn't their code that was the problem, it was libraries they were using which had lightup references to System.Web.

For components with no dependencies, it's easy... you know it's not there, delete the code (or conditionally compile it for migration). Dependencies are the tricky problem.

[ericstj]

Do you know if they were blocked by this, or were able to rebuild that component?

[bartonjs]

The porting experiment was timeboxed. They had source and could fork the component; but because some of the users are still using classic ASP they can't just remove the lightup from the shared library. So the choices were fork, dual-compile (which is difficult in their engineering system), or try to make System.Web work "just enough". They pursed the latter, but got bit by the System.Web in the shared runtime, and not understanding how to teach the .deps.json about their attempt at overriding the shared framework.

A NuGet package would probably work fine for their needs, and presumably that would make the .deps.json build in the way they wanted.

(@ryandle feel free to correct me / add to my recollection / add data from beyond my involvement in the project)

[davidfowl]

I like the idea of the out of band package, I think we should discuss this more as I have some thoughts about it as well.

[ryandle]

There was also a 2nd dependency on a DLL owned by a different internal team. A primary entry point of their DLL has a reference to HostingEnvironment.SiteName to see if its running in ASP.NET. (They had a few other references to System.Web too but they were all branched behind this one IIRC.) That source is not something we can fork/recompile/change the distribution of. We're currently blocked on that library. @davidfowl is talking with the owning team internally and would have more details on it. I'm also happy to chat more.

[mjsabby]

Would there be any interest in bringing abstract classes like HttpResponseBase and HttpContextBase? The following is a screenshot of our internal slimmed down version we have in Bing which is implemented a top of Kestrel and some home grown HttpContext fillers.

[ericstj]

Sure, we can bring more dummy methods. It looks to me like you probably brought back some of these because you needed the type closure but didn't necessarily need methods to work. This version of System.Web includes the entire type closure / API surface.

It'd be nice to know which methods you'd need to return default behavior rather than throw.

[ericstj]

Reopening for feedback from @mjsabby

[mjsabby]

Thanks @ericstj -- what happens to enums? I would think we could bring back all the enums?

For implementation there is really only RouteData, RouteValueDictionary, RequestContext and StopRoutingHandler.

We needed HttpContext, but only as a TypeRef, because they are part of signatures.

If the above are available, we can completely retire our fake System.Web hacks.

[ericstj]

Enums are defined completely. Check out the package here https://dnceng.visualstudio.com/public/_packaging?_a=package&feed=dotnet-experimental&package=System.Web.Compatibility&protocolType=NuGet&version=0.0.1-alpha.20352.2

Also here's what the generated source looks like:

System.Web.notsupported.zip

[mjsabby]

Excellent. I verified that in fact, those are the only implementations I need.

[ericstj]

For implementation there is really only RouteData, RouteValueDictionary, RequestContext and StopRoutingHandler.

I will add these.

[ericstj]

@mjsabby there are some transitive types. RouteBase HttpContextBase and their closure. I assume you don't need to be able to construct these?

[mjsabby]

Nopes, the PR you have is all I need.

[ericstj]

Fixed in https://github.com/dotnet/corefxlab/pull/2942

[ericstj]

You can find the latest package here https://dnceng.visualstudio.com/public/_packaging?_a=package&feed=dotnet-experimental&package=System.Web.Compatibility&protocolType=NuGet&view=versions (will be available shortly)