Better handle resx scenarios (dotnet/corefx#38012)

* Better handle resx scenarios

There were two resx scenarios that we weren't handling well.

1. BinaryFormatted data that is missing the type information

BinaryFormatted data never takes the type into account, it's only used
to check the deserialized data after it's read.  In the old resx reader
it would deserialize the data in the build task, only to reserialize it
back, recording the type information.  Since we're eliminating build
time deserialziation we cannot do this, so just permit the payload to
flow through without recording the type information.  This is
effectively what happened before since the user never recorded the
type information in the resx so it isn't introducing any new opportunity
for inconsistencies.  To implement this I used the existing ResX format
with a sentinel type to indicate that the BinaryFormatter payload type
was unknown.

2. Primitive types stored as string

ResX reader deserialized all types during the build, we're trying to
eliminate this as it results in build time / cross-framework type
loading.  In doing so we lose the ability to handle primitive types
since the only way we currently write primitive types is when they
are passed in as live objects.  To fix this, we'll make the string-based
type converter method aware of primitive types, and permit it to
deserialize those primitive types (IOW: parse the string via
typeconverter) so that we still write these as primitive resources.
We'll rename this method to AddResource to indicate it is more
generic than just handling pre-serialized data.  To identify primitive types
we use a string comparer to match the type name written in the resx,
and map it to a known type (in the build framework).

* Respond to feedback

* Apply suggestions from code review

Co-Authored-By: Rainer Sigwald <[email protected]>


Commit migrated from dotnet/corefx@00c7405

src/libraries/System.Resources.Extensions/ref/System.Resources.Extensions.cs

@@ -25,13 +25,14 @@ public void AddResource(string name, byte[] value) { }
         public void AddResource(string name, System.IO.Stream value, bool closeAfterWrite = false) { }
         public void AddResource(string name, object value) { }
         public void AddResource(string name, string value) { }
+        public void AddResource(string name, string typeName, string value) { }
         public void Close() { }
         public void Dispose() { }
         public void Generate() { }
 
+        public void AddBinaryFormattedResource(string name, byte[] value) { }
         public void AddBinaryFormattedResource(string name, string typeName, byte[] value) { }
         public void AddActivatorResource(string name, string typeName, System.IO.Stream value, bool closeAfterWrite = false) { }
         public void AddTypeConverterResource(string name, string typeName, byte[] value) { }
-        public void AddTypeConverterResource(string name, string typeName, string value) { }
     }
 }

src/libraries/System.Resources.Extensions/src/System.Resources.Extensions.csproj

@@ -19,6 +19,10 @@
     <Compile Include="System\Resources\Extensions\DeserializingResourceReader.cs" />
     <Compile Include="System\Resources\Extensions\PreserializedResourceWriter.cs" />
     <Compile Include="System\Resources\Extensions\SerializationFormat.cs" />
+    <Compile Include="System\Resources\Extensions\TypeNameComparer.cs" />
+    <Compile Include="$(CommonPath)\CoreLib\System\Numerics\Hashing\HashHelpers.cs">
+      <Link>System\Numerics\Hashing\HashHelpers.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup>
     <Reference Include="System.Memory" />

src/libraries/System.Resources.Extensions/src/System/Resources/Extensions/DeserializingResourceReader.cs

@@ -2,28 +2,28 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
 using System.ComponentModel;
 using System.IO;
