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

made Checksum type to truncate byte array given to it if it is bigger than HashSize #33363

Merged
merged 10 commits into from
Feb 19, 2019
118 changes: 69 additions & 49 deletions src/Workspaces/Core/Portable/Workspace/Solution/Checksum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,65 +16,85 @@ 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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we dropping these comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it no longer matters whether the different system uses a different size for SHA1. First, we no longer use SHA1 and for SHA256, we always truncate it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the warning still seems applicable though right? Different runtimes will do different lengths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, that it will truncate to Hashsize. so different runtime having different size doesn't matter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment moved to Checksum_Factory

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(default);

private Sha1Hash _checkSum;
private readonly HashData _checksum;

public unsafe Checksum(byte[] checksum)
/// <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)
{
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)
if (checksum.Length < HashSize)
{
// Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize.
_checkSum = Sha1Hash.FromPointer((Sha1Hash*)data);
throw new ArgumentException($"checksum must be equal or bigger than the hash size: {HashSize}", nameof(checksum));
}

return FromWorker(checksum);
}

public unsafe Checksum(ImmutableArray<byte> checksum)
/// <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(ImmutableArray<byte> checksum)
{
if (checksum.Length == 0)
{
_checkSum = default;
return;
return Null;
}
else if (checksum.Length != Sha1HashSize)

if (checksum.Length < HashSize)
{
throw new ArgumentException($"{nameof(checksum)} must be a SHA-1 hash", nameof(checksum));
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(bytes);
checksum.CopyTo(sourceIndex: 0, bytes, destinationIndex: 0, length: HashSize);

return FromWorker(bytes);
}
}

fixed (byte* data = bytes)
{
// Avoid a direct dereferencing assignment since sizeof(Sha1Hash) may be greater than Sha1HashSize.
_checkSum = Sha1Hash.FromPointer((Sha1Hash*)data);
}
public static Checksum FromSerialized(byte[] checksum)
{
if (checksum.Length == 0)
{
return Null;
}

if (checksum.Length != HashSize)
{
throw new ArgumentException($"{nameof(checksum)} must be equal to the hash size: {HashSize}", nameof(checksum));
}

return FromWorker(checksum);
}

private static unsafe Checksum FromWorker(byte[] checksum)
{
fixed (byte* data = checksum)
{
// Avoid a direct dereferencing assignment since sizeof(HashData) may be greater than HashSize.
return new Checksum(HashData.FromPointer((HashData*)data));
}
}

private Checksum(Sha1Hash hash)
private Checksum(HashData hash)
{
_checkSum = hash;
_checksum = hash;
}

public bool Equals(Checksum other)
Expand All @@ -84,24 +104,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)
Expand All @@ -117,10 +137,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)
{
Expand All @@ -133,11 +153,11 @@ 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 = Sha1HashSize)]
private struct Sha1Hash : IEquatable<Sha1Hash>
[StructLayout(LayoutKind.Explicit, Size = HashSize)]
private struct HashData : IEquatable<HashData>
{
[FieldOffset(0)]
private long Data1;
Expand All @@ -148,10 +168,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)
Expand All @@ -161,18 +181,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();
Expand All @@ -186,9 +206,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
Expand Down
51 changes: 30 additions & 21 deletions src/Workspaces/Core/Portable/Workspace/Solution/Checksum_Factory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,40 @@ public static Checksum Create(Stream stream)
{
using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1))
{
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();

// 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);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.FromSerialized(Convert.FromBase64String(value));
}

protected override void WriteValue(JsonWriter writer, Checksum value, JsonSerializer serializer)
Expand Down