Remove unneeded type SecureStringHasher (#31707)
authorLevi Broderick <GrabYourPitchforks@users.noreply.github.com>
Tue, 4 Feb 2020 18:43:20 +0000 (10:43 -0800)
committerGitHub <noreply@github.com>
Tue, 4 Feb 2020 18:43:20 +0000 (10:43 -0800)
string.GetHashCode is already randomized on .NET Core, no need for a custom hashing routine

src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
src/libraries/System.Private.Xml/src/System/Xml/BinaryXml/XmlBinaryReader.cs
src/libraries/System.Private.Xml/src/System/Xml/Core/SecureStringHasher.cs [deleted file]
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlTextWriter.cs
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlWellFormedWriter.cs
src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs
src/libraries/System.Private.Xml/tests/Misc/RandomizedHashing.cs [deleted file]
src/libraries/System.Private.Xml/tests/Misc/System.Xml.Misc.Tests.csproj

index d7229d0..64face0 100644 (file)
     <Compile Include="System\Xml\Core\ReadContentAsBinaryHelperAsync.cs" />
     <Compile Include="System\Xml\Core\ReadOnlyTernaryTree.cs" />
     <Compile Include="System\Xml\Core\ReadState.cs" />
-    <Compile Include="System\Xml\Core\SecureStringHasher.cs" />
     <Compile Include="System\Xml\Core\ValidatingReaderNodeData.cs" />
     <Compile Include="System\Xml\Core\ValidationType.cs" />
     <Compile Include="System\Xml\Core\WhitespaceHandling.cs" />
index dcd0bd1..e747239 100644 (file)
@@ -98,9 +98,10 @@ namespace System.Xml
                 return this.prefix.GetHashCode() ^ this.localname.GetHashCode();
             }
 
-            public int GetNSHashCode(SecureStringHasher hasher)
+            public int GetNSHashCode()
             {
-                return hasher.GetHashCode(this.namespaceUri) ^ hasher.GetHashCode(this.localname);
+                // ValueTuple uses HashCode.Combine with the randomized string.GetHashCode as inputs
+                return (namespaceUri, localname).GetHashCode();
             }
 
 
@@ -189,11 +190,11 @@ namespace System.Xml
                 namespaceUri = this.name.namespaceUri;
             }
 
-            public int GetLocalnameAndNamespaceUriAndHash(SecureStringHasher hasher, out string localname, out string namespaceUri)
+            public int GetLocalnameAndNamespaceUriAndHash(out string localname, out string namespaceUri)
             {
                 localname = this.name.localname;
                 namespaceUri = this.name.namespaceUri;
-                return this.hashCode = this.name.GetNSHashCode(hasher);
+                return this.hashCode = this.name.GetNSHashCode();
             }
 
             public bool MatchNS(string localname, string namespaceUri)
@@ -332,7 +333,6 @@ namespace System.Xml
         private readonly bool _ignoreComments;
         private readonly DtdProcessing _dtdProcessing;
 
-        private readonly SecureStringHasher _hasher;
         private XmlCharType _xmlCharType;
         private readonly Encoding _unicode;
 
@@ -368,8 +368,7 @@ namespace System.Xml
             _qnameOther.Clear();
             _qnameElement.Clear();
             _xmlspacePreserve = false;
-            _hasher = new SecureStringHasher();
-            _namespaces = new Dictionary<string, NamespaceDecl>(_hasher);
+            _namespaces = new Dictionary<string, NamespaceDecl>();
             AddInitNamespace(string.Empty, string.Empty);
             AddInitNamespace(_xml, _xnt.Add(XmlReservedNs.NsXml));
             AddInitNamespace(_xmlns, _nsxmlns);