-using System.Reflection;
 using System.Runtime.Serialization.Formatters.Binary;
 
 namespace System.Resources.Extensions
 {
     public partial class DeserializingResourceReader
     {
         private bool _assumeBinaryFormatter = false;
-        private BinaryFormatter _formatter = null;
+        private BinaryFormatter? _formatter = null;
 
         private bool ValidateReaderType(string readerType)
         {
             // our format?
-            if (CompareNames(readerType, PreserializedResourceWriter.DeserializingResourceReaderFullyQualifiedName))
+            if (TypeNameComparer.Instance.Equals(readerType, PreserializedResourceWriter.DeserializingResourceReaderFullyQualifiedName))
             {
                 return true;
             }
 
             // default format?
-            if (CompareNames(readerType, PreserializedResourceWriter.ResourceReaderFullyQualifiedName))
+            if (TypeNameComparer.Instance.Equals(readerType, PreserializedResourceWriter.ResourceReaderFullyQualifiedName))
             {
                 // we can read the default format, we just assume BinaryFormatter and don't
                 // read the SerializationFormat
@@ -74,6 +74,13 @@ private object DeserializeObject(int typeIndex)
 
                         value = ReadBinaryFormattedObject();
 
+                        if (type == typeof(UnknownType))
+                        {
+                            // type information was omitted at the time of writing
+                            // allow the payload to define the type
+                            type = value.GetType();
+                        }
+
                         long bytesRead = _store.BaseStream.Position - originalPosition;
 
                         // Ensure BF read what we expected.
@@ -160,48 +167,5 @@ private object DeserializeObject(int typeIndex)
             return value;
         }
 
-        // Compare two type names ignoring version
-        private static bool CompareNames(string typeName1, string typeName2)
-        {
-            // First, compare type names
-            int comma1 = typeName1.IndexOf(',');
-            int comma2 = typeName2.IndexOf(',');
-            if (comma1 != comma2)
-                return false;
-
-            // both are missing assembly name, compare entire string as type name
-            if (comma1 == -1)
-                return string.Equals(typeName1, typeName2, StringComparison.Ordinal);
-
-            // compare the type name portion
-            ReadOnlySpan<char> type1 = typeName1.AsSpan(0, comma1);
-            ReadOnlySpan<char> type2 = typeName2.AsSpan(0, comma2);
-            if (!type1.Equals(type2, StringComparison.Ordinal))
-                return false;
-
-            // Now, compare assembly display names (IGNORES VERSION AND PROCESSORARCHITECTURE)
-            // also, for  mscorlib ignores everything, since that's what the binder is going to do
-            while (Char.IsWhiteSpace(typeName1[++comma1]))
-                ;
-            while (Char.IsWhiteSpace(typeName2[++comma2]))
-                ;
-
-            // case insensitive
-            AssemblyName an1 = new AssemblyName(typeName1.Substring(comma1));
-            AssemblyName an2 = new AssemblyName(typeName2.Substring(comma2));
-            if (!string.Equals(an1.Name, an2.Name, StringComparison.OrdinalIgnoreCase))
-                return false;
-
-            // to match IsMscorlib() in VM
-            if (string.Equals(an1.Name, "mscorlib", StringComparison.OrdinalIgnoreCase))
-                return true;
-
-            if (an1.CultureInfo?.LCID != an2.CultureInfo?.LCID)
-                return false;
-
-            byte[] pkt1 = an1.GetPublicKeyToken();
-            byte[] pkt2 = an2.GetPublicKeyToken();
-            return pkt1.AsSpan().SequenceEqual(pkt2);
-        }
     }
 }

src/libraries/System.Resources.Extensions/src/System/Resources/Extensions/PreserializedResourceWriter.cs

@@ -2,11 +2,16 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+using System.Collections.Generic;
+using System.ComponentModel;
 using System.Diagnostics;
 using System.IO;
 
 namespace System.Resources.Extensions
 {
+    internal class UnknownType { }
+
     partial class PreserializedResourceWriter
     {
         // indicates if the types of resources saved will require the DeserializingResourceReader
@@ -17,6 +22,11 @@ partial class PreserializedResourceWriter
         internal const string DeserializingResourceReaderFullyQualifiedName = "System.Resources.Extensions.DeserializingResourceReader, System.Resources.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51";
         internal const string RuntimeResourceSetFullyQualifiedName = "System.Resources.Extensions.RuntimeResourceSet, System.Resources.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51";
 
+        // an internal type name used to represent an unknown resource type, explicitly omit version to save
+        // on size and avoid changes in user resources.  This works since we only ever load this type name 
+        // from calls to GetType from this assembly.
+        private static readonly string UnknownObjectTypeName = typeof(UnknownType).FullName;
+
         private string ResourceReaderTypeName => _requiresDeserializingResourceReader ?
             DeserializingResourceReaderFullyQualifiedName :
             ResourceReaderFullyQualifiedName;
@@ -25,14 +35,42 @@ partial class PreserializedResourceWriter
             RuntimeResourceSetFullyQualifiedName :
             ResSetTypeName;
 
+        // a collection of primitive types in a dictionary, indexed by type name
+        // using a comparer which handles type name comparisons similar to what
+        // is done by reflection
+        private static readonly IReadOnlyDictionary<string, Type> s_primitiveTypes = new Dictionary<string, Type>(16, TypeNameComparer.Instance)
+        {
+            { typeof(string).FullName, typeof(string) },
+            { typeof(int).FullName, typeof(int) },
+            { typeof(bool).FullName, typeof(bool) },
+            { typeof(char).FullName, typeof(char) },
+            { typeof(byte).FullName, typeof(byte) },
+            { typeof(sbyte).FullName, typeof(sbyte) },
+            { typeof(short).FullName, typeof(short) },
+            { typeof(long).FullName, typeof(long) },
+            { typeof(ushort).FullName, typeof(ushort) },
+            { typeof(uint).FullName, typeof(uint) },
+            { typeof(ulong).FullName, typeof(ulong) },
+            { typeof(float).FullName, typeof(float) },
+            { typeof(double).FullName, typeof(double) },
+            { typeof(decimal).FullName, typeof(decimal) },
+            { typeof(DateTime).FullName, typeof(DateTime) },
+            { typeof(TimeSpan).FullName, typeof(TimeSpan) }
+            // byte[] and Stream are primitive types but do not define a conversion from string
+        };
+
         /// <summary>
-        /// Adds a resource of specified type represented by a string value which will be 
-        /// passed to the type's TypeConverter when reading the resource.
+        /// Adds a resource of specified type represented by a string value.
+        /// If the type is a primitive type, the value will be converted using TypeConverter by the writer
+        /// to that primitive type and stored in the resources in binary format.
+        /// If the type is not a primitive type, the string value will be stored in the resources as a 
+        /// string and converted with a TypeConverter for the type when reading the resource.  
+        /// This is done to avoid activating arbitrary types during resource writing.
         /// </summary>
         /// <param name="name">Resource name</param>
         /// <param name="typeName">Assembly qualified type name of the resource</param>
         /// <param name="value">Value of the resource in string form understood by the type's TypeConverter</param>
-        public void AddTypeConverterResource(string name, string typeName, string value)
+        public void AddResource(string name, string typeName, string value)
         {
             if (name == null)
                 throw new ArgumentNullException(nameof(name));
@@ -41,9 +79,39 @@ public void AddTypeConverterResource(string name, string typeName, string value)
             if (value == null)
                 throw new ArgumentNullException(nameof(value));
 
-            AddResourceData(name, typeName, new ResourceDataRecord(SerializationFormat.TypeConverterString, value));
+            // determine if the type is a primitive type
+            if (s_primitiveTypes.TryGetValue(typeName, out Type primitiveType))
+            {
+                // directly add strings
+                if (primitiveType == typeof(string))
+                {
+                    AddResource(name, value);
+                }
+                else
+                {
+                    // for primitive types that are not strings, convert the string value to the 
+                    // primitive type value.  
+                    // we intentionally avoid calling GetType on the user provided type name
+                    // and instead will only ever convert to one of the known types.
+                    TypeConverter converter = TypeDescriptor.GetConverter(primitiveType);
+
+                    if (converter == null)
+                    {
+                        throw new TypeLoadException(SR.Format(SR.TypeLoadException_CannotLoadConverter, primitiveType));
+                    }
 
-            _requiresDeserializingResourceReader = true;
+                    object primitiveValue = converter.ConvertFromInvariantString(value);
+
+                    Debug.Assert(primitiveValue.GetType() == primitiveType);
+
+                    AddResource(name, primitiveValue);
+                }
+            }
+            else
+            {
+                AddResourceData(name, typeName, new ResourceDataRecord(SerializationFormat.TypeConverterString, value));
+                _requiresDeserializingResourceReader = true;
+            }
         }
 
         /// <summary>
@@ -67,6 +135,29 @@ public void AddTypeConverterResource(string name, string typeName, byte[] value)
             _requiresDeserializingResourceReader = true;
         }
 
+        /// <summary>
+        /// Adds a resource of unknown type represented by a byte[] value which will be 
+        /// passed to BinaryFormatter when reading the resource.
+        /// </summary>
+        /// <param name="name">Resource name</param>
+        /// <param name="value">Value of the resource in byte[] form understood by BinaryFormatter</param>
+        public void AddBinaryFormattedResource(string name, byte[] value)
+        {
+            if (name == null)
+                throw new ArgumentNullException(nameof(name));
+            if (value == null)
+                throw new ArgumentNullException(nameof(value));
+
+            // Some resx-files are missing type information for binary-formatted resources.
+            // These would have previously been handled by deserializing once, capturing the type
+            // and reserializing when writing the resources.  We don't want to do that so instead
+            // we just omit the type.
+            AddResourceData(name, UnknownObjectTypeName, new ResourceDataRecord(SerializationFormat.BinaryFormatter, value));
+
+            // ResourceReader will validate the type so we must use the new reader.
+            _requiresDeserializingResourceReader = true;
+        }
+
         /// <summary>
         /// Adds a resource of specified type represented by a byte[] value which will be 
         /// passed to BinaryFormatter when reading the resource.
@@ -127,7 +218,7 @@ internal ResourceDataRecord(SerializationFormat format, object data, bool closeA
 
         private void WriteData(BinaryWriter writer, object dataContext)
         {
-            ResourceDataRecord record = dataContext as ResourceDataRecord;
+            ResourceDataRecord? record = dataContext as ResourceDataRecord;
 
             Debug.Assert(record != null);