Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obsolete Encoding.UTF7 property and UTF7Encoding ctors #37535

Merged
merged 14 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion docs/project/list-of-obsoletions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.IO.Ports/tests/SerialPort/Encoding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/System.IO.Ports/tests/SerialPort/ReadTo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
16 changes: 16 additions & 0 deletions src/libraries/System.IO.Ports/tests/Support/PortsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,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>
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,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" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
14 changes: 14 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Obsoletions.cs
Original file line number Diff line number Diff line change
@@ -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";
}
}
48 changes: 42 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Text/Encoding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

What custom provider?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you call Encoding.RegisterProvider, you can override the return value of Encoding.GetEncoding("utf-7"), Encoding.GetEncoding("utf-8"), and any other encoding instance. If you register a custom provider for UTF-7, we never query the AppContext switch because this switch statement never get hit.

Registering a custom provider doesn't change the return value of the built-in accelerators (like Encoding.UTF7 or Encoding.UTF8) since they're hardcoded to always use the built-in implementation.

Copy link
Member

@stephentoub stephentoub Jun 8, 2020

Choose a reason for hiding this comment

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

Right. It's the "re-enabled" lingo that confuses me. There's no way for someone to get at our built-in Encoding to "re-enable" it via Encoding.RegisterProvider, is there? They've have to implement one from scratch, no?

// 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)
Expand All @@ -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;
Expand All @@ -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));
}

Expand All @@ -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>
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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)
Expand Down
Loading