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

Limit PNSE by using default comparer in HybridGlobalization #96541

Closed
wants to merge 5 commits into from
Closed
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
41 changes: 23 additions & 18 deletions docs/design/features/globalization-hybrid-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,36 @@ Hybrid has higher priority than sharding or custom modes, described in globaliza

**SortKey**

SortKey equivalent is not available in JS. Following APIs will throw `PlatformNotSupportedException`:

Affected public APIs:
- System.Globalization.CompareInfo.GetSortKey
- System.Globalization.CompareInfo.GetSortKeyLength
- System.Globalization.CompareInfo.GetHashCode
Indirectly affected APIs (the list might not be complete):
- Microsoft.VisualBasic.Collection.Add
- System.Collections.Hashtable.Add
- System.Collections.Hashtable.GetHash
- System.Collections.CaseInsensitiveHashCodeProvider.GetHashCode
- System.Collections.Specialized.NameObjectCollectionBase.BaseAdd
- System.Collections.Specialized.NameValueCollection.Add
- System.Collections.Specialized.NameObjectCollectionBase.BaseGet
- System.Collections.Specialized.NameValueCollection.Get
- System.Collections.Specialized.NameObjectCollectionBase.BaseRemove
- System.Collections.Specialized.NameValueCollection.Remove
- System.Collections.Specialized.OrderedDictionary.Add
- System.Collections.Specialized.NameObjectCollectionBase.BaseSet
- System.Collections.Specialized.NameValueCollection.Set
- System.Data.DataColumnCollection.Add
- System.Collections.Generic.HashSet
- System.Collections.Generic.Dictionary

Some classes use these functions internally. We were able to maintain support for most of them, with changed behavior.

Indirectly affected APIs that throw `PlatformNotSupportedException`:
- System.Collections.CaseInsensitiveHashCodeProvider.GetHashCode (deprecated)
- Microsoft.VisualBasic.Collection.Add (string-keyed collections only)
- System.Data.DataColumnCollection.Add (string-keyed collections only)
- System.Net.Mail.MailAddress.GetHashCode
- System.Xml.Xsl.XslCompiledTransform.Transform

Web API does not have an equivalent, so they throw `PlatformNotSupportedException`.
Indirectly affected APIs with changed behavior (string-keyed collections):

