Prohibit BF unless the app opts in (dotnet#48527)

* Prohibit BF unless the app opts in * Address feedback * First unit test * 2nd unit test. Checkpoint * sq * sq * sq * Address Levi's feedback * sq * Address nits * Linker changes and tests * sq * sq * Linker warnings ids * Address comments * Change trimming test so linker can detect a pattern * sq * Address comments * sq Co-authored-by: Eric Erhardt <[email protected]>
macsux · Mar 31, 2021 · 3c135c5 · 3c135c5
1 parent 28629d9
commit 3c135c5
Show file tree

Hide file tree

Showing 14 changed files with 423 additions and 20 deletions.
diff --git a/docs/workflow/trimming/feature-switches.md b/docs/workflow/trimming/feature-switches.md
@@ -19,6 +19,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif
 | StartupHookSupport | System.StartupHookProvider.IsSupported | Startup hooks are disabled when set to false. Startup hook related functionality can be trimmed. |
 | TBD | System.Threading.ThreadPool.EnableDispatchAutoreleasePool | When set to true, creates an NSAutoreleasePool around each thread pool work item on applicable platforms. |
 | CustomResourceTypesSupport | System.Resources.ResourceManager.AllowCustomResourceTypes | Use of custom resource types is disabled when set to false. ResourceManager code paths that use reflection for custom types can be trimmed. |
+| EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization | System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization | BinaryFormatter serialization support is trimmed when set to false. |
 
 Any feature-switch which defines property can be set in csproj file or
 on the command line as any other MSBuild property. Those without predefined property name

diff --git a/eng/testing/linker/project.csproj.template b/eng/testing/linker/project.csproj.template
@@ -29,6 +29,7 @@
 
   <ItemGroup>
     {AdditionalProjectReferences}
+    {TrimmerRootAssemblies}
   </ItemGroup>
 
   <!-- Logic to override the default IlLink tasks that come from the SDK and use the one

diff --git a/eng/testing/linker/trimmingTests.targets b/eng/testing/linker/trimmingTests.targets
@@ -66,6 +66,11 @@
       <_additionalProjectSourceFiles Include="%(TestConsoleApps.AdditionalSourceFiles)" />
     </ItemGroup>
 
+    <PropertyGroup>
+      <_TrimmerRootAssemblies>%(TestConsoleApps.TrimmerRootAssemblies)</_TrimmerRootAssemblies>
+      <_TrimmerRootAssemblies Condition="'$(_TrimmerRootAssemblies)' != ''">&lt;TrimmerRootAssembly Include=&quot;$(_TrimmerRootAssemblies)&quot; /&gt;</_TrimmerRootAssemblies>
+    </PropertyGroup>
+
     <MakeDir Directories="$(_projectDir)" />
     <WriteLinesToFile File="$(_projectFile)"
                       Lines="$([System.IO.File]::ReadAllText('$(ProjectTemplate)')
@@ -77,6 +82,7 @@
                                                  .Replace('{MicrosoftNETILLinkTasksVersion}', '$(MicrosoftNETILLinkTasksVersion)')
                                                  .Replace('{ExtraTrimmerArgs}', '%(TestConsoleApps.ExtraTrimmerArgs)')
                                                  .Replace('{AdditionalProjectReferences}', '$(_additionalProjectReferencesString)')
+                                                 .Replace('{TrimmerRootAssemblies}', '$(_TrimmerRootAssemblies)')
                                                  .Replace('{RepositoryEngineeringDir}', '$(RepositoryEngineeringDir)')
                                                  .Replace('{MonoAOTCompilerDir}', '$(MonoAOTCompilerDir)')
                                                  .Replace('{MonoProjectRoot}', '$(MonoProjectRoot)')

diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.ComponentModel.TypeConverter/src/ILLink/ILLink.Substitutions.xml
@@ -0,0 +1,7 @@
+<linker>
+  <assembly fullname="System.ComponentModel.TypeConverter">
+    <type fullname="System.ComponentModel.Design.DesigntimeLicenseContextSerializer" feature="System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization" featurevalue="false">
+      <method signature="System.Boolean get_EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization()" body="stub" value="false" />
+    </type>
+  </assembly>
+</linker>
diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.ComponentModel.TypeConverter/src/ILLink/ILLink.Suppressions.xml
@@ -1,18 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <linker>
   <assembly fullname="System.ComponentModel.TypeConverter, PublicKeyToken=b03f5f7f11d50a3a">
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2026</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.ComponentModel.Design.DesigntimeLicenseContextSerializer.Deserialize(System.IO.Stream,System.String,System.ComponentModel.Design.RuntimeLicenseContext)</property>
-    </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2026</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.ComponentModel.Design.DesigntimeLicenseContextSerializer.Serialize(System.IO.Stream,System.String,System.ComponentModel.Design.DesigntimeLicenseContext)</property>
-    </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
       <argument>IL2072</argument>

diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx
@@ -250,4 +250,7 @@
   <data name="MemberRelationshipService_RelationshipNotSupported" xml:space="preserve">
     <value>Relationships between {0}.{1} and {2}.{3} are not supported.</value>
   </data>
+  <data name="BinaryFormatterMessage" xml:space="preserve">
+    <value>BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information.</value>
+  </data>
 </root>
diff --git a/...raries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj b/...raries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj
@@ -4,6 +4,9 @@
     <IsPartialFacadeAssembly>true</IsPartialFacadeAssembly>
     <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
   </PropertyGroup>
+  <ItemGroup>
+    <ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
+  </ItemGroup>
   <ItemGroup>
     <Compile Include="MS\Internal\Xml\Linq\ComponentModel\XComponentModel.cs" />
     <Compile Include="System\ComponentModel\ArrayConverter.cs" />

diff --git a/...odel.TypeConverter/src/System/ComponentModel/Design/DesigntimeLicenseContextSerializer.cs b/...odel.TypeConverter/src/System/ComponentModel/Design/DesigntimeLicenseContextSerializer.cs
@@ -5,6 +5,7 @@
 using System.Runtime.Serialization;
 using System.Collections;
 using System.IO;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 
 namespace System.ComponentModel.Design
@@ -14,6 +15,10 @@ namespace System.ComponentModel.Design
     /// </summary>
     public class DesigntimeLicenseContextSerializer
     {
+        internal const byte BinaryWriterMagic = 255;
+
+        private static bool EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization { get; } = AppContext.TryGetSwitch("System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization", out bool isEnabled) ? isEnabled : false;
+
         // Not creatable.
         private DesigntimeLicenseContextSerializer()
         {
@@ -24,26 +29,156 @@ private DesigntimeLicenseContextSerializer()
         /// using the specified key and output stream.
         /// </summary>
         public static void Serialize(Stream o, string cryptoKey, DesigntimeLicenseContext context)
+        {
+            if (EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization)
+            {
+                SerializeWithBinaryFormatter(o, cryptoKey, context);
+            }
+            else
+            {
+                using (BinaryWriter writer = new BinaryWriter(o, encoding: Text.Encoding.UTF8, leaveOpen: true))
+                {
+                    writer.Write(BinaryWriterMagic); // flag to identify BinaryWriter
+                    writer.Write(cryptoKey);
+                    writer.Write(context._savedLicenseKeys.Count);
+                    foreach (DictionaryEntry keyAndValue in context._savedLicenseKeys)
+                    {
+                        writer.Write(keyAndValue.Key.ToString());
+                        writer.Write(keyAndValue.Value.ToString());
+                    }
+                }
+            }
+        }
+
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
+            Justification = "Only simple types (string and HashTable) are serialized with BinaryFormatter. These types can be serialized in a trimmed application.")]
+        private static void SerializeWithBinaryFormatter(Stream o, string cryptoKey, DesigntimeLicenseContext context)
         {
             IFormatter formatter = new BinaryFormatter();
-#pragma warning disable SYSLIB0011 // Issue https://github.com/dotnet/runtime/issues/39293 tracks finding an alternative to BinaryFormatter
+#pragma warning disable SYSLIB0011
             formatter.Serialize(o, new object[] { cryptoKey, context._savedLicenseKeys });
 #pragma warning restore SYSLIB0011
         }
 
-        internal static void Deserialize(Stream o, string cryptoKey, RuntimeLicenseContext context)
+        private class StreamWrapper : Stream
         {
-#pragma warning disable SYSLIB0011 // Issue https://github.com/dotnet/runtime/issues/39293 tracks finding an alternative to BinaryFormatter
-            IFormatter formatter = new BinaryFormatter();
+            private Stream _stream;
+            private bool _readFirstByte;
+            internal byte _firstByte;
+            public StreamWrapper(Stream stream)
+            {
+                _stream = stream;
+                _readFirstByte = false;
+                _firstByte = 0;
+            }
+
+            public override bool CanRead => _stream.CanRead;
+
+            public override bool CanSeek => _stream.CanSeek;
+
+            public override bool CanWrite => _stream.CanWrite;
+
+            public override long Length => _stream.Length;
+
+            public override long Position { get => _stream.Position; set => _stream.Position = value; }
+
+            public override void Flush() => _stream.Flush();
+
+            public override int Read(byte[] buffer, int offset, int count)
+            {
+                Debug.Assert(_stream.Position != 0, "Expected the first byte to be read first");
+                if (_stream.Position == 1)
+                {
+                    Debug.Assert(_readFirstByte == true);
+                    // Add the first byte read by ReadByte into buffer here
+                    buffer[offset] = _firstByte;
+                    return _stream.Read(buffer, offset + 1, count - 1) + 1;
+                }
+                return _stream.Read(buffer, offset, count);
+            }
+
+            public override long Seek(long offset, SeekOrigin origin) => _stream.Seek(offset, origin);
+
+            public override void SetLength(long value) => _stream.SetLength(value);
 
-            object obj = formatter.Deserialize(o);
+            public override void Write(byte[] buffer, int offset, int count) => _stream.Write(buffer, offset, count);
+
+            public override int ReadByte()
+            {
+                byte read = (byte)_stream.ReadByte();
+                _firstByte = read;
+                _readFirstByte = true;
+                return read;
+            }
+        }
+
+        /// <summary>
+        /// During deserialization, the stream passed in may be binary formatted or may have used binary writer. This is a quick test to discern between them.
+        /// </summary>
+        private static bool StreamIsBinaryFormatted(StreamWrapper stream)
+        {
+            // For binary formatter, the first byte is the SerializationHeaderRecord and has a value 0
+            int firstByte = stream.ReadByte();
+            if (firstByte != 0)
+            {
+                return false;
+            }
+
+            return true;
+        }
+
+        [DynamicDependency(DynamicallyAccessedMemberTypes.NonPublicConstructors, typeof(Hashtable))]
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
+            Justification = "HashTable's Serialization ctor will be preserved by the DynamicDependency.")]
+        private static void DeserializeUsingBinaryFormatter(StreamWrapper wrappedStream, string cryptoKey, RuntimeLicenseContext context)
+        {
+            if (EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization)
+            {
+#pragma warning disable SYSLIB0011
+                IFormatter formatter = new BinaryFormatter();
+
+                object obj = formatter.Deserialize(wrappedStream);
 #pragma warning restore SYSLIB0011
 
-            if (obj is object[] value)
+                if (obj is object[] value)
+                {
+                    if (value[0] is string && (string)value[0] == cryptoKey)
+                    {
+                        context._savedLicenseKeys = (Hashtable)value[1];
+                    }
+                }
+            }
+            else
+            {
+                throw new NotSupportedException(SR.BinaryFormatterMessage);
+            }
+        }
+
+        internal static void Deserialize(Stream o, string cryptoKey, RuntimeLicenseContext context)
+        {
+            StreamWrapper wrappedStream = new StreamWrapper(o);
+            if (StreamIsBinaryFormatted(wrappedStream))
+            {
+                DeserializeUsingBinaryFormatter(wrappedStream, cryptoKey, context);
+            }
+            else
             {
-                if (value[0] is string && (string)value[0] == cryptoKey)
+                using (BinaryReader reader = new BinaryReader(wrappedStream, encoding: Text.Encoding.UTF8, leaveOpen: true))
                 {
-                    context._savedLicenseKeys = (Hashtable)value[1];
+                    byte binaryWriterIdentifer = wrappedStream._firstByte;
+                    Debug.Assert(binaryWriterIdentifer == BinaryWriterMagic, $"Expected the first byte to be {BinaryWriterMagic}");
+                    string streamCryptoKey = reader.ReadString();
+                    int numEntries = reader.ReadInt32();
+                    if (streamCryptoKey == cryptoKey)
+                    {
+                        context._savedLicenseKeys.Clear();
+                        for (int i = 0; i < numEntries; i++)
+                        {
+                            string key = reader.ReadString();
+                            string value = reader.ReadString();
+                            context._savedLicenseKeys.Add(key, value);
+                        }
+                    }
                 }
             }
         }