Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib/Microsoft.Fx.Portability/DataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public static class DataExtensions
new JsonMultiDictionaryConverter<MemberInfo, AssemblyInfo>(),
new JsonToStringConverter<FrameworkName>(s => new FrameworkName(s)),
new JsonToStringConverter<Version>(s => new Version(s)),
new JsonPublicKeyTokenConverter(),
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/lib/Microsoft.Fx.Portability/DocumentationLinks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ public static class DocumentationLinks
{
public static readonly Uri PrivacyPolicy = new Uri("https://privacy.microsoft.com/en-us/privacystatement");

public static readonly Uri About = new Uri("https://go.microsoft.com/fwlink/?LinkId=506955");
public static readonly Uri About = new Uri("https://aka.ms/dotnet-portabilityanalyzer");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Microsoft.Fx.Portability.ObjectModel;
using Newtonsoft.Json;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Security.Cryptography.X509Certificates;
using System.Text;

namespace Microsoft.Fx.Portability.Utils.JsonConverters
{
internal class JsonPublicKeyTokenConverter : JsonConverter<PublicKeyToken>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this serialization used and what is it optimized for? Serializing as a string would be a much more efficient and easy to deal with encoding, but this looks like it would work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably used in returning a response or sending a request. Must not have any tests associated with it so my change wasn't caught.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twsouthwick This PR fix the current problem if both client and server sides using the new binary contains PublicKeyToken struct, but if old version of client without knowledge of PublicKeyToken works with new Server, the serialization will still break. Looks like PR #784 broke backward compat serialization.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to have a converter that would work for both. Do a string serialization (maybe that's what @marklio was referring to) and it should then work for backwards compatibility

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can serialize with PublicKeyToken.ToString() and deserialize with PublicKeyToken.Parse(string). That should provide round tripping

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted a new commit. Verified working with backward compat.

{
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
ImmutableArray<byte> bytes = serializer
.Deserialize<ImmutableArray<byte>>(reader);
return new PublicKeyToken(bytes);
}

public override void WriteJson(JsonWriter writer, object obj, JsonSerializer serializer)
{
PublicKeyToken pkToken = (PublicKeyToken)obj;
serializer.Serialize(writer, (ImmutableArray<byte>)pkToken.Token);
}
}
}