In `Hybrid Globalization` these collections use `StringComparer.Ordinal` as a default comparer:
- System.Collections.Generic.Dictionary
- System.Collections.Generic.HashSet
- System.Collections.Hashtable
You can overwrite this comparer by using constructor with parameter, e.g. [Dictionary<TKey,TValue>(IEqualityComparer<TKey>)](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.-ctor#system-collections-generic-dictionary-2-ctor(system-collections-generic-iequalitycomparer((-0)))). The only supported options for string-keys are: `StringComparer.Ordinal` and `StringComparer.OrdinalIgnoreCase`, other options throw `PlatformNotSupportedException`.

`Hashtable` is non-generic collection that defines the types of keys in the runtime. `Hybrid Globalization` does not support using `string` keys together with `non-string` keys in the same collection.

These collections will throw `PlatformNotSupportedException` if they were constructed without `StringComparer.Ordinal` or `StringComparer.OrdinalIgnoreCase` but use string keys.
- System.Collections.Specialized.NameValueCollection
- System.Collections.Specialized.NameObjectCollectionBase
- System.Collections.Specialized.OrderedDictionary

**Case change**

Expand Down
8 changes: 6 additions & 2 deletions src/libraries/System.Collections.Specialized/tests/Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ public static class Helpers
{
public static MyNameObjectCollection CreateNameObjectCollection(int count)
{
MyNameObjectCollection nameObjectCollection = new MyNameObjectCollection();
MyNameObjectCollection nameObjectCollection = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new MyNameObjectCollection(StringComparer.OrdinalIgnoreCase) :
new MyNameObjectCollection();

for (int i = 0; i < count; i++)
{
Expand All @@ -19,7 +21,9 @@ public static MyNameObjectCollection CreateNameObjectCollection(int count)

public static NameValueCollection CreateNameValueCollection(int count, int start = 0)
{
NameValueCollection nameValueCollection = new NameValueCollection();
NameValueCollection nameValueCollection = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();

for (int i = start; i < start + count; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public MyNameObjectCollection(int capacity, IHashCodeProvider hashProvider, ICom

public bool HasKeys() => BaseHasKeys();

public void Add(string name, Foo value) => BaseAdd(name, value);
public void Add(string name, Foo value) => BaseAdd(name, value);

public void Remove(string name) => BaseRemove(name);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ public void Constructor_Provider_Comparer()
Assert.Equal(0, coll.Count);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void Constructor_Int_Provider_Comparer()
{
#pragma warning disable CS0618 // Type or member is obsolete
MyNameObjectCollection coll = new MyNameObjectCollection(5, CaseInsensitiveHashCodeProvider.DefaultInvariant, CaseInsensitiveComparer.DefaultInvariant);
MyNameObjectCollection coll = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new MyNameObjectCollection(5, StringComparer.OrdinalIgnoreCase) :
new MyNameObjectCollection(5, CaseInsensitiveHashCodeProvider.DefaultInvariant, CaseInsensitiveComparer.DefaultInvariant);
#pragma warning restore CS0618 // Type or member is obsolete
coll.Add("a", new Foo("1"));
int i = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseCopyToTests
{
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0, 0)]
[InlineData(0, 5)]
[InlineData(10, 0)]
[InlineData(10, 5)]
public void CopyTo(int count, int index)
{
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(count);
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(count);
ICollection collection = nameObjectCollection;

string[] copyArray = new string[index + collection.Count + index];
Expand All @@ -39,7 +39,7 @@ public void CopyTo(int count, int index)
Assert.Equal(previousCount, copyArray.Length);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void CopyTo_Invalid(int count)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
{
public class GetAllValuesTests
{
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0, typeof(object))]
[InlineData(0, typeof(Foo))]
[InlineData(10, typeof(object))]
Expand All @@ -33,10 +33,12 @@ private static void VerifyGetAllValues(NameObjectCollectionBase nameObjectCollec
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public static void GetAllValues_Invalid()
{
MyNameObjectCollection nameObjectCollection = new MyNameObjectCollection();
MyNameObjectCollection nameObjectCollection = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new MyNameObjectCollection(StringComparer.OrdinalIgnoreCase) :
new MyNameObjectCollection();
AssertExtensions.Throws<ArgumentNullException>("type", () => nameObjectCollection.GetAllValues(null));

nameObjectCollection.Add("name", new Foo("value"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseGetEnumeratorTests
{
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void GetEnumerator(int count)
Expand All @@ -29,7 +29,7 @@ public void GetEnumerator(int count)
}
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void GetEnumerator_Invalid(int count)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseKeysTests
{
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void Keys_PreservesInstance(int count)
Expand All @@ -16,7 +16,7 @@ public void Keys_PreservesInstance(int count)
Assert.Same(nameObjectCollection.Keys, nameObjectCollection.Keys);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void Keys_GetEnumerator(int count)
Expand All @@ -40,7 +40,7 @@ public void Keys_GetEnumerator(int count)
}
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void Keys_GetEnumerator_Invalid(int count)
Expand Down Expand Up @@ -85,7 +85,7 @@ public void Keys_GetEnumerator_Invalid(int count)
Assert.Throws<InvalidOperationException>(() => enumerator.Reset());
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void Keys_Properties(int count)
Expand All @@ -97,7 +97,7 @@ public void Keys_Properties(int count)
Assert.False(keysCollection.IsSynchronized);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0, 0)]
[InlineData(0, 5)]
[InlineData(10, 0)]
Expand Down Expand Up @@ -145,7 +145,7 @@ private static void Keys_CopyTo_Helper(MyNameObjectCollection nameObjectCollecti
Assert.Equal(previousCount, keysArray.Length);
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void Keys_CopyTo_Invalid(int count)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseReadOnlyTests
{
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void IsReadOnly_Set()
{
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(10);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseRemoveAtTests
{
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void RemoveAt()
{
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(10);
Expand Down Expand Up @@ -59,7 +59,7 @@ public void RemoveAt()
}
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0)]
[InlineData(10)]
public void RemoveAt_InvalidIndex_ThrowsArgumentOutOfRangeException(int count)
Expand All @@ -69,7 +69,7 @@ public void RemoveAt_InvalidIndex_ThrowsArgumentOutOfRangeException(int count)
AssertExtensions.Throws<ArgumentOutOfRangeException>("index", () => nameObjectCollection.RemoveAt(count));
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void RemoveAt_ReadOnly_ThrowsNotSupportedException()
{
MyNameObjectCollection nameObjectCollection = Helpers.CreateNameObjectCollection(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ namespace System.Collections.Specialized.Tests
{
public class NameObjectCollectionBaseSetItemTests
{
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void Set_ObjectAtIndex_ModifiesCollection()
{
var noc = new MyNameObjectCollection();
var noc = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new MyNameObjectCollection(StringComparer.OrdinalIgnoreCase) :
new MyNameObjectCollection();
for (int i = 0; i < 10; i++)
{
var foo1 = new Foo("Value_1");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace System.Collections.Specialized.Tests
{
public class NameValueCollectionAddNameValueCollectionTests
{
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Theory]
[InlineData(0, 0)]
[InlineData(0, 5)]
[InlineData(5, 0)]
Expand Down Expand Up @@ -43,11 +43,15 @@ public void Add(int count1, int count2)
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void Add_ExistingKeys()
{
NameValueCollection nameValueCollection1 = new NameValueCollection();
NameValueCollection nameValueCollection2 = new NameValueCollection();
NameValueCollection nameValueCollection1 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
NameValueCollection nameValueCollection2 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();

string name = "name";
string value1 = "value1";
Expand All @@ -61,11 +65,15 @@ public void Add_ExistingKeys()
Assert.Equal(new string[] { value2, value1 }, nameValueCollection2.GetValues(name));
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void Add_MultipleValues()
{
NameValueCollection nameValueCollection1 = new NameValueCollection();
NameValueCollection nameValueCollection2 = new NameValueCollection();
NameValueCollection nameValueCollection1 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
NameValueCollection nameValueCollection2 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();

string name = "name";
string value1 = "value1";
Expand All @@ -83,9 +91,15 @@ public void Add_MultipleValues()
[Fact]
public void Add_NameValueCollection_WithNullKeys()
{
NameValueCollection nameValueCollection1 = new NameValueCollection();
NameValueCollection nameValueCollection2 = new NameValueCollection();
NameValueCollection nameValueCollection3 = new NameValueCollection();
NameValueCollection nameValueCollection1 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
NameValueCollection nameValueCollection2 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
NameValueCollection nameValueCollection3 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();


string nullKeyValue1 = "value";
Expand All @@ -104,11 +118,15 @@ public void Add_NameValueCollection_WithNullKeys()
Assert.Equal(nullKeyValue1 + "," + nullKeyValue2, nameValueCollection3[null]);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
[Fact]
public void Add_NameValueCollection_WithNullValues()
{
NameValueCollection nameValueCollection1 = new NameValueCollection();
NameValueCollection nameValueCollection2 = new NameValueCollection();
NameValueCollection nameValueCollection1 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
NameValueCollection nameValueCollection2 = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();

string nullValueName = "name";
nameValueCollection1.Add(nullValueName, null);
Expand All @@ -122,7 +140,10 @@ public void Add_NameValueCollection_WithNullValues()
[Fact]
public void Add_NullNameValueCollection_ThrowsArgumentNullException()
{
AssertExtensions.Throws<ArgumentNullException>("c", () => new NameValueCollection().Add(null));
NameValueCollection nameValueCollection = PlatformDetection.IsHybridGlobalizationOnBrowser ?
new NameValueCollection(StringComparer.OrdinalIgnoreCase) :
new NameValueCollection();
AssertExtensions.Throws<ArgumentNullException>("c", () => nameValueCollection.Add(null));
}
}
}
Loading
Loading