Obsolete Encoding.UTF7 property and UTF7Encoding ctors (dotnet#37535)

- Disallow Encoding.GetEncoding("utf-7", ...) from returning an Encoding instance - Minor code cleanup by removing unused ifdefs - A compat switch is available to re-enable Encoding.GetEncoding("utf-7", ...)
TIHan · Jun 30, 2020 · 74cfc0f · 74cfc0f
1 parent 7007cc2
commit 74cfc0f
Show file tree

Hide file tree

Showing 27 changed files with 329 additions and 56 deletions.
diff --git a/docs/project/list-of-obsoletions.md b/docs/project/list-of-obsoletions.md
@@ -13,5 +13,5 @@ Currently the identifiers `MSLIB0001` through `MSLIB0999` are carved out for obs
 
 | Diagnostic ID    | Description |
 | :--------------- | :---------- |
-|  __`MSLIB0001`__ | (Reserved for `Encoding.UTF7`.) |
+|  __`MSLIB0001`__ | The UTF-7 encoding is insecure and should not be used. Consider using UTF-8 instead. |
 |  __`MSLIB0002`__ | `PrincipalPermissionAttribute` is not honored by the runtime and must not be used. |
diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
@@ -369,14 +369,7 @@ private set
 
                 if (value != null &&
                         (value.Equals(Encoding.BigEndianUnicode)
-                        || value.Equals(Encoding.Unicode)
-#if FEATURE_UTF32
-                        || value.Equals(Encoding.UTF32)
-#endif // FEATURE_UTF32
-#if FEATURE_UTF7
-                        || value.Equals(Encoding.UTF7)
-#endif // FEATURE_UTF7
-                        ))
+                        || value.Equals(Encoding.Unicode)))
                 {
                     throw new ArgumentException(SR.EntryNameEncodingNotSupported, nameof(EntryNameEncoding));
                 }

diff --git a/src/libraries/System.IO.Ports/tests/SerialPort/Encoding.cs b/src/libraries/System.IO.Ports/tests/SerialPort/Encoding.cs
@@ -111,7 +111,7 @@ public void Encoding_ISCIIAssemese()
         public void Encoding_UTF7()
         {
             Debug.WriteLine("Verifying UTF7Encoding Encoding");
-            VerifyException(Encoding.UTF7, ThrowAt.Set, typeof(ArgumentException));
+            VerifyException(LegacyUTF7Encoding, ThrowAt.Set, typeof(ArgumentException));
         }
 
         [ConditionalFact(nameof(HasOneSerialPort))]

