Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AssemblyNameInfo.FullName does not include public key #102051

Closed
jkotas opened this issue May 9, 2024 · 7 comments · Fixed by #105649
Closed

AssemblyNameInfo.FullName does not include public key #102051

jkotas opened this issue May 9, 2024 · 7 comments · Fixed by #105649

Comments

@jkotas
Copy link
Member

jkotas commented May 9, 2024

Repro

using System.Reflection;
using System.Reflection.Metadata;

var mscorlib = "mscorlib, Version=4.0.0.0, Culture=neutral, PublicKey=00000000000000000400000000000000";

Console.WriteLine(new AssemblyName(mscorlib).FullName);
Console.WriteLine(AssemblyNameInfo.Parse(mscorlib).FullName);

Actual result

mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
mscorlib, Version=4.0.0.0, Culture=neutral

Expected result

PublicKey (or PublicKeyToken) is included in AssemblyNameInfo.FullName

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 9, 2024
@jkotas jkotas added area-System.Reflection.Metadata and removed untriaged New issue has not been triaged by the area owner area-System.Reflection labels May 9, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member Author

jkotas commented May 9, 2024

@adamsitnik I have run into this while working on #102036. I see that you have mentioned this behavior in the documentation, but it is not the behavior that I would expect.

Somewhat related to this issue is a problem with conversions of PublicKey to PublicKeyToken. In #102036, I ended up calling into AssemblyName for this conversion in number of places, but it does not look pretty. Would it make sense to have a public API for it?

@adamsitnik
Copy link
Member

PublicKey (or PublicKeyToken) is included in AssemblyNameInfo.FullName

IIRC we did not want the AssemblyNameInfo to perform any conversions and validations of the PublicKey[Token].

Would including PublicKey in AssemblyNameInfo.FullName be enough for your scenario?

Have used it here just to let it be a key in the dictionary (and part of the info was missing) or did you also want to treat two AssemblyNameInfo instances, one with PublicKey and another with PublicKeyToken that map to the same PublicKeyToken as equal?

@jkotas
Copy link
Member Author

jkotas commented May 10, 2024

IIRC we did not want the AssemblyNameInfo to perform any conversions and validations of the PublicKey[Token].

I do like that AssemblyNameInfo does not perform any implicit conversions of PublicKey[Token]. I think it is the right design decision.

There are situations where one needs to do an explicit PublicKey -> PublicKeyToken conversion. There is no straightforward way to do it. I think we may want to add an API for the explicit conversion.

Would including PublicKey in AssemblyNameInfo.FullName be enough for your scenario?

Yes, I would address the behavior that I did not expect. I assumed that AssemblyNameInfo.FullName/AssemblyNameInfo.Parse is going to roundtrip all (non-obsolete) properties.

Have used it here just to let it be a key in the dictionary

I have run into several different issues while coding my change. In the specific place you have linked, the missing conversion would just result into inefficiency (much longer public key stored into binary instead of public key token).

@adamsitnik adamsitnik added this to the 9.0.0 milestone May 10, 2024
@adamsitnik
Copy link
Member

Yes, I would address the behavior that I did not expect.

Great, then we agree on the solution.

When it comes to the fix I am currently busy with... backporting Full Framework with Security updates and finishing the PayloadReader API. I think I should be able to get back to this in +- 2 weeks.

@adamsitnik adamsitnik self-assigned this May 15, 2024
@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 26, 2024

@adamsitnik is this still planned for 9.0?

@adamsitnik
Copy link
Member

@adamsitnik is this still planned for 9.0?

Yes, thank you for the reminder! I've sent #105649 to address it.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants