Don't early terminate on null for 64bit NR HashCode (dotnet/coreclr#19331)

* Don't early terminate on null for 64bit NR HashCode

* Improved GetNonRandomizedHashCode

* Update message on GetNonRandomizedHashCode

* Consume null terminator rather than special case odd lengths

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

2 files changed, 27 insertions, 44 deletions

diff --git a/src/System.Private.CoreLib/shared/System/Collections/Generic/NonRandomizedStringEqualityComparer.cs b/src/System.Private.CoreLib/shared/System/Collections/Generic/NonRandomizedStringEqualityComparer.cs
index e7efa22b2..91b782061 100644
--- a/src/System.Private.CoreLib/shared/System/Collections/Generic/NonRandomizedStringEqualityComparer.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/Generic/NonRandomizedStringEqualityComparer.cs
@@ -23,7 +23,7 @@ namespace System.Collections.Generic
 
         public sealed override bool Equals(string x, string y) => string.Equals(x, y);
 
-        public sealed override int GetHashCode(string obj) => obj?.GetLegacyNonRandomizedHashCode() ?? 0;
+        public sealed override int GetHashCode(string obj) => obj?.GetNonRandomizedHashCode() ?? 0;
 
         public void GetObjectData(SerializationInfo info, StreamingContext context)
         {
diff --git a/src/System.Private.CoreLib/shared/System/String.Comparison.cs b/src/System.Private.CoreLib/shared/System/String.Comparison.cs
index 3ddc90a50..8ac31796b 100644
--- a/src/System.Private.CoreLib/shared/System/String.Comparison.cs
+++ b/src/System.Private.CoreLib/shared/System/String.Comparison.cs
@@ -753,54 +753,37 @@ namespace System
         // that string.Equals(A, B, C), then they will return the same hash code with this comparison C.
         public int GetHashCode(StringComparison comparisonType) => StringComparer.FromComparison(comparisonType).GetHashCode(this);
 
-        // Use this if and only if you need the hashcode to not change across app domains (e.g. you have an app domain agile
-        // hash table).
-        internal int GetLegacyNonRandomizedHashCode()
+        // Use this if and only if 'Denial of Service' attacks are not a concern (i.e. never used for free-form user input),
+        // or are otherwise mitigated
+        internal unsafe int GetNonRandomizedHashCode()
         {
-            unsafe
+            fixed (char* src = &_firstChar)
             {
-                fixed (char* src = &_firstChar)
-                {
-                    Debug.Assert(src[this.Length] == '\0', "src[this.Length] == '\\0'");
-                    Debug.Assert(((int)src) % 4 == 0, "Managed string should start at 4 bytes boundary");
-#if BIT64
-                    int hash1 = 5381;
-#else // !BIT64 (32)
-                    int hash1 = (5381<<16) + 5381;
-#endif
-                    int hash2 = hash1;
+                Debug.Assert(src[this.Length] == '\0', "src[this.Length] == '\\0'");
+                Debug.Assert(((int)src) % 4 == 0, "Managed string should start at 4 bytes boundary");
 
-#if BIT64
-                    int c;
-                    char* s = src;
-                    while ((c = s[0]) != 0)
-                    {
-                        hash1 = ((hash1 << 5) + hash1) ^ c;
-                        c = s[1];
-                        if (c == 0)
-                            break;
-                        hash2 = ((hash2 << 5) + hash2) ^ c;
-                        s += 2;
-                    }
-#else // !BIT64 (32)
-                    // 32 bit machines.
-                    int* pint = (int *)src;
-                    int len = this.Length;
-                    while (len > 2)
-                    {
-                        hash1 = ((hash1 << 5) + hash1 + (hash1 >> 27)) ^ pint[0];
-                        hash2 = ((hash2 << 5) + hash2 + (hash2 >> 27)) ^ pint[1];
-                        pint += 2;
-                        len  -= 4;
-                    }
+                uint hash1 = (5381 << 16) + 5381;
+                uint hash2 = hash1;
 
-                    if (len > 0)
-                    {
-                        hash1 = ((hash1 << 5) + hash1 + (hash1 >> 27)) ^ pint[0];
-                    }
-#endif
-                    return hash1 + (hash2 * 1566083941);
+                uint* ptr = (uint*)src;
+                int length = this.Length;
+
+                while (length > 2)
+                {
+                    length -= 4;
+                    // Where length is 4n-1 (e.g. 3,7,11,15,19) this additionally consumes the null terminator
+                    hash1 = (((hash1 << 5) | (hash1 >> 27)) + hash1) ^ ptr[0];
+                    hash2 = (((hash2 << 5) | (hash2 >> 27)) + hash2) ^ ptr[1];
+                    ptr += 2;
                 }
+
+                if (length > 0)
+                {
+                    // Where length is 4n-3 (e.g. 1,5,9,13,17) this additionally consumes the null terminator
+                    hash2 = (((hash2 << 5) | (hash2 >> 27)) + hash2) ^ ptr[0];
+                }
+
+                return (int)(hash1 + (hash2 * 1566083941));
             }
         }