toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.5k stars 259 forks source link

System.UriFormatException: Invalid URI: The hostname could not be parsed. #479 #480

Closed junalmeida closed 2 years ago

junalmeida commented 2 years ago

Describe the bug Facing this exception while testing with dotnet test on ubuntu-latest on Azure DevOps: I still have no idea of the value that DefaultAssemblyDirectoryFormatter is passing to Uri from the Assembly, no idea which Assembly in this the loop.

The assembly directory depends on what framework you're using, among other things, like whether you bundled the entrypoint project into a single DLL solution or not. This isn't exactly a RazorLight question, though.

The project is not bundle into a single DLL. This is happening during test on ubuntu. I'm trying to debug to see what Assembly is causing the error, but anyway simply passing Assembly.Location over to UriBuilder is known to cause problems. Don't we need a check?

To Reproduce Steps to reproduce the behavior: Just run tests on a setup with Embedded Resource Project.

Expected behavior Probably expect that you might get an assembly with no valid location, as according to https://github.com/dotnet/corert/issues/5467, the location may or may not exist, or could even be null.

Information (please complete the following information):

Additional context It is a good practice to discuss before simply closing an issue. Thanks for understanding.

https://github.com/toddams/RazorLight/blob/aceda132f1b97a49663906a22b8c280a57b078da/src/RazorLight/Compilation/DefaultAssemblyDirectoryFormatter.cs#L10-L11

