Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
Fixing HashCode Seed Initialization on BCL package (#43004)
Browse files Browse the repository at this point in the history
* Fixing HashCode Seed Initialization on BCL package

* Adding Branding and building of HashCode package

* Addressing PR Feedback

* Adding Unit Test

* Address PR feedback

* Adding test for appcontext switch too

* Disable EnsureSeedReturnsEqualValuesWhenAppContextSwitchIsSet on netcoreapp

* Adding retry helper in case the test fails in Linux due to timing

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
  • Loading branch information
joperezr and ericstj authored Nov 12, 2020
1 parent 46c33fd commit fed0fe1
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.Bcl.HashCode/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Import Project="..\Directory.Build.props" />
<PropertyGroup>
<AssemblyVersion>1.0.0.0</AssemblyVersion>
<PackageVersion>1.1.0</PackageVersion>
<PackageVersion>1.1.1</PackageVersion>
<StrongNameKeyId>Open</StrongNameKeyId>
<!-- This assembly should never be placed inbox as it is only for downlevel compatibility. -->
</PropertyGroup>
Expand Down
13 changes: 11 additions & 2 deletions src/Microsoft.Bcl.HashCode/src/Interop.GetRandomBytes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,21 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.InteropServices;
using System.Security.Cryptography;

internal partial class Interop
{
internal static unsafe void GetRandomBytes(byte* buffer, int length)
{
byte[] bytes = Guid.NewGuid().ToByteArray();
buffer = (byte*)BitConverter.ToUInt32(bytes, 0);
if (!LocalAppContextSwitches.UseNonRandomizedHashSeed)
{
using (RandomNumberGenerator rng = RandomNumberGenerator.Create())
{
byte[] tmp = new byte[length];
rng.GetBytes(tmp);
Marshal.Copy(tmp, 0, (IntPtr)buffer, length);
}
}
}
}
18 changes: 18 additions & 0 deletions src/Microsoft.Bcl.HashCode/src/LocalAppContextSwitches.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.Runtime.CompilerServices;

namespace System
{
internal static partial class LocalAppContextSwitches
{
private static int s_useNonRandomizedHashSeed;
public static bool UseNonRandomizedHashSeed
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => GetCachedSwitchValue("Switch.System.Data.UseNonRandomizedHashSeed", ref s_useNonRandomizedHashSeed);
}
}
}
6 changes: 5 additions & 1 deletion src/Microsoft.Bcl.HashCode/src/Microsoft.Bcl.HashCode.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsPartialFacadeAssembly Condition="'$(TargetFramework)' != 'netstandard2.0' AND '$(TargetsNetFx)' != 'true'">true</IsPartialFacadeAssembly>
<OmitResources>$(IsPartialFacadeAssembly)</OmitResources>
<Configurations>net461-Debug;net461-Release;netcoreapp-Debug;netcoreapp-Release;netcoreapp2.1-Debug;netcoreapp2.1-Release;netfx-Debug;netfx-Release;netstandard-Debug;netstandard-Release;netstandard2.1-Debug;netstandard2.1-Release</Configurations>
<Configurations>netstandard-Debug;netstandard-Release;net461-Debug;net461-Release;netcoreapp-Debug;netcoreapp-Release;netcoreapp2.1-Debug;netcoreapp2.1-Release;netfx-Debug;netfx-Release;netstandard2.1-Debug;netstandard2.1-Release</Configurations>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup Condition="'$(IsPartialFacadeAssembly)' != 'true'">
Expand All @@ -13,6 +13,10 @@
</Compile>
<Compile Include="BitOperations.cs" />
<Compile Include="Interop.GetRandomBytes.cs" />
<Compile Include="LocalAppContextSwitches.cs" />
<Compile Include="$(CommonPath)\CoreLib\System\LocalAppContextSwitches.Common.cs">
<Link>Common\CoreLib\System\LocalAppContextSwitches.Common.cs</Link>
</Compile>
<Reference Include="mscorlib" />
<Reference Include="System" />
</ItemGroup>
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.Bcl.HashCode/tests/Configurations.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project DefaultTargets="Build">
<PropertyGroup>
<BuildConfigurations>
netfx;
netcoreapp;
</BuildConfigurations>
</PropertyGroup>
</Project>
68 changes: 68 additions & 0 deletions src/Microsoft.Bcl.HashCode/tests/HashCodeSeedInitializerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// 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 Microsoft.DotNet.RemoteExecutor;
using System;
using System.Reflection;
using Xunit;

namespace Microsoft.Bcl.HashCode.Tests
{
public class HashCodeSeedInitializerTests
{
[Fact]
public void EnsureSeedReturnsDifferentValuesTest()
{
// Adding 5 retries since in Netcoreapp-Linux the seed that is generated is time-dependant so
// this test might fail there. This will ensure we reduce flakiness while still providing
// regression coverage.
RetryHelper.Execute(() =>
{
int FirstSeed = CalculateHashCodeInRemoteProcess(setAppContextSwitch: false);
int SecondSeed = CalculateHashCodeInRemoteProcess(setAppContextSwitch: false);
Assert.NotEqual(FirstSeed, SecondSeed);
});
}

[Fact]
[SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp, "AppContextSwitch is not supported on .NETCore")]
public void EnsureSeedReturnsEqualValuesWhenAppContextSwitchIsSet()
{
int FirstSeed = CalculateHashCodeInRemoteProcess(setAppContextSwitch: true);
int SecondSeed = CalculateHashCodeInRemoteProcess(setAppContextSwitch: true);

Assert.Equal(FirstSeed, SecondSeed);
}

private int CalculateHashCodeInRemoteProcess(bool setAppContextSwitch)
{
RemoteInvokeOptions options = new RemoteInvokeOptions()
{
CheckExitCode = false
};
var executor = (setAppContextSwitch) ?
RemoteExecutor.Invoke(() =>
{
AppContext.SetSwitch("Switch.System.Data.UseNonRandomizedHashSeed", true);
return GetHashCodeSeed();
}, options) :
RemoteExecutor.Invoke(GetHashCodeSeed, options);

int hashedResult = executor.ExitCode;
executor.Dispose();

return hashedResult;

int GetHashCodeSeed()
{
System.HashCode hc = new System.HashCode();
hc.Add(5);
return hc.ToHashCode();
};
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Configurations>netfx-Debug;netfx-Release;netcoreapp-Debug;netcoreapp-Release</Configurations>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
</PropertyGroup>
<ItemGroup>
<Compile Include="HashCodeSeedInitializerTests.cs" />
</ItemGroup>
</Project>
3 changes: 3 additions & 0 deletions src/packages.builds
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
<AdditionalProperties>$(AdditionalProperties)</AdditionalProperties>
</Project>
<!-- add specific builds / pkgproj's here to include in servicing builds -->
<Project Include="$(MSBuildThisFileDirectory)Microsoft.Bcl.HashCode\pkg\Microsoft.Bcl.HashCode.pkgproj">
<AdditionalProperties>$(AdditionalProperties)</AdditionalProperties>
</Project>
</ItemGroup>

<ItemGroup>
Expand Down

0 comments on commit fed0fe1

Please sign in to comment.