From 8c8656bb5163e695ddf6ea5d2b0086771670da7e Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Wed, 13 Feb 2019 13:13:23 -0800 Subject: [PATCH 1/9] replace SHA1 to SHA256 --- .../Portable/Workspace/Solution/Checksum.cs | 105 ++++++++++-------- .../Workspace/Solution/Checksum_Factory.cs | 45 ++++---- .../ServiceHub/Shared/RoslynJsonConverter.cs | 2 +- 3 files changed, 83 insertions(+), 69 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 7b3d2fc0394c..6c72b1b546b6 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -16,65 +16,78 @@ namespace Microsoft.CodeAnalysis internal sealed partial class Checksum : IObjectWritable, IEquatable<Checksum> { /// <summary> - /// The intended size of the <see cref="Sha1Hash"/> structure. Some runtime environments are known to deviate - /// from this size, so the implementation code should remain functionally correct even when the structure size - /// is greater than this value. + /// The intended size of the <see cref="HashData"/> structure. /// </summary> - /// <seealso href="https://bugzilla.xamarin.com/show_bug.cgi?id=60298">LayoutKind.Explicit, Size = 12 ignored with 64bit alignment</seealso> - /// <seealso href="https://github.com/dotnet/roslyn/issues/23722">Checksum throws on Mono 64-bit</seealso> - private const int Sha1HashSize = 20; + private const int HashSize = 20; - public static readonly Checksum Null = new Checksum(Array.Empty<byte>()); + public static readonly Checksum Null = new Checksum((HashData)default); - private Sha1Hash _checkSum; + private HashData _checksum; - public unsafe Checksum(byte[] checksum) + public static unsafe Checksum From(byte[] checksum, bool truncate = false) { + Validate(checksum.Length, truncate); + if (checksum.Length == 0) { - _checkSum = default; - return; - } - else if (checksum.Length != Sha1HashSize) - { - throw new ArgumentException($"{nameof(checksum)} must be a SHA-1 hash", nameof(checksum)); + return Null; } fixed (byte* data = checksum) { - // Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize. - _checkSum = Sha1Hash.FromPointer((Sha1Hash*)data); + // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. + return new Checksum(HashData.FromPointer((HashData*)data)); } } - public unsafe Checksum(ImmutableArray<byte> checksum) + public static unsafe Checksum From(ImmutableArray<byte> checksum, bool truncate = false) { + Validate(checksum.Length, truncate); + if (checksum.Length == 0) { - _checkSum = default; - return; - } - else if (checksum.Length != Sha1HashSize) - { - throw new ArgumentException($"{nameof(checksum)} must be a SHA-1 hash", nameof(checksum)); + return Null; } using (var pooled = SharedPools.ByteArray.GetPooledObject()) { var bytes = pooled.Object; - checksum.CopyTo(bytes); + checksum.CopyTo(sourceIndex: 0, bytes, destinationIndex: 0, length: HashSize); fixed (byte* data = bytes) { - // Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize. - _checkSum = Sha1Hash.FromPointer((Sha1Hash*)data); + // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. + return new Checksum(HashData.FromPointer((HashData*)data)); + } + } + } + + private static void Validate(int length, bool truncate) + { + if (length == 0) + { + return; + } + + if (truncate) + { + if (length < HashSize) + { + throw new ArgumentException($"checksum must be bigger than the hash size: {HashSize}", "checksum"); } + + return; + } + + if (length != HashSize) + { + throw new ArgumentException($"checksum must be the hash size: {HashSize}", "checksum"); } } - private Checksum(Sha1Hash hash) + private Checksum(HashData hash) { - _checkSum = hash; + _checksum = hash; } public bool Equals(Checksum other) @@ -84,24 +97,24 @@ public bool Equals(Checksum other) return false; } - return _checkSum == other._checkSum; + return _checksum == other._checksum; } public override bool Equals(object obj) => Equals(obj as Checksum); public override int GetHashCode() - => _checkSum.GetHashCode(); + => _checksum.GetHashCode(); public override unsafe string ToString() { - var data = new byte[sizeof(Sha1Hash)]; + var data = new byte[sizeof(HashData)]; fixed (byte* dataPtr = data) { - *(Sha1Hash*)dataPtr = _checkSum; + *(HashData*)dataPtr = _checksum; } - return Convert.ToBase64String(data, 0, Sha1HashSize); + return Convert.ToBase64String(data, 0, HashSize); } public static bool operator ==(Checksum left, Checksum right) @@ -117,10 +130,10 @@ public override unsafe string ToString() bool IObjectWritable.ShouldReuseInSerialization => true; public void WriteTo(ObjectWriter writer) - => _checkSum.WriteTo(writer); + => _checksum.WriteTo(writer); public static Checksum ReadFrom(ObjectReader reader) - => new Checksum(Sha1Hash.ReadFrom(reader)); + => new Checksum(HashData.ReadFrom(reader)); public static string GetChecksumLogInfo(Checksum checksum) { @@ -136,8 +149,8 @@ public static string GetChecksumsLogInfo(IEnumerable<Checksum> checksums) /// This structure stores the 20-byte SHA 1 hash as an inline value rather than requiring the use of /// <c>byte[]</c>. /// </summary> - [StructLayout(LayoutKind.Explicit, Size = Sha1HashSize)] - private struct Sha1Hash : IEquatable<Sha1Hash> + [StructLayout(LayoutKind.Explicit, Size = HashSize)] + private struct HashData : IEquatable<HashData> { [FieldOffset(0)] private long Data1; @@ -148,10 +161,10 @@ private struct Sha1Hash : IEquatable<Sha1Hash> [FieldOffset(16)] private int Data3; - public static bool operator ==(Sha1Hash x, Sha1Hash y) + public static bool operator ==(HashData x, HashData y) => x.Equals(y); - public static bool operator !=(Sha1Hash x, Sha1Hash y) + public static bool operator !=(HashData x, HashData y) => !x.Equals(y); public void WriteTo(ObjectWriter writer) @@ -161,18 +174,18 @@ public void WriteTo(ObjectWriter writer) writer.WriteInt32(Data3); } - public static unsafe Sha1Hash FromPointer(Sha1Hash* hash) + public static unsafe HashData FromPointer(HashData* hash) { - Sha1Hash result = default; + HashData result = default; result.Data1 = hash->Data1; result.Data2 = hash->Data2; result.Data3 = hash->Data3; return result; } - public static Sha1Hash ReadFrom(ObjectReader reader) + public static HashData ReadFrom(ObjectReader reader) { - Sha1Hash result = default; + HashData result = default; result.Data1 = reader.ReadInt64(); result.Data2 = reader.ReadInt64(); result.Data3 = reader.ReadInt32(); @@ -186,9 +199,9 @@ public override int GetHashCode() } public override bool Equals(object obj) - => obj is Sha1Hash other && Equals(other); + => obj is HashData other && Equals(other); - public bool Equals(Sha1Hash other) + public bool Equals(HashData other) { return Data1 == other.Data1 && Data2 == other.Data2 diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs index c4be6ea88a08..bad091955880 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs @@ -16,33 +16,34 @@ internal partial class Checksum { public static Checksum Create(Stream stream) { - using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1)) + using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA256)) { - return ComputeChecksum(stream, hash); - } - } - - private static Checksum ComputeChecksum(Stream stream, IncrementalHash hash) - { - using (var pooledBuffer = SharedPools.ByteArray.GetPooledObject()) - { - stream.Seek(0, SeekOrigin.Begin); - - var buffer = pooledBuffer.Object; - var bufferLength = buffer.Length; - int bytesRead; - do + using (var pooledBuffer = SharedPools.ByteArray.GetPooledObject()) { - bytesRead = stream.Read(buffer, 0, bufferLength); - if (bytesRead > 0) + stream.Seek(0, SeekOrigin.Begin); + + var buffer = pooledBuffer.Object; + var bufferLength = buffer.Length; + int bytesRead; + do { - hash.AppendData(buffer, 0, bytesRead); + bytesRead = stream.Read(buffer, 0, bufferLength); + if (bytesRead > 0) + { + hash.AppendData(buffer, 0, bytesRead); + } } - } - while (bytesRead > 0); + while (bytesRead > 0); - var bytes = hash.GetHashAndReset(); - return new Checksum(bytes); + var bytes = hash.GetHashAndReset(); + + // from SHA2, take only first HashSize for checksum + // this is recommended way to replace SHA1 with SHA256 maintaining same + // memory footprint and collision resistance + // calculating SHA256 is slightly slower than SHA1 but not as much. usually + // about 1.1~1.2 slower. + return Checksum.From(bytes, truncate: true); + } } } diff --git a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs index 9a5eb2729afe..707fe1fd4274 100644 --- a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs +++ b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs @@ -175,7 +175,7 @@ private class ChecksumJsonConverter : BaseJsonConverter<Checksum> protected override Checksum ReadValue(JsonReader reader, JsonSerializer serializer) { var value = (string)reader.Value; - return value == null ? null : new Checksum(Convert.FromBase64String(value)); + return value == null ? null : Checksum.From(Convert.FromBase64String(value)); } protected override void WriteValue(JsonWriter writer, Checksum value, JsonSerializer serializer) From cb58a07c98a9c0bb8508fc85ecd77e8c9e4abede Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Wed, 13 Feb 2019 13:18:10 -0800 Subject: [PATCH 2/9] tweak --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 6c72b1b546b6..be842fdf56f4 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -20,7 +20,7 @@ internal sealed partial class Checksum : IObjectWritable, IEquatable<Checksum> /// </summary> private const int HashSize = 20; - public static readonly Checksum Null = new Checksum((HashData)default); + public static readonly Checksum Null = new Checksum(default); private HashData _checksum; @@ -73,7 +73,7 @@ private static void Validate(int length, bool truncate) { if (length < HashSize) { - throw new ArgumentException($"checksum must be bigger than the hash size: {HashSize}", "checksum"); + throw new ArgumentException($"checksum must be equal or bigger than the hash size: {HashSize}", "checksum"); } return; From 980eac0a73f9b6050b204a311510e7e5c7677333 Mon Sep 17 00:00:00 2001 From: Joey Robichaud <joseph.robichaud@microsoft.com> Date: Thu, 14 Feb 2019 11:26:49 -0800 Subject: [PATCH 3/9] Update src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs Co-Authored-By: heejaechang <heejaechang@outlook.com> --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index be842fdf56f4..93f9672d55f9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -35,7 +35,7 @@ public static unsafe Checksum From(byte[] checksum, bool truncate = false) fixed (byte* data = checksum) { - // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. + // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than HashSize. return new Checksum(HashData.FromPointer((HashData*)data)); } } From 25ea34efabf9b708d628da5e355c151135998b36 Mon Sep 17 00:00:00 2001 From: Joey Robichaud <joseph.robichaud@microsoft.com> Date: Thu, 14 Feb 2019 11:27:04 -0800 Subject: [PATCH 4/9] Update src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs Co-Authored-By: heejaechang <heejaechang@outlook.com> --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 93f9672d55f9..684d061cba70 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -56,7 +56,7 @@ public static unsafe Checksum From(ImmutableArray<byte> checksum, bool truncate fixed (byte* data = bytes) { - // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. + // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than HashSize. return new Checksum(HashData.FromPointer((HashData*)data)); } } From 8624ff11f50a3f9be6b52b93e468bd7a62f04690 Mon Sep 17 00:00:00 2001 From: Joey Robichaud <joseph.robichaud@microsoft.com> Date: Thu, 14 Feb 2019 11:27:14 -0800 Subject: [PATCH 5/9] Update src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs Co-Authored-By: heejaechang <heejaechang@outlook.com> --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 684d061cba70..a8655b345f3e 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -146,7 +146,7 @@ public static string GetChecksumsLogInfo(IEnumerable<Checksum> checksums) } /// <summary> - /// This structure stores the 20-byte SHA 1 hash as an inline value rather than requiring the use of + /// This structure stores the 20-byte SHA256 hash as an inline value rather than requiring the use of /// <c>byte[]</c>. /// </summary> [StructLayout(LayoutKind.Explicit, Size = HashSize)] From 97641e8e88cc14bd5afbb9e597a7c04f7cedb52c Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Thu, 14 Feb 2019 17:01:56 -0800 Subject: [PATCH 6/9] tweaks based on PR feedback --- .../Portable/Workspace/Solution/Checksum.cs | 61 +++++++++++-------- .../Workspace/Solution/Checksum_Factory.cs | 2 +- .../ServiceHub/Shared/RoslynJsonConverter.cs | 2 +- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index be842fdf56f4..7f080a821da8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -22,66 +22,73 @@ internal sealed partial class Checksum : IObjectWritable, IEquatable<Checksum> public static readonly Checksum Null = new Checksum(default); - private HashData _checksum; + private readonly HashData _checksum; - public static unsafe Checksum From(byte[] checksum, bool truncate = false) + /// <summary> + /// Create Checksum from given byte array. if byte array is bigger than + /// <see cref="HashSize"/>, it will be truncated to the size + /// </summary> + public static Checksum From(byte[] checksum) { - Validate(checksum.Length, truncate); - if (checksum.Length == 0) { return Null; } - fixed (byte* data = checksum) + if (checksum.Length < HashSize) { - // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. - return new Checksum(HashData.FromPointer((HashData*)data)); + throw new ArgumentException($"checksum must be equal or bigger than the hash size: {HashSize}", nameof(checksum)); } + + return FromWorker(checksum); } - public static unsafe Checksum From(ImmutableArray<byte> checksum, bool truncate = false) + /// <summary> + /// Create Checksum from given byte array. if byte array is bigger than + /// <see cref="HashSize"/>, it will be truncated to the size + /// </summary> + public static unsafe Checksum From(ImmutableArray<byte> checksum) { - Validate(checksum.Length, truncate); - if (checksum.Length == 0) { return Null; } + if (checksum.Length < HashSize) + { + throw new ArgumentException($"{nameof(checksum)} must be equal or bigger than the hash size: {HashSize}", nameof(checksum)); + } + using (var pooled = SharedPools.ByteArray.GetPooledObject()) { var bytes = pooled.Object; checksum.CopyTo(sourceIndex: 0, bytes, destinationIndex: 0, length: HashSize); - fixed (byte* data = bytes) - { - // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. - return new Checksum(HashData.FromPointer((HashData*)data)); - } + return FromWorker(bytes); } } - private static void Validate(int length, bool truncate) + public static Checksum FromSerialized(byte[] checksum) { - if (length == 0) + if (checksum.Length == 0) { - return; + return Null; } - if (truncate) + if (checksum.Length != HashSize) { - if (length < HashSize) - { - throw new ArgumentException($"checksum must be equal or bigger than the hash size: {HashSize}", "checksum"); - } - - return; + throw new ArgumentException($"{nameof(checksum)} must be equal to the hash size: {HashSize}", nameof(checksum)); } - if (length != HashSize) + return FromWorker(checksum); + } + + private static unsafe Checksum FromWorker(byte[] checksum) + { + fixed (byte* data = checksum) { - throw new ArgumentException($"checksum must be the hash size: {HashSize}", "checksum"); + // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than Sha1HashSize. + return new Checksum(HashData.FromPointer((HashData*)data)); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs index bad091955880..f4bd20e37f7a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs @@ -42,7 +42,7 @@ public static Checksum Create(Stream stream) // memory footprint and collision resistance // calculating SHA256 is slightly slower than SHA1 but not as much. usually // about 1.1~1.2 slower. - return Checksum.From(bytes, truncate: true); + return Checksum.From(bytes); } } } diff --git a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs index 707fe1fd4274..53cd382395d9 100644 --- a/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs +++ b/src/Workspaces/Remote/ServiceHub/Shared/RoslynJsonConverter.cs @@ -175,7 +175,7 @@ private class ChecksumJsonConverter : BaseJsonConverter<Checksum> protected override Checksum ReadValue(JsonReader reader, JsonSerializer serializer) { var value = (string)reader.Value; - return value == null ? null : Checksum.From(Convert.FromBase64String(value)); + return value == null ? null : Checksum.FromSerialized(Convert.FromBase64String(value)); } protected override void WriteValue(JsonWriter writer, Checksum value, JsonSerializer serializer) From 0e1d6e5c3738e5ad6309883a3d8594b978e3f8ca Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Thu, 14 Feb 2019 17:22:08 -0800 Subject: [PATCH 7/9] removed unnecessary unsafe --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 91f305fb481a..790cea00fba4 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -47,7 +47,7 @@ public static Checksum From(byte[] checksum) /// Create Checksum from given byte array. if byte array is bigger than /// <see cref="HashSize"/>, it will be truncated to the size /// </summary> - public static unsafe Checksum From(ImmutableArray<byte> checksum) + public static Checksum From(ImmutableArray<byte> checksum) { if (checksum.Length == 0) { @@ -153,7 +153,7 @@ public static string GetChecksumsLogInfo(IEnumerable<Checksum> checksums) } /// <summary> - /// This structure stores the 20-byte SHA256 hash as an inline value rather than requiring the use of + /// This structure stores the 20-byte SHA256 hash as an inline value rather than requiring the use of /// <c>byte[]</c>. /// </summary> [StructLayout(LayoutKind.Explicit, Size = HashSize)] From 33a29be7cd1ef3869219ea8ac58d1ceb36ee88e6 Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Fri, 15 Feb 2019 10:51:53 -0800 Subject: [PATCH 8/9] updated comment --- .../Workspace/Solution/Checksum_Factory.cs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs index f4bd20e37f7a..d56265f55a9e 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs @@ -16,7 +16,7 @@ internal partial class Checksum { public static Checksum Create(Stream stream) { - using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA256)) + using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1)) { using (var pooledBuffer = SharedPools.ByteArray.GetPooledObject()) { @@ -37,11 +37,19 @@ public static Checksum Create(Stream stream) var bytes = hash.GetHashAndReset(); - // from SHA2, take only first HashSize for checksum - // this is recommended way to replace SHA1 with SHA256 maintaining same - // memory footprint and collision resistance - // calculating SHA256 is slightly slower than SHA1 but not as much. usually - // about 1.1~1.2 slower. + // if bytes array is bigger than certain size, checksum + // will truncate it to predetermined size. for more detail, + // see the Checksum type + // + // the truncation can happen since different hash algorithm or + // same algorithm on different platform can have different hash size + // which might be bigger than the Checksum HashSize. + // + // hash algorithm used here should remain functionally correct even + // after the truncation + // + // ex) "https://bugzilla.xamarin.com/show_bug.cgi?id=60298" - LayoutKind.Explicit, Size = 12 ignored with 64bit alignment + // or "https://github.com/dotnet/roslyn/issues/23722" - Checksum throws on Mono 64-bit return Checksum.From(bytes); } } From a416bd6acd22047490c0988061ce6e8f73e1f12f Mon Sep 17 00:00:00 2001 From: HeeJae Chang <hechang@microsoft.com> Date: Mon, 18 Feb 2019 16:02:36 -0800 Subject: [PATCH 9/9] moved comments around and removed reference to SHA in checksum --- src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs | 5 ++++- .../Core/Portable/Workspace/Solution/Checksum_Factory.cs | 3 --- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs index 790cea00fba4..45876eaa65ac 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs @@ -88,6 +88,9 @@ private static unsafe Checksum FromWorker(byte[] checksum) fixed (byte* data = checksum) { // Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than HashSize. + // + // ex) "https://bugzilla.xamarin.com/show_bug.cgi?id=60298" - LayoutKind.Explicit, Size = 12 ignored with 64bit alignment + // or "https://github.com/dotnet/roslyn/issues/23722" - Checksum throws on Mono 64-bit return new Checksum(HashData.FromPointer((HashData*)data)); } } @@ -153,7 +156,7 @@ public static string GetChecksumsLogInfo(IEnumerable<Checksum> checksums) } /// <summary> - /// This structure stores the 20-byte SHA256 hash as an inline value rather than requiring the use of + /// This structure stores the 20-byte hash as an inline value rather than requiring the use of /// <c>byte[]</c>. /// </summary> [StructLayout(LayoutKind.Explicit, Size = HashSize)] diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs index d56265f55a9e..5a694c9d9561 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs @@ -47,9 +47,6 @@ public static Checksum Create(Stream stream) // // hash algorithm used here should remain functionally correct even // after the truncation - // - // ex) "https://bugzilla.xamarin.com/show_bug.cgi?id=60298" - LayoutKind.Explicit, Size = 12 ignored with 64bit alignment - // or "https://github.com/dotnet/roslyn/issues/23722" - Checksum throws on Mono 64-bit return Checksum.From(bytes); } }