diff --git a/src/libraries/System.IO.Ports/tests/SerialPort/ReadTo.cs b/src/libraries/System.IO.Ports/tests/SerialPort/ReadTo.cs
@@ -691,7 +691,7 @@ private void PerformReadOnCom1FromCom2(SerialPort com1, SerialPort com2, string
             int totalCharsRead;
             int lastIndexOfNewLine = -newLineStringLength;
             string expectedString;
-            bool isUTF7Encoding = com1.Encoding.EncodingName == Encoding.UTF7.EncodingName;
+            bool isUTF7Encoding = IsUTF7Encoding(com1.Encoding);
 
             char[] charsToWrite = strToWrite.ToCharArray();
             byte[] bytesToWrite = com1.Encoding.GetBytes(charsToWrite);
@@ -805,7 +805,7 @@ private void VerifyReadToWithWriteLine(Encoding encoding, string newLine)
                 Random rndGen = new Random(-55);
                 StringBuilder strBldrToWrite;
                 string strExpected;
-                bool isUTF7Encoding = encoding.EncodingName == Encoding.UTF7.EncodingName;
+                bool isUTF7Encoding = IsUTF7Encoding(encoding);
 
                 Debug.WriteLine("Verifying ReadTo with WriteLine encoding={0}, newLine={1}", encoding, newLine);
 
@@ -861,10 +861,10 @@ private string GenRandomNewLine(bool validAscii)
 
         private int GetUTF7EncodingBytes(char[] chars, int index, int count)
         {
-            byte[] bytes = Encoding.UTF7.GetBytes(chars, index, count);
+            byte[] bytes = LegacyUTF7Encoding.GetBytes(chars, index, count);
             int byteCount = bytes.Length;
 
-            while (Encoding.UTF7.GetCharCount(bytes, 0, byteCount) == count)
+            while (LegacyUTF7Encoding.GetCharCount(bytes, 0, byteCount) == count)
             {
                 --byteCount;
             }

diff --git a/src/libraries/System.IO.Ports/tests/SerialPort/Read_char_int_int.cs b/src/libraries/System.IO.Ports/tests/SerialPort/Read_char_int_int.cs
@@ -950,18 +950,9 @@ private void VerifyBytesFollowedByChars(Encoding encoding)
                     Fail("ERROR!!!: Expected to read {0} chars actually read {1}", xmitCharBuffer.Length, numRead);
                 }
 
-                if (encoding.EncodingName == Encoding.UTF7.EncodingName)
+                if (IsUTF7Encoding(encoding))
                 {
-                    //If UTF7Encoding is being used we might leave a - in the stream
-                    if (com1.BytesToRead == xmitByteBuffer.Length + 1)
-                    {
-                        int byteRead;
-
-                        if ('-' != (char)(byteRead = com1.ReadByte()))
-                        {
-                            Fail("Err_29282naie Expected '-' to be left in the stream with UTF7Encoding and read {0}", byteRead);
-                        }
-                    }
+                    Fail("UTF-7 encoding not expected to be passed to this test.");
                 }
 
                 if (xmitByteBuffer.Length != (numRead = com1.Read(rcvByteBuffer, 0, rcvByteBuffer.Length)))

diff --git a/src/libraries/System.IO.Ports/tests/Support/PortsTest.cs b/src/libraries/System.IO.Ports/tests/Support/PortsTest.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Text;
 using System.IO;
 using Legacy.Support;
 using Xunit;
@@ -36,5 +37,20 @@ public static void Fail(string format, params object[] args)
         {
             Assert.True(false, string.Format(format, args));
         }
+
+#pragma warning disable MSLIB0001 // Encoding.UTF7 property is obsolete
+        protected static Encoding LegacyUTF7Encoding => Encoding.UTF7;
+#pragma warning restore MSLIB0001
+
+        /// <summary>
+        /// Returns a value stating whether <paramref name="encoding"/> is UTF-7.
+        /// </summary>
+        /// <remarks>
+        /// This method checks only for the code page 65000.
+        /// </remarks>
+        internal static bool IsUTF7Encoding(Encoding encoding)
+        {
+            return (encoding.CodePage == LegacyUTF7Encoding.CodePage);
+        }
     }
 }
diff --git a/src/libraries/System.Net.Http.Json/tests/UnitTests/TranscodingReadStreamTests.cs b/src/libraries/System.Net.Http.Json/tests/UnitTests/TranscodingReadStreamTests.cs
@@ -232,15 +232,6 @@ public Task ReadAsync_Works_WhenInputIs_Unicode(string message)
             return ReadAsyncTest(sourceEncoding, message);
         }
 
-        [Theory]
-        [MemberData(nameof(ReadAsyncInputLatin), "utf-7")]
-        [MemberData(nameof(ReadAsyncInputUnicode), "utf-7")]
-        public Task ReadAsync_Works_WhenInputIs_UTF7(string message)
-        {
-            Encoding sourceEncoding = Encoding.UTF7;
-            return ReadAsyncTest(sourceEncoding, message);
-        }
-
         [Theory]
         [MemberData(nameof(ReadAsyncInputLatin), "iso-8859-1")]
         public Task ReadAsync_Works_WhenInputIs_WesternEuropeanEncoding(string message)

diff --git a/src/libraries/System.Net.Http.Json/tests/UnitTests/TranscodingWriteStreamTests.cs b/src/libraries/System.Net.Http.Json/tests/UnitTests/TranscodingWriteStreamTests.cs
@@ -42,14 +42,6 @@ public Task WriteAsync_Works_WhenOutputIs_Unicode(string message)
             return WriteAsyncTest(targetEncoding, message);
         }
 
-        [Theory]
-        [MemberData(nameof(WriteAsyncInputLatin))]
-        public Task WriteAsync_Works_WhenOutputIs_UTF7(string message)
-        {
-            Encoding targetEncoding = Encoding.UTF7;
-            return WriteAsyncTest(targetEncoding, message);
-        }
-
         [Theory]
         [MemberData(nameof(WriteAsyncInputLatin))]
         public Task WriteAsync_Works_WhenOutputIs_WesternEuropeanEncoding(string message)

diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml
@@ -9,5 +9,8 @@
     <type fullname="System.Globalization.GlobalizationMode">
       <method signature="System.Boolean get_Invariant()" body="stub" value="true" feature="System.Globalization.Invariant" featurevalue="true" />
     </type>
+    <type fullname="System.LocalAppContextSwitches">
+      <method signature="System.Boolean get_EnableUnsafeUTF7Encoding()" body="stub" value="false" feature="System.Text.Encoding.EnableUnsafeUTF7Encoding" featurevalue="false" />
+    </type>
   </assembly>
 </linker>
diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
@@ -3742,6 +3742,9 @@
   <data name="SwitchExpressionException_UnmatchedValue" xml:space="preserve">
     <value>Unmatched value was {0}.</value>
   </data>
+  <data name="Encoding_UTF7_Disabled" xml:space="preserve">
+    <value>Support for UTF-7 is disabled. See {0} for more information.</value>
+  </data>
   <data name="IDynamicInterfaceCastable_DoesNotImplementRequested" xml:space="preserve">
     <value>Type '{0}' returned by IDynamicInterfaceCastable does not implement the requested interface '{1}'.</value>
   </data>

diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
@@ -455,6 +455,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Object.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\ObjectDisposedException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\ObsoleteAttribute.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Obsoletions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\OperatingSystem.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\OperationCanceledException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\OutOfMemoryException.cs" />

diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs b/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.cs
@@ -8,6 +8,13 @@ namespace System
 {
     internal static partial class LocalAppContextSwitches
     {
+        private static int s_enableUnsafeUTF7Encoding;
+        public static bool EnableUnsafeUTF7Encoding
+        {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            get => GetCachedSwitchValue("System.Text.Encoding.EnableUnsafeUTF7Encoding", ref s_enableUnsafeUTF7Encoding);
+        }
+
         private static int s_enforceJapaneseEraYearRanges;
         public static bool EnforceJapaneseEraYearRanges
         {

diff --git a/src/libraries/System.Private.CoreLib/src/System/Obsoletions.cs b/src/libraries/System.Private.CoreLib/src/System/Obsoletions.cs
@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System
+{
+    internal static class Obsoletions
+    {
+        internal const string SharedUrlFormat = "https://aka.ms/dotnet-warnings/{0}";
+
+        internal const string SystemTextEncodingUTF7Message = "The UTF-7 encoding is insecure and should not be used. Consider using UTF-8 instead.";
+        internal const string SystemTextEncodingUTF7DiagId = "MSLIB0001";
+    }
+}
diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs b/src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs
@@ -5,6 +5,7 @@
 using System.Diagnostics;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
 using System.IO;
 using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
@@ -105,7 +106,7 @@ public abstract partial class Encoding : ICloneable
         internal const int ISO_8859_1 = 28591;    // Latin1
 
         // Special code pages
-        private const int CodePageUTF7 = 65000;
+        internal const int CodePageUTF7 = 65000;
         private const int CodePageUTF8 = 65001;
         private const int CodePageUTF32 = 12000;
         private const int CodePageUTF32BE = 12001;
@@ -214,7 +215,7 @@ public static void RegisterProvider(EncodingProvider provider)
 
         public static Encoding GetEncoding(int codepage)
         {
-            Encoding? result = EncodingProvider.GetEncodingFromProvider(codepage);
+            Encoding? result = FilterDisallowedEncodings(EncodingProvider.GetEncodingFromProvider(codepage));
             if (result != null)
                 return result;
 
@@ -225,7 +226,6 @@ public static Encoding GetEncoding(int codepage)
                 case CodePageBigEndian: return BigEndianUnicode; // 1201
                 case CodePageUTF32: return UTF32;                // 12000
                 case CodePageUTF32BE: return BigEndianUTF32;     // 12001
-                case CodePageUTF7: return UTF7;                  // 65000
                 case CodePageUTF8: return UTF8;                  // 65001
                 case CodePageASCII: return ASCII;                // 20127
                 case ISO_8859_1: return Latin1;                  // 28591
@@ -236,6 +236,27 @@ public static Encoding GetEncoding(int codepage)
                 case CodePageNoThread:                           // 3 CP_THREAD_ACP
                 case CodePageNoSymbol:                           // 42 CP_SYMBOL
                     throw new ArgumentException(SR.Format(SR.Argument_CodepageNotSupported, codepage), nameof(codepage));
+
+                case CodePageUTF7:                               // 65000
+                    {
+                        // Support for UTF-7 is disabled by default. It can be re-enabled by registering a custom
+                        // provider (which early-exits this method before the 'switch' statement) or by using
+                        // AppContext. If support is not enabled, we'll provide a friendly error message stating
+                        // how the developer can re-enable it in their application.
+
+                        if (LocalAppContextSwitches.EnableUnsafeUTF7Encoding)
+                        {
+#pragma warning disable MSLIB0001 // Encoding.UTF7 property getter is obsolete
+                            return UTF7;
+#pragma warning restore MSLIB0001
+                        }
+                        else
+                        {
+                            string moreInfoUrl = string.Format(CultureInfo.InvariantCulture, Obsoletions.SharedUrlFormat, Obsoletions.SystemTextEncodingUTF7DiagId);
+                            string exceptionMessage = SR.Format(SR.Encoding_UTF7_Disabled, moreInfoUrl);
+                            throw new NotSupportedException(exceptionMessage); // matches generic "unknown code page" exception type
+                        }
+                    }
             }
 
             if (codepage < 0 || codepage > 65535)
@@ -250,7 +271,7 @@ public static Encoding GetEncoding(int codepage)
         public static Encoding GetEncoding(int codepage,
             EncoderFallback encoderFallback, DecoderFallback decoderFallback)
         {
-            Encoding? baseEncoding = EncodingProvider.GetEncodingFromProvider(codepage, encoderFallback, decoderFallback);
+            Encoding? baseEncoding = FilterDisallowedEncodings(EncodingProvider.GetEncodingFromProvider(codepage, encoderFallback, decoderFallback));
 
             if (baseEncoding != null)
                 return baseEncoding;
@@ -274,7 +295,7 @@ public static Encoding GetEncoding(string name)
             // add the corresponding item in EncodingTable.
             // Otherwise, the code below will throw exception when trying to call
             // EncodingTable.GetCodePageFromName().
-            return EncodingProvider.GetEncodingFromProvider(name) ??
+            return FilterDisallowedEncodings(EncodingProvider.GetEncodingFromProvider(name)) ??
                 GetEncoding(EncodingTable.GetCodePageFromName(name));
         }
 
@@ -287,10 +308,24 @@ public static Encoding GetEncoding(string name,
             // add the corresponding item in EncodingTable.
             // Otherwise, the code below will throw exception when trying to call
             // EncodingTable.GetCodePageFromName().
-            return EncodingProvider.GetEncodingFromProvider(name, encoderFallback, decoderFallback) ??
+            return FilterDisallowedEncodings(EncodingProvider.GetEncodingFromProvider(name, encoderFallback, decoderFallback)) ??
                 GetEncoding(EncodingTable.GetCodePageFromName(name), encoderFallback, decoderFallback);
         }
 
+        // If the input encoding is forbidden (currently, only UTF-7), returns null.
+        // Otherwise returns the input encoding unchanged.
+        private static Encoding? FilterDisallowedEncodings(Encoding? encoding)
+        {
+            if (LocalAppContextSwitches.EnableUnsafeUTF7Encoding)
+            {
+                return encoding;
+            }
+            else
+            {
+                return (encoding?.CodePage == CodePageUTF7) ? null : encoding;
+            }
+        }
+
         /// <summary>
         /// Get the <see cref="EncodingInfo"/> list from the runtime and all registered encoding providers
         /// </summary>
@@ -1020,6 +1055,7 @@ public virtual string GetString(byte[] bytes, int index, int count) =>
         // Returns an encoding for the UTF-7 format. The returned encoding will be
         // an instance of the UTF7Encoding class.
 
+        [Obsolete(Obsoletions.SystemTextEncodingUTF7Message, DiagnosticId = Obsoletions.SystemTextEncodingUTF7DiagId, UrlFormat = Obsoletions.SharedUrlFormat)]
         public static Encoding UTF7 => UTF7Encoding.s_default;
 
         // Returns an encoding for the UTF-8 format. The returned encoding will be

diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/EncodingTable.cs b/src/libraries/System.Private.CoreLib/src/System/Text/EncodingTable.cs
@@ -106,20 +106,31 @@ private static int InternalGetCodePageFromName(string name)
         // Return a list of all EncodingInfo objects describing all of our encodings
         internal static EncodingInfo[] GetEncodings()
         {
+            // If UTF-7 encoding is not enabled, we adjust the return array length by -1
+            // to account for the skipped EncodingInfo element.
+
             ushort[] mappedCodePages = s_mappedCodePages;
-            EncodingInfo[] arrayEncodingInfo = new EncodingInfo[mappedCodePages.Length];
+            EncodingInfo[] arrayEncodingInfo = new EncodingInfo[(LocalAppContextSwitches.EnableUnsafeUTF7Encoding) ? mappedCodePages.Length : (mappedCodePages.Length - 1)];
             string webNames = s_webNames;
             int[] webNameIndices = s_webNameIndices;
+            int arrayEncodingInfoIdx = 0;
 
             for (int i = 0; i < mappedCodePages.Length; i++)
             {
-                arrayEncodingInfo[i] = new EncodingInfo(
-                    mappedCodePages[i],
+                int codePage = mappedCodePages[i];
+                if (codePage == Encoding.CodePageUTF7 && !LocalAppContextSwitches.EnableUnsafeUTF7Encoding)
+                {
+                    continue; // skip this entry; UTF-7 is disabled
+                }
+
+                arrayEncodingInfo[arrayEncodingInfoIdx++] = new EncodingInfo(
+                    codePage,
                     webNames[webNameIndices[i]..webNameIndices[i + 1]],
-                    GetDisplayName(mappedCodePages[i], i)
+                    GetDisplayName(codePage, i)
                     );
             }
 
+            Debug.Assert(arrayEncodingInfoIdx == arrayEncodingInfo.Length);
             return arrayEncodingInfo;
         }
 
@@ -132,13 +143,28 @@ internal static EncodingInfo[] GetEncodings(Dictionary<int, EncodingInfo> encodi
 
             for (int i = 0; i < mappedCodePages.Length; i++)
             {
-                if (!encodingInfoList.ContainsKey(mappedCodePages[i]))
+                int codePage = mappedCodePages[i];
+                if (!encodingInfoList.ContainsKey(codePage))
                 {
-                    encodingInfoList[mappedCodePages[i]] = new EncodingInfo(mappedCodePages[i], webNames[webNameIndices[i]..webNameIndices[i + 1]],
-                                                                            GetDisplayName(mappedCodePages[i], i));
+                    // If UTF-7 encoding is not enabled, don't add it to the provided dictionary instance.
+                    // Exception: If somebody already registered a custom UTF-7 provider, the dictionary
+                    // will already contain an entry for the UTF-7 code page key, and we'll let it go through.
+
+                    if (codePage != Encoding.CodePageUTF7 || LocalAppContextSwitches.EnableUnsafeUTF7Encoding)
+                    {
+                        encodingInfoList[codePage] = new EncodingInfo(codePage, webNames[webNameIndices[i]..webNameIndices[i + 1]],
+                                                                                GetDisplayName(codePage, i));
+                    }
                 }
             }
 
+            // Just in case a provider registered UTF-7 without the application's consent
+
+            if (!LocalAppContextSwitches.EnableUnsafeUTF7Encoding)
+            {
+                encodingInfoList.Remove(Encoding.CodePageUTF7); // won't throw if doesn't exist
+            }
+
             var result = new EncodingInfo[encodingInfoList.Count];
             int j = 0;
             foreach (KeyValuePair<int, EncodingInfo> pair in encodingInfoList)