RickStrahl / Westwind.AspNetCore.Markdown

An ASP.NET Core Markdown support library that provides Markdown parsing, a Markdown TagHelper and Markdown Page Handler Middleware
MIT License
247 stars 30 forks source link

NullRefException on .NET Core 3 because of IHostingEnvironment #13

Closed haacked closed 4 years ago

haacked commented 4 years ago

In asp.net core 3, the IHostingEnvironment class has been made obsolete in favor of IWebHostingEnvironment. In my app, this caused this line to be null https://github.com/RickStrahl/Westwind.AspNetCore.Markdown/blob/master/Westwind.AspNetCore.Markdown/MarkdownPageProcessorMiddleware/MarkdownPageProcessorMiddleware.cs#L50

Which results in a NullReferenceException.

I ended up writing an adapter class and registering it to get things to work...

#pragma warning disable 618
    public class ObsoleteHostingEnvironmentAdapter : IHostingEnvironment
#pragma warning restore 618
    {
        public ObsoleteHostingEnvironmentAdapter(IWebHostEnvironment environment)
        {
            ApplicationName = environment.ApplicationName;
            ContentRootFileProvider = environment.ContentRootFileProvider;
            ContentRootPath = environment.ContentRootPath;
            EnvironmentName = environment.EnvironmentName;
            WebRootFileProvider = environment.WebRootFileProvider;
            WebRootPath = environment.WebRootPath;
        }

        public string ApplicationName { get; set; }
        public IFileProvider ContentRootFileProvider { get; set; }
        public string ContentRootPath { get; set; }
        public string EnvironmentName { get; set; }
        public IFileProvider WebRootFileProvider { get; set; }
        public string WebRootPath { get; set; }
    }

Then in Startup.

#pragma warning disable 618
            // The Markdown Middleware needs this.
            services.AddSingleton<IHostingEnvironment>(new ObsoleteHostingEnvironmentAdapter(Environment));
#pragma warning restore 618

The recommended fix is to change the type in the middleware to IHostingEnvironment, but that will break older users. 😦 You could probably have your own adapter class.

RickStrahl commented 4 years ago

Thanks Phil. I guess I hadn't even looked at making this work on 3.0 at this point.

This is a bloody pain because it's not directly fixable inside of the package itself without completely changing the targeting. The only way to make this work on both platforms is to change to a platform specific implementation and multi-target which is ugly in this case.

Your solution is a good workaround, but it has to be done at the project level to get access to the IWebHostingEnvironment type.

I think as soon as 3.1 LTS is released I'll move everything over to 3.x and just forget about older versions.

RickStrahl commented 4 years ago

Hey Phil, with that change is the middleware working for you in 3.0?

I just set this up for multi-targeting and that fixes the hosting environment, but the re-routing to the controller appears to not work properly. What is your routing setup so this works on your end?

This gets me into the middleware as expected, but doesn't reach the rendering controller:

           app.UseRouting();

            app.UseMarkdown();

            app.UseStaticFiles();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapRazorPages();
                endpoints.MapDefaultControllerRoute();
            });

Never mind figured it out.

Have to explicitly add the assembly so MVC can find it:

// We need to use MVC so we can use a Razor Configuration Template
services.AddMvc()
    .AddApplicationPart(typeof(MarkdownPageProcessorMiddleware).Assembly);
RickStrahl commented 4 years ago

Ok I've updated the source code and set it up for multi-targeting for Core 2.1 and 3.0. It took some doing though there are a few small changes that made this a lot harder than it should have been.

I pushed up a preview package for 3.3.0 to check out.

haacked commented 4 years ago

I tested 3.3.0-preview-1 and it worked great! Thanks!