Handle potential null in ALC.ResolveSatelliteAssembly

[CoffeeFlux]

This was found on wasm, where assemblies are primarily loaded from a bundle rather than on the filesystem itself and return "" for Assembly.Location. This bug should affect desktop too if you have System.Private.CoreLib in the root of the file system (for whatever reason).

This manifested oddly in dotnet/aspnetcore#24773 because we threw an exception while trying to look up the corlib satellite assembly, which in turn triggered another load on that same resource due to the exception text, the usual recursive problem. The solution is to not throw the exception in the first place.

[jkotas]

return "/" for Assembly.Location.

I would expect these to return "" for Assembly.Location . Do they really return "/" ? That sounds like another bug.

[jkotas]

Do we need a test for this? This should be testable for all targets platforms.

[CoffeeFlux]

Sorry, it does return "", got mixed up there. The Mono behavior for that is correct, and I'll edit the description.

This seems somewhat annoying to test on non-wasm since I don't know how to actually run a test suite from the root of a file system. I'm not sure how I'd go about it and would like to get this fix to the aspnetcore team asap, but if you can offer some direction on how to actually test this in practice I'm happy to add it.

[jkotas]

This seems somewhat annoying to test

Can the problem be triggered by resolving satellite assembly for a user assembly with empty Location?

[CoffeeFlux]

I think it'll only crash with the corlib resource assembly? Normally we would just swallow the error and keep moving with the loading algorithm, but the recursive lookup is what's causing problems.

I guess I could load an assembly from memory and then call ResolveSatelliteAssembly on its ALC with a corresponding resources file via reflection to trigger this otherwise? Is that worth testing?

[CoffeeFlux]

I guess the alternative for testing is to just create a single-file project on desktop (probably with a Mono bundle) and test with that, which seems more useful but again somewhat difficult to fit into our current testing scheme. I'm not opposed to that route, but I'd rather open an issue and get this PR in without since I think it would be fairly involved.

[jkotas]

If the problem can be hit for CoreLib only, then I agree it is too difficult to add a test for it, not worth it.

[SamMonoRT]

Looks good to merge.