Make BinFmt changewave dependent on runtime enablement

Opting out of 17.8 features (via changewaves env variable) while `EnableUnsafeBinaryFormatterSerialization` is not allowed by the runtime can lead to MSBuild crashing ungracefully. This is because BinaryFormatter serialization usage in core now leads to runtime failures (as of 8.0) unless explicitly opted-in by user. So MSBuild moved to alternative (secure) serialization. MSBuild normally allows users to opt out of the last batch of changes - with changewaves. In this specific case it can mean shooting self into foot without realizing. Resolution: Ignoring the opt-out of the new secure serialization unless the BinaryFormatter is explicitly allowed by user in runtime (by editing `MSBuild.runtimeconfig.json` in the SDK).
dotnet · Sep 26, 2023 · 9f2e9b4 · 9f2e9b4
1 parent 6cdef42
commit 9f2e9b4
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 15 deletions.
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -26,8 +26,9 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 ### 17.8
 - [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)
 - [Delete destination file before copy](https://github.com/dotnet/msbuild/pull/8685)
-- [New serialization approach for transferring build exceptions between processes](https://github.com/dotnet/msbuild/pull/8779)
 - [Moving from SHA1 to SHA256 for Hash task](https://github.com/dotnet/msbuild/pull/8812)
+- [Deprecating custom derived BuildEventArgs](https://github.com/dotnet/msbuild/pull/8917) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`
+
 
 ### 17.6
 - [Parse invalid property under target](https://github.com/dotnet/msbuild/pull/8190)
@@ -68,4 +69,4 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Optimized immutable files up to date checks](https://github.com/dotnet/msbuild/pull/6974)
 - [Add Microsoft.IO.Redist for directory enumeration](https://github.com/dotnet/msbuild/pull/6771)
 - [Process-wide caching of ToolsetConfigurationSection](https://github.com/dotnet/msbuild/pull/6832)
-- [Normalize RAR output paths](https://github.com/dotnet/msbuild/pull/6533)
+- [Normalize RAR output paths](https://github.com/dotnet/msbuild/pull/6533)
diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs
@@ -587,7 +587,8 @@ private void SendPacket(INodePacket packet)
 #if RUNTIME_TYPE_NETCORE
                 if (packet is LogMessagePacketBase logMessage
                     && logMessage.EventType == LoggingEventType.CustomEvent 
-                    && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8) 
+                    &&
+                    (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8) || !Traits.Instance.EscapeHatches.IsBinaryFormatterSerializationAllowed)
                     && Traits.Instance.EscapeHatches.EnableWarningOnCustomBuildEvent)
                 {
                     BuildEventArgs buildEvent = logMessage.NodeBuildEvent.Value.Value;

diff --git a/src/Framework/BinaryTranslator.cs b/src/Framework/BinaryTranslator.cs
@@ -582,12 +582,6 @@ public void TranslateDotNet<T>(ref T value)
 
             public void TranslateException(ref Exception value)
             {
-                if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8))
-                {
-                    TranslateDotNet<Exception>(ref value);
-                    return;
-                }
-
                 if (!TranslateNullable(value))
                 {
                     return;
@@ -1291,12 +1285,6 @@ public void TranslateDotNet<T>(ref T value)
 
             public void TranslateException(ref Exception value)
             {
-                if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8))
-                {
-                    TranslateDotNet<Exception>(ref value);
-                    return;
-                }
-
                 if (!TranslateNullable(value))
                 {
                     return;

diff --git a/src/Framework/Traits.cs b/src/Framework/Traits.cs
@@ -400,6 +400,27 @@ public bool EnableWarningOnCustomBuildEvent
             }
         }
 
+        private bool? _isBinaryFormatterSerializationAllowed;
+        public bool IsBinaryFormatterSerializationAllowed
+        {
+            get
+            {
+                if (!_isBinaryFormatterSerializationAllowed.HasValue)
+                {
+#if RUNTIME_TYPE_NETCORE
+                    AppContext.TryGetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization",
+                        out bool enabled);
+                    _isBinaryFormatterSerializationAllowed = enabled;
+#else
+                    _isBinaryFormatterSerializationAllowed = true;
+#endif
+                }
+
+                return _isBinaryFormatterSerializationAllowed.Value;
+            }
+        }
+
+
         private static bool? ParseNullableBoolFromEnvironmentVariable(string environmentVariable)
         {
             var value = Environment.GetEnvironmentVariable(environmentVariable);