AssemblyName.GetPublicKeyToken() may return byte[160] instead of byte[8]

[AArnott]

This is a regression in .NET 7, possibly due to #62866.
This only repros when the AssemblyName is obtained from AssemblyName.GetAssemblyName when passed the path to an ILMerge output assembly.

Consider the following test:

string assemblyPath = Environment.ExpandEnvironmentVariables("ILmergeProduced.dll");
Assert.Equal(8, AssemblyName.GetAssemblyName(this.assemblyPath).GetPublicKeyToken()?.Length)

That test passes on net472 and net6.0, but fails on net7.0 with:

Assert.Equal() Failure
Expected: 8
Actual:   160

Consider this full repro.
Just open in VS and run tests in Test Explorer (assuming .NET 7 Preview 5 is installed).

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This is a regression in .NET 7, possibly due to #62866.
This only repros when the AssemblyName is obtained from AssemblyName.GetAssemblyName when passed the path to an ILMerge output assembly.

Consider the following test:

string assemblyPath = Environment.ExpandEnvironmentVariables("ILmergeProduced.dll");
Assert.Equal(8, AssemblyName.GetAssemblyName(this.assemblyPath).GetPublicKeyToken()?.Length)

That test passes on net472 and net6.0, but fails on net7.0 with:

Assert.Equal() Failure
Expected: 8
Actual:   160

Consider this full repro.
Just open in VS and run tests in Test Explorer (assuming .NET 7 Preview 5 is installed).

Author:	AArnott
Assignees:	-
Labels:	area-AssemblyLoader-coreclr, untriaged
Milestone:	-

[VSadov]

@AArnott - thanks for reporting. I will take a look.
I wonder if there could be a smaller repro - so that I could put it into a testcase.

[jkotas]

Related to or duplicate of #66785

[AArnott]

@VSadov the repro is just two lines of code, plus the assembly to be loaded. Maybe check in the assembly?

[VSadov]

This is not related to #66785 . That is a matter of visualizing missing PublicKeyToken - whether we write out PublicKeyToken=null or not if the key is missing.

It looks like 6.0 prints that and we should too, but I wonder if that is always the case or there are other hints to consider on whether to print this or not.

[VSadov]

I assume #69157 fixed this.

[jkotas]

#69157 did not fix this. #69157 affected Assembly.Load only. This bug is in AssemblyName.GetAssemblyName

[jkotas]

This is not related to #66785 . That is a matter of visualizing missing PublicKeyToken - whether we write out PublicKeyToken=null or not if the key is missing.

I think you meant to add this comment to #66785 .

I believe that the problem in both #66785 and #69153 (this issue) is that the AssemblyName.GetAssemblyName is setting the flags on AssemblyName differently from how they used to be set by internal runtime GetAssemblyName.

[VSadov]

I thought #66785 was an issue similar to the one that was fixed in

runtime/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/MetadataReader.netstandard.cs

Line 16 in 215b39a

// compat: normalize 'null' culture name to "" to match AssemblyName.GetAssemblyName()

just like with culture name we expect empty instead of null when metadata has Nil handle for the token

but maybe this is an issue with flags.

[jkotas]

AssemblyName.GetPublicKeyToken() may return byte[160] instead of byte[8]

The runtime unconditionally sets afPublicKey on the name read from assembly definition: https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/baseassemblyspec.inl#L310-L311 . Notice that this quirk is only applied to name read from assembly definition, but it is not applied to name read from assembly reference.

System.Reflection.Metadata does not have this quirk that compensates for mall-formed assemblies with missing afPublicKey bit.

[VSadov]

@AArnott Is it possible to share the dll that exhibits this behavior (ILmergeProduced.dll ?)

I have managed to make the attached test to run, but it is passing. I think it would be simpler if I just have the dll.

[AArnott]

@VSadov Yes, check out my full repro (linked from the issue description) to get an actual repro that includes the dll.

[VSadov]

With @AArnott help I was able to reproduce the bug. @jkotas is right, #69157 does not fix it.

The fix in #69169 (i.e. forcing PublicKey bit in definitions) is not sufficient either, but having a consistent repro definitely helps.