Skip to content

Commit ff483f2

Browse files
AArnottAArnott
committed
Avoid inexact stream reads
1 parent 2009e91 commit ff483f2

File tree

4 files changed

+46
-39
lines changed

4 files changed

+46
-39
lines changed

‎src/NerdBank.GitVersioning/ManagedGit/GitObjectStream.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public GitObjectStream(Stream stream, string objectType)
4545
private void ReadObjectTypeAndLength(string objectType)
4646
{
4747
Span<byte> buffer = stackalloc byte[128];
48-
this.Read(buffer.Slice(0, objectType.Length + 1));
48+
this.ReadAll(buffer.Slice(0, objectType.Length + 1));
4949

5050
string? actualObjectType = GitRepository.GetString(buffer.Slice(0, objectType.Length));
5151
this.ObjectType = actualObjectType;
@@ -55,7 +55,7 @@ private void ReadObjectTypeAndLength(string objectType)
5555

5656
while (headerLength < buffer.Length)
5757
{
58-
this.Read(buffer.Slice(headerLength, 1));
58+
this.ReadAll(buffer.Slice(headerLength, 1));
5959

6060
if (buffer[headerLength] == 0)
6161
{

‎src/NerdBank.GitVersioning/ManagedGit/GitPackReader.cs‎

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,60 +24,67 @@ public static Stream GetObject(GitPack pack, Stream stream, long offset, string
2424
throw new ArgumentNullException(nameof(stream));
2525
}
2626

27-
// Read the signature
27+
try
28+
{
29+
// Read the signature
2830
#if DEBUG
29-
stream.Seek(0, SeekOrigin.Begin);
30-
Span<byte> buffer = stackalloc byte[12];
31-
stream.ReadAll(buffer);
31+
stream.Seek(0, SeekOrigin.Begin);
32+
Span<byte> buffer = stackalloc byte[12];
33+
stream.ReadAll(buffer);
3234

33-
Debug.Assert(buffer.Slice(0, 4).SequenceEqual(Signature));
35+
Debug.Assert(buffer.Slice(0, 4).SequenceEqual(Signature));
3436

35-
int versionNumber = BinaryPrimitives.ReadInt32BigEndian(buffer.Slice(4, 4));
36-
Debug.Assert(versionNumber == 2);
37+
int versionNumber = BinaryPrimitives.ReadInt32BigEndian(buffer.Slice(4, 4));
38+
Debug.Assert(versionNumber == 2);
3739

38-
int numberOfObjects = BinaryPrimitives.ReadInt32BigEndian(buffer.Slice(8, 4));
40+
int numberOfObjects = BinaryPrimitives.ReadInt32BigEndian(buffer.Slice(8, 4));
3941
#endif
4042

41-
stream.Seek(offset, SeekOrigin.Begin);
43+
stream.Seek(offset, SeekOrigin.Begin);
4244

43-
(GitPackObjectType type, long decompressedSize) = ReadObjectHeader(stream);
45+
(GitPackObjectType type, long decompressedSize) = ReadObjectHeader(stream);
4446

45-
if (type == GitPackObjectType.OBJ_OFS_DELTA)
46-
{
47-
long baseObjectRelativeOffset = ReadVariableLengthInteger(stream);
48-
long baseObjectOffset = offset - baseObjectRelativeOffset;
47+
if (type == GitPackObjectType.OBJ_OFS_DELTA)
48+
{
49+
long baseObjectRelativeOffset = ReadVariableLengthInteger(stream);
50+
long baseObjectOffset = offset - baseObjectRelativeOffset;
4951

50-
var deltaStream = new ZLibStream(stream, decompressedSize);
51-
Stream? baseObjectStream = pack.GetObject(baseObjectOffset, objectType);
52+
var deltaStream = new ZLibStream(stream, decompressedSize);
53+
Stream? baseObjectStream = pack.GetObject(baseObjectOffset, objectType);
5254

53-
return new GitPackDeltafiedStream(baseObjectStream, deltaStream);
54-
}
55-
else if (type == GitPackObjectType.OBJ_REF_DELTA)
56-
{
57-
Span<byte> baseObjectId = stackalloc byte[20];
58-
stream.ReadAll(baseObjectId);
55+
return new GitPackDeltafiedStream(baseObjectStream, deltaStream);
56+
}
57+
else if (type == GitPackObjectType.OBJ_REF_DELTA)
58+
{
59+
Span<byte> baseObjectId = stackalloc byte[20];
60+
stream.ReadAll(baseObjectId);
5961

60-
Stream baseObject = pack.GetObjectFromRepository(GitObjectId.Parse(baseObjectId), objectType)!;
61-
var seekableBaseObject = new GitPackMemoryCacheStream(baseObject);
62+
Stream baseObject = pack.GetObjectFromRepository(GitObjectId.Parse(baseObjectId), objectType)!;
63+
var seekableBaseObject = new GitPackMemoryCacheStream(baseObject);
6264

63-
var deltaStream = new ZLibStream(stream, decompressedSize);
65+
var deltaStream = new ZLibStream(stream, decompressedSize);
6466

65-
return new GitPackDeltafiedStream(seekableBaseObject, deltaStream);
66-
}
67+
return new GitPackDeltafiedStream(seekableBaseObject, deltaStream);
68+
}
69+
70+
// Tips for handling deltas: https://github.com/choffmeister/gitnet/blob/4d907623d5ce2d79a8875aee82e718c12a8aad0b/src/GitNet/GitPack.cs
71+
if (type != packObjectType)
72+
{
73+
throw new GitException($"An object of type {objectType} could not be located at offset {offset}.") { ErrorCode = GitException.ErrorCodes.ObjectNotFound };
74+
}
6775

68-
// Tips for handling deltas: https://github.com/choffmeister/gitnet/blob/4d907623d5ce2d79a8875aee82e718c12a8aad0b/src/GitNet/GitPack.cs
69-
if (type != packObjectType)
76+
return new ZLibStream(stream, decompressedSize);
77+
}
78+
catch (EndOfStreamException eof)
7079
{
71-
throw new GitException($"An object of type {objectType} could not be located at offset {offset}.") { ErrorCode = GitException.ErrorCodes.ObjectNotFound };
80+
throw new GitException($"An object of type {objectType} could not be located at offset {offset}.", eof) { ErrorCode = GitException.ErrorCodes.ObjectNotFound };
7281
}
73-
74-
return new ZLibStream(stream, decompressedSize);
7582
}
7683

7784
private static (GitPackObjectType ObjectType, long Length) ReadObjectHeader(Stream stream)
7885
{
7986
Span<byte> value = stackalloc byte[1];
80-
stream.Read(value);
87+
stream.ReadAll(value);
8188

8289
var type = (GitPackObjectType)((value[0] & 0b0111_0000) >> 4);
8390
long length = value[0] & 0b_1111;
@@ -91,7 +98,7 @@ private static (GitPackObjectType ObjectType, long Length) ReadObjectHeader(Stre
9198

9299
do
93100
{
94-
stream.Read(value);
101+
stream.ReadAll(value);
95102
length = length | ((value[0] & 0b0111_1111L) << shift);
96103
shift += 7;
97104
}

‎test/Nerdbank.GitVersioning.Tests/ManagedGit/GitPackDeltafiedStreamTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public void TestDeltaStream(string basePath, string deltaPath, string expectedPa
2323
using (Stream expectedStream = TestUtilities.GetEmbeddedResource(expectedPath))
2424
{
2525
expected = new byte[expectedStream.Length];
26-
expectedStream.Read(expected);
26+
expectedStream.ReadAll(expected);
2727
}
2828

2929
byte[] actual = new byte[expected.Length];
@@ -34,7 +34,7 @@ public void TestDeltaStream(string basePath, string deltaPath, string expectedPa
3434
{
3535
////Assert.Equal(expected.Length, deltafiedStream.Length);
3636

37-
deltafiedStream.Read(actual);
37+
deltafiedStream.ReadAll(actual);
3838

3939
Assert.Equal(expected, actual);
4040
}

‎test/Nerdbank.GitVersioning.Tests/ManagedGit/ZLibStreamTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void SeekTest()
4141
// Seek past the commit 137 header, and make sure we can read the 'tree' word
4242
Assert.Equal(11, stream.Seek(11, SeekOrigin.Begin));
4343
byte[] tree = new byte[4];
44-
stream.Read(tree);
44+
stream.ReadAll(tree);
4545
Assert.Equal("tree", Encoding.UTF8.GetString(tree));
4646

4747
// Valid no-ops

0 commit comments

Comments
 (0)