@@ -2721,7 +2720,7 @@ namespace System.Xml
             for (int i = 0; i < _attrCount; i++)
             {
                 string localname, namespaceUri;
-                int hash = _attributes[i].GetLocalnameAndNamespaceUriAndHash(_hasher, out localname, out namespaceUri);
+                int hash = _attributes[i].GetLocalnameAndNamespaceUriAndHash(out localname, out namespaceUri);
                 int index = hash & (tblSize - 1);
                 int next = _attrHashTbl[index];
                 _attrHashTbl[index] = i + 1;
diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Core/SecureStringHasher.cs b/src/libraries/System.Private.Xml/src/System/Xml/Core/SecureStringHasher.cs
deleted file mode 100644 (file)
index 8bc643e..0000000
+++ /dev/null
@@ -1,31 +0,0 @@
-// 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.
-
-// Warning: Do not port this code to Desktop!
-
-// Desktop has its own implementation.
-// The difference between those implementations is caused by randomized hashing feature.
-// On Desktop randomized hashing for strings is disabled by default (it is on only for collections)
-// On CoreClr platforms randomized hashing is enabled by default meaning that we can use standard
-// string.GetHashCode implementation.
-
-using System;
-using System.Collections.Generic;
-
-namespace System.Xml
-{
-    // The SecureStringHasher implements IEqualityComparer for strings and therefore can be used in generic IDictionary.
-    internal class SecureStringHasher : IEqualityComparer<string>
-    {
-        public bool Equals(string x, string y)
-        {
-            return string.Equals(x, y, StringComparison.Ordinal);
-        }
-
-        public int GetHashCode(string key)
-        {
-            return key.GetHashCode();
-        }
-    }
-}
\ No newline at end of file
index 555afa9..98e5a3a 100644 (file)
@@ -1519,7 +1519,7 @@ namespace System.Xml
             else if (nsIndex == MaxNamespacesWalkCount)
             {
                 // add all
-                _nsHashtable = new Dictionary<string, int>(new SecureStringHasher());
+                _nsHashtable = new Dictionary<string, int>();
                 for (int i = 0; i <= nsIndex; i++)
                 {
                     AddToNamespaceHashtable(i);
index f3742b4..24fcb08 100644 (file)
@@ -66,10 +66,6 @@ namespace System.Xml
         // char type tables
         private XmlCharType _xmlCharType = XmlCharType.Instance;
 
-        // hash randomizer
-        private readonly SecureStringHasher _hasher;
-
-
         //
         // Constants
         //
@@ -288,8 +284,6 @@ namespace System.Xml
             _elemTop = 0;
 
             _attrStack = new AttrName[AttributeArrayInitialSize];
-
-            _hasher = new SecureStringHasher();
         }
 
         //
@@ -1848,7 +1842,7 @@ namespace System.Xml
             else if (_nsTop == MaxNamespacesWalkCount)
             {
                 // add all
-                _nsHashtable = new Dictionary<string, int>(_hasher);
+                _nsHashtable = new Dictionary<string, int>();
                 for (int i = 0; i <= _nsTop; i++)
                 {
                     AddToNamespaceHashtable(i);
@@ -2229,7 +2223,7 @@ namespace System.Xml
                 {
                     if (_attrHashTable == null)
                     {
-                        _attrHashTable = new Dictionary<string, int>(_hasher);
+                        _attrHashTable = new Dictionary<string, int>();
                     }
                     Debug.Assert(_attrHashTable.Count == 0);
                     for (int i = 0; i < top; i++)
index f080dfe..ce05faa 100644 (file)
@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// Warning: Do not port implementation of GetHashCode to Desktop. See notes in SecureStringHasher
-
 using System.Collections;
 using System.Diagnostics;
 
diff --git a/src/libraries/System.Private.Xml/tests/Misc/RandomizedHashing.cs b/src/libraries/System.Private.Xml/tests/Misc/RandomizedHashing.cs
deleted file mode 100644 (file)
index 12b49dc..0000000
+++ /dev/null
@@ -1,92 +0,0 @@
-// 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.
-
-using System;
-using System.Xml;
-using System.Linq;
-using Xunit;
-
-namespace System.Xml.Tests
-{
-    // These tests will fail on Desktop as randomized hashing is disabled there.
-    // Dictionary<XmlQualifiedName, ...> and Dictionary<SecureStringHasher, ...>
-    //   should use randomized hashing even on Desktop
-    public class XmlMiscTests
-    {
-        // Hashes of string for .NET Framework 2.0-4.6.2
-        // if that algorithm will be used the test will fail
-        private static readonly string[] _strings = new string[] { "foo", "asdf", "randhashing" };
-        private static readonly Tuple<int, int, int>[] _knownNonRandomizedHashesOfStrings = new Tuple<int, int, int>[] {
-            new Tuple<int, int, int>(-1788410455, -352695115, 1368290095),
-            new Tuple<int, int, int>(1502598398, -1233826608, 1092416431)
-        };
-
-        // Few generated hash collisions for System.Text.StringOrCharArray.GetHashCode
-        // These should have the same hash when randomized hashing is disabled on CoreCLR
-        // The algorithm used for generation is as following:
-        // - If we take closer look at the loop, we can note that hash code is a linear combination of partial hashes of every odd and every even character
-        // - That means that for example string ABCDEF hash(ABCDEF) = partial_hash(ACE) + M*partial_hash(BDF)
-        // - That significantly simplifies search as collision will always occur when parial_hash(ACE)=partial_hash(BDF)
-        // - ACE and BDF pairs were searched brute force which really quickly gave results
-        private static readonly Tuple<string, string>[] _collidingStringsPairs = new Tuple<string, string>[]
-        {
-            new Tuple<string, string>("01`@a@", "10@`@a"),
-            new Tuple<string, string>("01Ps6t", "10sPt6"),
-            new Tuple<string, string>("AFdBbC", "FABdCb"),
-            new Tuple<string, string>("01xXUt", "10XxtU"),
-            new Tuple<string, string>("01Ps6t", "10sPt6"),
-            new Tuple<string, string>("12;E7J", "21E;J7"),
-            new Tuple<string, string>("024u3P", "20u4P3"),
-            new Tuple<string, string>("23`B8[", "32B`[8"),
-            new Tuple<string, string>("039X3q", "30X9q3"),
-            new Tuple<string, string>("23MoOl", "32oMlO"),
-            new Tuple<string, string>("45QOd;", "54OQ;d"),
-            new Tuple<string, string>("45HjrQ", "54jHQr"),
-            new Tuple<string, string>("46v:r<", "64:v<r")
-        };
-
-        [Fact]
-        public void StringsDontHashToAnyKnownNonRandomizedSets()
-        {
-            var setOfHashes = new Tuple<int, int, int>(_strings[0].GetHashCode(), _strings[1].GetHashCode(), _strings[2].GetHashCode());
-            Assert.DoesNotContain(setOfHashes, _knownNonRandomizedHashesOfStrings);
-        }
-
-        [Fact]
-        public void StringsDoNotUseAlgorithmSimilarToCoreClrWhenRandomizedHashingIsDisabled()
-        {
-            // Even though GetHashCode gives different results on .NET 4.6 and CoreCLR with disabled
-            // hash randomization those pairs are causing collisions on both platforms as they meet
-            // certain properties causing hash collisions
-
-            // Checking few different hash codes - if at least one is different then
-            // CoreCLR implementation is not being used
-            foreach (var collPair in _collidingStringsPairs)
-            {
-                if (collPair.Item1.GetHashCode() != collPair.Item2.GetHashCode())
-                {
-                    return;
-                }
-            }
-
-            Assert.True(false);
-        }
-
-        [Fact]
-        public void XmlQualifiedNameUsesStringGetHashCode()
-        {
-            Assert.Equal("foo".GetHashCode(), new XmlQualifiedName("foo").GetHashCode());
-            Assert.Equal("asdf".GetHashCode(), new XmlQualifiedName("asdf").GetHashCode());
-            Assert.Equal("randhashing".GetHashCode(), new XmlQualifiedName("randhashing").GetHashCode());
-        }
-
-        [Fact]
-        public void SecureStringHasherUsesStringGetHashCode()
-        {
-            Assert.Equal("foo".GetHashCode(), new SecureStringHasher().GetHashCode("foo"));
-            Assert.Equal("asdf".GetHashCode(), new SecureStringHasher().GetHashCode("asdf"));
-            Assert.Equal("randhashing".GetHashCode(), new SecureStringHasher().GetHashCode("randhashing"));
-        }
-    }
-}
index fcc5035..e2950df 100644 (file)
@@ -4,8 +4,6 @@
     <Configurations>$(NetCoreAppCurrent)-Debug;$(NetCoreAppCurrent)-Release</Configurations>
   </PropertyGroup>
   <ItemGroup>
-    <Compile Include="RandomizedHashing.cs" />
-    <Compile Include="..\..\src\System\Xml\Core\SecureStringHasher.cs" />
     <Compile Include="XmlUrlResolverTests.cs" />
   </ItemGroup>
 </Project>
\ No newline at end of file