System.UriFormatException: Invalid URI: The hostname could not be parsed.
  Stack Trace:
      at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind, UriCreationOptions& creationOptions)
   at System.Uri..ctor(String uriString)
   at System.UriBuilder..ctor(String uri)
   at RazorLight.Compilation.DefaultAssemblyDirectoryFormatter.GetAssemblyDirectory(Assembly assembly)
   at RazorLight.Compilation.DefaultMetadataReferenceManager.<Resolve>b__12_2(Assembly p)
   at System.Linq.Enumerable.SelectArrayIterator`2.ToList()
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at RazorLight.Compilation.DefaultMetadataReferenceManager.Resolve(Assembly assembly, DependencyContext dependencyContext)
   at RazorLight.Compilation.DefaultMetadataReferenceManager.Resolve(Assembly assembly)
   at RazorLight.Compilation.RoslynCompilationService.EnsureOptions()
   at RazorLight.Compilation.RoslynCompilationService.get_ParseOptions()
   at RazorLight.Compilation.RoslynCompilationService.CreateSyntaxTree(SourceText sourceText)
   at RazorLight.Compilation.RoslynCompilationService.CreateCompilation(String compilationContent, String assemblyName)
   at RazorLight.Compilation.RoslynCompilationService.CompileAndEmit(IGeneratedRazorTemplate razorTemplate)
   at RazorLight.Compilation.RazorTemplateCompiler.CompileAndEmitAsync(RazorLightProjectItem projectItem)
   at RazorLight.Compilation.RazorTemplateCompiler.OnCacheMissAsync(String templateKey)
   at RazorLight.EngineHandler.CompileTemplateAsync(String key)
junalmeida commented 2 years ago

This is what I could find about the issue, I'm debugging with justMyCode = false to see which assembly was in the variable when the exception happens:

image

It seems the path is valid though.

Let me know if I can provide any other information.

SerhiyBalan commented 2 years ago

same issue works perfectly at Azure Functions (Windows) Doesn't work at all at Azure App Service (Linux)

SerhiyBalan commented 2 years ago

So far, the only solution I've found is to roll back to version 2.0.0-rc.3. It works well on Linux.

jzabroski commented 2 years ago

I think it's a .NET 6.0 breaking change due to the code relying on an undocumented bug in UriBuilder for unknown URI Schems.

https://github.com/dotnet/runtime/issues/61363#issuecomment-964254260

You were relying on an undocumented bug affecting unknown Uri schemes.

I wonder if we explicitly use the file:// protocol if it will work correctly.

jzabroski commented 2 years ago

@junalmeida Do you want to try submitting a PR to fix it if I'm right?

SerhiyBalan commented 2 years ago

I think it's a .NET 6.0 breaking change due to the code relying on an undocumented bug in UriBuilder for unknown URI Schems.

my backend uses Core 3.1 and runs under Linux, and I have exactly the same issue

when I run it locally under Windows, everything works well

jzabroski commented 2 years ago

@SerhiyBalan

  1. The tests run on all 3 major OSes: https://github.com/toddams/RazorLight/actions/runs/2071823518
  2. The tests run on many different .NET Core Onward TFMs: https://github.com/toddams/RazorLight/blob/master/tests/RazorLight.Tests/RazorLight.Tests.csproj#L4

And then there are tests that effectively exercise this general code path concept, indirectly: https://github.com/toddams/RazorLight/blob/22381db30ca3c61b4b1e4004e1536407dd138e49/tests/RazorLight.Tests/Extensions/ServiceCollectionExtensionsTest.cs#L25 (It probably should directly use the production code, but whatever).

SerhiyBalan commented 2 years ago

@jzabroski 2.0.0-rc.3 works perfectly for me

2.0.0-rc.4 and later - doesn't work - System.UriFormatException: Invalid URI: The hostname could not be parsed. Stack trace refers to the same file as OP mentioned

So currently I'm staying on the outdated version because it works at the production

jzabroski commented 2 years ago

I see that I added https://github.com/toddams/RazorLight/pull/407 which uses AppContext.BaseDirectory for unit tests.

If you add the following and replace the DI reference for the interface, does it work?

    public class PassthroughAssemblyDirectoryFormatter : IAssemblyDirectoryFormatter
    {
        public string GetAssemblyDirectory(Assembly assembly)
        {
            return assembly.Location;
        }
    }
junalmeida commented 2 years ago

@jzabroski I would love to have the necessary expertise on your project to issue a good PR to fix this, however I don't think I have it yet, i.e I don't know how are your practices on tests and stuff.

I have one more thing to add:

I'm currently NOT using DI, instead, I'm using the RazorLightEngineBuilder which doesn't have the UseNetFrameworkLegacyFix. So to make a test I temporarily changed it to a self-contained ServiceCollection like this:

var services = new ServiceCollection();
        services.AddRazorLight()
            .UseNetFrameworkLegacyFix()
            .UseMemoryCachingProvider()
            .SetOperatingAssembly(typeof(HtmlGenerator).Assembly)
            .ExcludeAssemblies(ExcludedAssemblies);

        var project = new RazorLight.Razor.EmbeddedRazorProject(typeof(HtmlGenerator).Assembly, "REDACTED.HtmlTemplates")
        {
            Extension = "",
        };

        services.AddSingleton<RazorLight.Razor.RazorLightProject>(project);

        var container = services.BuildServiceProvider();
        engine = (RazorLightEngine)container.GetRequiredService<IRazorLightEngine>();

On the other end, the AddRazorLight doesn't have UseProject which I am using, so as a side note, those two ways to build the engine are not consistent between each other.

I can confirm that UseNetFrameworkLegacyFix works for me on version 2.0.0 on linux.

jzabroski commented 2 years ago

@junalmeida I see. The reason the UseNetFrameworkLegacyFix is called that is because Assembly.CodeBase is undefined in a single file application and now throws an exception in .NET Core. See: https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file/overview#api-incompatibility

Technically, Assembly.CodeBase still works when you're not in a single file application scenario. However, it is brittle. I suppose that:

@SerhiyBalan Does that work for you? I still think we don't have an explanation for why you can't use UriBuilder - can you try a simple console app that calls the new UriBuilder constructor:

public void Main()
{
  var location = typeof(Program).Assembly.Location();
  Console.WriteLine(location);
  var uri = new UriBuilder(location);
}
junalmeida commented 2 years ago

@jzabroski Don't you think that would require the dev to understand the inner workings of System.Reflection? Not everyone knows that. I'd try instead a default approach which can do an automatic fall back, but yet provide ways to force one or other method?

Just thinking out loud.

SerhiyBalan commented 2 years ago

@jzabroski Thank you for helping!

I'm on vacation and will return on Tuesday. I will try everything from here and let you know if anything helped me to resolve the issue with the latest version of RazorLight

jzabroski commented 2 years ago

@jzabroski Don't you think that would require the dev to understand the inner workings of System.Reflection?

  1. There's no reflection happening here. This is just the rules for the .NET Platform.
  2. I expect developers to understand the .NET Platform, or they will likely have disappointing careers or drive their coworkers nuts, or both.
  3. I expect developers to read and follow the manual for the products they choose to use/build on top of, even though many do not. For example, you did not follow the issue template originally, my expectations were violated, so I closed the issue.
junalmeida commented 2 years ago

@jzabroski Well, the Assembly type is a member of System.Reflection. I mean, the dev would need to know what the property CodeBase means to fully understand what a method named after UseAssemblyCodeBase does.

Most developers won't go that far IMO, only the most seniors. I've worked with all range of seniority in terms of .NET platform. I'm thinking that you might deal with a long range of seniority as well on your user base.

Anyway, just an opinion, of course if you maintain this project you are the best to choose.

Again, if I may, I haven't "follow" the issue template because I was simply using github as any user may do. You must be aware that I can go to any code line, right click and create a new issue referring a code line, and then no issue template was offered to me. Maybe you need to improve the repository issue settings, or make a suggestion to github.

My expectations as a user were also violated as the issue was closed with no chance of editing and improving it, we are even! 👯 Devs that don't "follow" the manual are out there, but maybe the "manual" not being clear enough is the reason for it not to be "followed". We are all humans here, some will follow 100%, some not much.

junalmeida commented 2 years ago

Getting back to the point, I just realized that Assembly.CodeBase is deprecated:

        [Obsolete("Assembly.CodeBase and Assembly.EscapedCodeBase are only included for .NET Framework compatibility. Use Assembly.Location.", DiagnosticId = "SYSLIB0012", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")]
        [RequiresAssemblyFiles("This member throws an exception for assemblies embedded in a single-file app")]
        public virtual string? CodeBase { get; }

So maybe the best is to not use UriBuilder at all, since the Assembly.Location property is supposed to be platform-dependant path? I don't know what kind of values you've got on other platforms to make use of UriBuilder for file paths.

jzabroski commented 2 years ago

Most developers won't go that far IMO, only the most seniors. I've worked with all range of seniority in terms of .NET platform. I'm thinking that you might deal with a long range of seniority as well on your user base.

I can't set a standard of incompetence, because then that's what I'll get.

People who waste my time by not following adult etiquette, like reading documentation and generally understanding their job, are an unfortunate part of participating in open source software.

I mean, the dev would need to know what the property CodeBase means to fully understand what a method named after UseAssemblyCodeBase does.

Correct.

My expectations as a user were also violated as the issue was closed with no chance of editing and improving it, we are even! 👯

This is a warning - you are trolling this thread at this point.

junalmeida commented 2 years ago

Well, I am not trolling anything, just stating my points as you asked/comment like on any topic discussion. I don't need a warning here, as an adult, you may do whatever you find appropriate with your opensource project. If you want to close it and leave the bug live for your users, that's your call. I just tried to be helpful reporting an issue to the community. I will be glad if you receive comments from the community respectfully.

SerhiyBalan commented 2 years ago

@jzabroski

Code in use:

public class RazorEngineService : IHtmlTemplateRendererService
{
    private static readonly IRazorLightEngine EngineService;

    static RazorEngineService()
    {
        EngineService = new RazorLightEngineBuilder()
            .UseEmbeddedResourcesProject(typeof(RazorEngineService))
            .SetOperatingAssembly(typeof(RazorEngineService).Assembly)
            .UseMemoryCachingProvider()
            .Build();
    }

    public async Task<string> GenerateCodeAsync<T>(string path, T model, string key)
    {
        var cacheResult = EngineService.Handler.Cache.RetrieveTemplate(key);
        if (cacheResult.Success)
        {
            // report is already compiled, render using a cache
            var templatePage = cacheResult.Template.TemplatePageFactory();
            return await EngineService.RenderTemplateAsync(templatePage, model);
        }

        return await EngineService.CompileRenderAsync(path, model);
    }
}

...

services.AddScoped<IHtmlTemplateRendererService, RazorEngineService>();

Error stack trace (Azure, Linux, Core 3.1)

at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind) at System.Uri..ctor(String uriString) at System.UriBuilder..ctor(String uri) at RazorLight.Compilation.DefaultAssemblyDirectoryFormatter.GetAssemblyDirectory(Assembly assembly) at RazorLight.Compilation.DefaultMetadataReferenceManager.b__12_2(Assembly p) at System.Linq.Enumerable.SelectArrayIterator2.ToList() at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source) at RazorLight.Compilation.DefaultMetadataReferenceManager.Resolve(Assembly assembly, DependencyContext dependencyContext) at RazorLight.Compilation.DefaultMetadataReferenceManager.Resolve(Assembly assembly) at RazorLight.Compilation.RoslynCompilationService.EnsureOptions() at RazorLight.Compilation.RoslynCompilationService.get_ParseOptions() at RazorLight.Compilation.RoslynCompilationService.CreateSyntaxTree(SourceText sourceText) at RazorLight.Compilation.RoslynCompilationService.CreateCompilation(String compilationContent, String assemblyName) at RazorLight.Compilation.RoslynCompilationService.CompileAndEmit(IGeneratedRazorTemplate razorTemplate) at RazorLight.Compilation.RazorTemplateCompiler.CompileAndEmitAsync(RazorLightProjectItem projectItem) at RazorLight.Compilation.RazorTemplateCompiler.OnCacheMissAsync(String templateKey) at RazorLight.Compilation.RazorTemplateCompiler.OnCacheMissAsync(String templateKey) at RazorLight.EngineHandler.CompileTemplateAsync(String key) at RazorLight.EngineHandler.CompileRenderAsync[T](String key, T model, ExpandoObject viewBag) at BackupRadar.Templates.RazorEngineService.GenerateCodeAsync[T](String path, T model, String key) in ....\RazorEngineService.cs

The suggestion to use PassthroughAssemblyDirectoryFormatter didn't work for me I used the following code at application startup to replace:

        public class PassthroughAssemblyDirectoryFormatter : IAssemblyDirectoryFormatter
        {
            public string GetAssemblyDirectory(Assembly assembly)
            {
                return assembly.Location;
            }
        }

.... 
        services.Replace(ServiceDescriptor.Transient<IAssemblyDirectoryFormatter, PassthroughAssemblyDirectoryFormatter>());`

If you have some suggestions, please go ahead

jzabroski commented 2 years ago

@SerhiyBalan Can I get detailed info on your Linux environment, e.g., which linux distro and version and libc and glibc? Maybe best to open a new ticket. This is great - I should be able to figure out a better approach.