From 6ee15d9ef3f9333085242d952afbba2ac571b69d Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Mon, 19 Apr 2021 13:01:27 +0200 Subject: [PATCH 1/6] Fix git parser on really big pack files. > 2GB --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 99 ++++++++++++++++---------- 1 file changed, 63 insertions(+), 36 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index dd6948810f50..23712f6fd697 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -451,24 +451,38 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G { // Move to the offset of the object fs.Seek(packageOffset.Offset, SeekOrigin.Begin); - int objectSize; byte[] packData = br.ReadBytes(2); - if (packData[0] < 128) + // We build the size combining the bits from sizeParts + // using bitwise operations (BigEndian). + // Example: + // - sizeParts = [-100, 53] => [10011100, 00110101] + // - size = 00000000 00000000 00000000 00000000 + int size = 0; + + // Clean first bit and add bits to size using OR. + // size 00000000 00000000 00000000 00000000 + // OR sizePart[1] & 0x7F 00110101 + // size 00000000 00000000 00000000 00110101 + size |= (packData[1] & 0x7F); + + // Move 4 bits to the left. + // size 00000000 00000000 00000011 01010000 + size <<= 4; + + // Clean four initial bits and add the result to size using OR. + // size 00000000 00000000 00000011 01010000 + // OR sizePart[0] & 0x0F 00001100 + // size 00000000 00000000 00000011 01011100 + size |= (packData[0] & 0x0F); + + // Advance 2 bytes to skip the zlib magic number + _ = br.ReadUInt16(); + + // Read the git commit object + using (var defStream = new DeflateStream(br.BaseStream, CompressionMode.Decompress)) { - objectSize = (int)(packData[0] & 0x0f); - packData = br.ReadBytes(objectSize); - } - else - { - objectSize = (((ushort)(packData[1] & 0x7f)) * 16) + ((ushort)(packData[0] & 0x0f)); - packData = br.ReadBytes(objectSize * 100); - } - - using (var ms = new MemoryStream(packData, 2, packData.Length - 2)) - using (var defStream = new DeflateStream(ms, CompressionMode.Decompress)) - { - byte[] buffer = new byte[8192]; + byte[] buffer = new byte[size]; int readBytes = defStream.Read(buffer, 0, buffer.Length); defStream.Close(); string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); @@ -507,46 +521,57 @@ public static bool TryGetPackageOffset(string idxFilePath, string commitSha, out using (var fs = new FileStream(idxFilePath, FileMode.Open, FileAccess.Read, FileShare.Read)) using (var br = new BigEndianBinaryReader(fs)) { + // Skip header and version fs.Seek(8, SeekOrigin.Begin); // First layer: 256 4-byte elements, with number of elements per folder - fs.Seek(previousIndex * 4, SeekOrigin.Current); - var numberOfPreviousObjects = br.ReadUInt32(); - var numberOfObjectsInIndex = br.ReadUInt32() - numberOfPreviousObjects; - fs.Seek(-8, SeekOrigin.Current); + uint numberOfObjectsInPreviousIndex = 0; + if (previousIndex > -1) + { + // Seek to previous index position and read the number of objects + fs.Seek(previousIndex * 4, SeekOrigin.Current); + numberOfObjectsInPreviousIndex = br.ReadUInt32(); + } + + // In the fanout table, every index has its objects + the previous ones. + // We need to subtract the previous index objects to know the correct + // actual number of objects for this specific index. + uint numberOfObjectsInIndex = br.ReadUInt32() - numberOfObjectsInPreviousIndex; - fs.Seek((255 - previousIndex) * 4, SeekOrigin.Current); - var totalNumberOfObjects = br.ReadUInt32(); + // Seek to last position. The last position contains the number of all objects. + fs.Seek((255 - (folderIndex + 1)) * 4, SeekOrigin.Current); + uint totalNumberOfObjects = br.ReadUInt32(); // Second layer: 20-byte elements with the names in order - fs.Seek(20 * (int)numberOfPreviousObjects, SeekOrigin.Current); + // Search the sha index in the second layer: the SHA listing. uint? indexOfCommit = null; + fs.Seek(20 * (int)numberOfObjectsInPreviousIndex, SeekOrigin.Current); for (uint i = 0; i < numberOfObjectsInIndex; i++) { - var str = BitConverter.ToString(br.ReadBytes(20)).Replace("-", string.Empty); + string str = BitConverter.ToString(br.ReadBytes(20)).Replace("-", string.Empty); if (str.Equals(commitSha, StringComparison.OrdinalIgnoreCase)) { - indexOfCommit = numberOfPreviousObjects + i; - fs.Seek(-20, SeekOrigin.Current); + indexOfCommit = numberOfObjectsInPreviousIndex + i; + + // If we find the SHA, we skip all SHA listing table. + fs.Seek(20 * (totalNumberOfObjects - (indexOfCommit.Value + 1)), SeekOrigin.Current); break; } } if (indexOfCommit.HasValue) { - uint indexOfObject = indexOfCommit.Value; - fs.Seek(20 * (totalNumberOfObjects - indexOfObject), SeekOrigin.Current); - // Third layer: 4 byte CRC for each object. We skip it fs.Seek(4 * totalNumberOfObjects, SeekOrigin.Current); + uint indexOfCommitValue = indexOfCommit.Value; + // Fourth layer: 4 byte per object of offset in pack file - fs.Seek(4 * indexOfObject, SeekOrigin.Current); - var offset = br.ReadUInt32(); - fs.Seek(-4, SeekOrigin.Current); + fs.Seek(4 * indexOfCommitValue, SeekOrigin.Current); + uint offset = br.ReadUInt32(); ulong packOffset; - if ((offset & 0x8000000) == 0) + if (((offset >> 31) & 1) == 0) { // offset is in the layer packOffset = (ulong)offset; @@ -554,11 +579,13 @@ public static bool TryGetPackageOffset(string idxFilePath, string commitSha, out else { // offset is not in this layer, clear first bit and look at it at the 5th layer - offset = offset & 0x7FFFFFFF; - fs.Seek(4 * (totalNumberOfObjects - indexOfObject), SeekOrigin.Current); - fs.Seek(8 * indexOfObject, SeekOrigin.Current); + offset &= 0x7FFFFFFF; + // Skip complete fourth layer. + fs.Seek(4 * (totalNumberOfObjects - (indexOfCommitValue + 1)), SeekOrigin.Current); + // Use the offset from fourth layer, to find the actual pack file offset in the fifth layer. + // In this case, the offset is 8 bytes long. + fs.Seek(8 * offset, SeekOrigin.Current); packOffset = br.ReadUInt64(); - fs.Seek(-8, SeekOrigin.Current); } packageOffset = new GitPackageOffset(idxFilePath, (long)packOffset); From 978cdf42724f92d833149d88fa34825e83dac0be Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Mon, 19 Apr 2021 18:28:59 +0200 Subject: [PATCH 2/6] Fix parser on big object size. --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 96 ++++++++++++++------------ 1 file changed, 50 insertions(+), 46 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index 23712f6fd697..2e72490d07a3 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -439,58 +439,62 @@ public static bool TryGetFromObjectFile(string filePath, out GitCommitObject com public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out GitCommitObject commitObject) { commitObject = default; - - string packFile = Path.ChangeExtension(packageOffset.FilePath, ".pack"); - if (File.Exists(packFile)) + try { - // packfile format explanation: - // https://codewords.recurse.com/issues/three/unpacking-git-packfiles#:~:text=idx%20file%20contains%20the%20index,pack%20file.&text=Objects%20in%20a%20packfile%20can,of%20storing%20the%20whole%20object. - - using (var fs = new FileStream(packFile, FileMode.Open, FileAccess.Read, FileShare.Read)) - using (var br = new BigEndianBinaryReader(fs)) + string packFile = Path.ChangeExtension(packageOffset.FilePath, ".pack"); + if (File.Exists(packFile)) { - // Move to the offset of the object - fs.Seek(packageOffset.Offset, SeekOrigin.Begin); - byte[] packData = br.ReadBytes(2); - - // We build the size combining the bits from sizeParts - // using bitwise operations (BigEndian). - // Example: - // - sizeParts = [-100, 53] => [10011100, 00110101] - // - size = 00000000 00000000 00000000 00000000 - int size = 0; - - // Clean first bit and add bits to size using OR. - // size 00000000 00000000 00000000 00000000 - // OR sizePart[1] & 0x7F 00110101 - // size 00000000 00000000 00000000 00110101 - size |= (packData[1] & 0x7F); - - // Move 4 bits to the left. - // size 00000000 00000000 00000011 01010000 - size <<= 4; - - // Clean four initial bits and add the result to size using OR. - // size 00000000 00000000 00000011 01010000 - // OR sizePart[0] & 0x0F 00001100 - // size 00000000 00000000 00000011 01011100 - size |= (packData[0] & 0x0F); - - // Advance 2 bytes to skip the zlib magic number - _ = br.ReadUInt16(); - - // Read the git commit object - using (var defStream = new DeflateStream(br.BaseStream, CompressionMode.Decompress)) + // packfile format explanation: + // https://codewords.recurse.com/issues/three/unpacking-git-packfiles#:~:text=idx%20file%20contains%20the%20index,pack%20file.&text=Objects%20in%20a%20packfile%20can,of%20storing%20the%20whole%20object. + + using (var fs = new FileStream(packFile, FileMode.Open, FileAccess.Read, FileShare.Read)) + using (var br = new BigEndianBinaryReader(fs)) { - byte[] buffer = new byte[size]; - int readBytes = defStream.Read(buffer, 0, buffer.Length); - defStream.Close(); - string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); - commitObject = new GitCommitObject(strContent); - return true; + // Move to the offset of the object + fs.Seek(packageOffset.Offset, SeekOrigin.Begin); + byte[] packData = br.ReadBytes(2); + + int objectSize = (int)(packData[0] & 0x0F); + + if (packData[0] > 128) + { + objectSize += (packData[1] & 0x7F) * 16; + if (packData[1] > 128) + { + int multiplier = 128; + byte pData = br.ReadByte(); + objectSize += (pData & 0x7F) * multiplier; + while (pData > 128) + { + multiplier *= 128; + pData = br.ReadByte(); + objectSize += (pData & 0x7F) * multiplier; + } + } + } + + // Advance 2 bytes to skip the zlib magic number + uint zlibMagicNumber = br.ReadUInt16(); + if ((byte)zlibMagicNumber == 0x78) + { + // Read the git commit object + using (var defStream = new DeflateStream(br.BaseStream, CompressionMode.Decompress)) + { + byte[] buffer = new byte[objectSize]; + int readBytes = defStream.Read(buffer, 0, buffer.Length); + defStream.Close(); + string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); + commitObject = new GitCommitObject(strContent); + return true; + } + } } } } + catch (Exception ex) + { + _ = ex; + } return false; } From 35f99b2b3b49568ce004274e39789d24df76fd63 Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Mon, 19 Apr 2021 19:00:43 +0200 Subject: [PATCH 3/6] add comment. --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index 2e72490d07a3..f64dc08effab 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -454,6 +454,7 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G fs.Seek(packageOffset.Offset, SeekOrigin.Begin); byte[] packData = br.ReadBytes(2); + // Extract the object size int objectSize = (int)(packData[0] & 0x0F); if (packData[0] > 128) @@ -491,9 +492,8 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G } } } - catch (Exception ex) + catch { - _ = ex; } return false; From c502dc47f2f32216b8cffbffcea62512ec7edb17 Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Mon, 19 Apr 2021 21:22:04 +0200 Subject: [PATCH 4/6] changes based in the comments. --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 175 ++++++++++++++----------- 1 file changed, 97 insertions(+), 78 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index f64dc08effab..a5844bfc84c1 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Text; using System.Text.RegularExpressions; +using Datadog.Trace.Logging; namespace Datadog.Trace.Ci { @@ -13,6 +14,8 @@ namespace Datadog.Trace.Ci /// </summary> internal class GitInfo { + private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(GitInfo)); + private GitInfo() { } @@ -219,8 +222,9 @@ private static GitInfo GetFrom(DirectoryInfo gitDirectory) } } } - catch + catch (Exception ex) { + Log.Error(ex, ex.Message); } return gitInfo; @@ -413,25 +417,32 @@ private GitCommitObject(string content) public static bool TryGetFromObjectFile(string filePath, out GitCommitObject commitObject) { commitObject = default; - using (var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read)) + try { - // We skip the 2 bytes zlib header magic number. - fs.Seek(2, SeekOrigin.Begin); - using (var defStream = new DeflateStream(fs, CompressionMode.Decompress)) + using (var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read)) { - byte[] buffer = new byte[8192]; - int readBytes = defStream.Read(buffer, 0, buffer.Length); - defStream.Close(); - - if (_commitByteArray.SequenceEqual(buffer.Take(_commitByteArray.Length))) + // We skip the 2 bytes zlib header magic number. + fs.Seek(2, SeekOrigin.Begin); + using (var defStream = new DeflateStream(fs, CompressionMode.Decompress)) { - string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); - string dataContent = strContent.Substring(strContent.IndexOf('\0') + 1); - commitObject = new GitCommitObject(dataContent); - return true; + byte[] buffer = new byte[8192]; + int readBytes = defStream.Read(buffer, 0, buffer.Length); + defStream.Close(); + + if (_commitByteArray.SequenceEqual(buffer.Take(_commitByteArray.Length))) + { + string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); + string dataContent = strContent.Substring(strContent.IndexOf('\0') + 1); + commitObject = new GitCommitObject(dataContent); + return true; + } } } } + catch (Exception ex) + { + Log.Error(ex, ex.Message); + } return false; } @@ -454,18 +465,18 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G fs.Seek(packageOffset.Offset, SeekOrigin.Begin); byte[] packData = br.ReadBytes(2); - // Extract the object size + // Extract the object size (https://codewords.recurse.com/images/three/varint.svg) int objectSize = (int)(packData[0] & 0x0F); - if (packData[0] > 128) + if (packData[0] >= 128) { objectSize += (packData[1] & 0x7F) * 16; - if (packData[1] > 128) + if (packData[1] >= 128) { int multiplier = 128; byte pData = br.ReadByte(); objectSize += (pData & 0x7F) * multiplier; - while (pData > 128) + while (pData >= 128) { multiplier *= 128; pData = br.ReadByte(); @@ -492,8 +503,9 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G } } } - catch + catch (Exception ex) { + Log.Error(ex, ex.Message); } return false; @@ -522,80 +534,87 @@ public static bool TryGetPackageOffset(string idxFilePath, string commitSha, out int folderIndex = int.Parse(index, System.Globalization.NumberStyles.HexNumber); int previousIndex = folderIndex > 0 ? folderIndex - 1 : folderIndex; - using (var fs = new FileStream(idxFilePath, FileMode.Open, FileAccess.Read, FileShare.Read)) - using (var br = new BigEndianBinaryReader(fs)) + try { - // Skip header and version - fs.Seek(8, SeekOrigin.Begin); - - // First layer: 256 4-byte elements, with number of elements per folder - uint numberOfObjectsInPreviousIndex = 0; - if (previousIndex > -1) + using (var fs = new FileStream(idxFilePath, FileMode.Open, FileAccess.Read, FileShare.Read)) + using (var br = new BigEndianBinaryReader(fs)) { - // Seek to previous index position and read the number of objects - fs.Seek(previousIndex * 4, SeekOrigin.Current); - numberOfObjectsInPreviousIndex = br.ReadUInt32(); - } + // Skip header and version + fs.Seek(8, SeekOrigin.Begin); + + // First layer: 256 4-byte elements, with number of elements per folder + uint numberOfObjectsInPreviousIndex = 0; + if (previousIndex > -1) + { + // Seek to previous index position and read the number of objects + fs.Seek(previousIndex * 4, SeekOrigin.Current); + numberOfObjectsInPreviousIndex = br.ReadUInt32(); + } - // In the fanout table, every index has its objects + the previous ones. - // We need to subtract the previous index objects to know the correct - // actual number of objects for this specific index. - uint numberOfObjectsInIndex = br.ReadUInt32() - numberOfObjectsInPreviousIndex; + // In the fanout table, every index has its objects + the previous ones. + // We need to subtract the previous index objects to know the correct + // actual number of objects for this specific index. + uint numberOfObjectsInIndex = br.ReadUInt32() - numberOfObjectsInPreviousIndex; - // Seek to last position. The last position contains the number of all objects. - fs.Seek((255 - (folderIndex + 1)) * 4, SeekOrigin.Current); - uint totalNumberOfObjects = br.ReadUInt32(); + // Seek to last position. The last position contains the number of all objects. + fs.Seek((255 - (folderIndex + 1)) * 4, SeekOrigin.Current); + uint totalNumberOfObjects = br.ReadUInt32(); - // Second layer: 20-byte elements with the names in order - // Search the sha index in the second layer: the SHA listing. - uint? indexOfCommit = null; - fs.Seek(20 * (int)numberOfObjectsInPreviousIndex, SeekOrigin.Current); - for (uint i = 0; i < numberOfObjectsInIndex; i++) - { - string str = BitConverter.ToString(br.ReadBytes(20)).Replace("-", string.Empty); - if (str.Equals(commitSha, StringComparison.OrdinalIgnoreCase)) + // Second layer: 20-byte elements with the names in order + // Search the sha index in the second layer: the SHA listing. + uint? indexOfCommit = null; + fs.Seek(20 * (int)numberOfObjectsInPreviousIndex, SeekOrigin.Current); + for (uint i = 0; i < numberOfObjectsInIndex; i++) { - indexOfCommit = numberOfObjectsInPreviousIndex + i; + string str = BitConverter.ToString(br.ReadBytes(20)).Replace("-", string.Empty); + if (str.Equals(commitSha, StringComparison.OrdinalIgnoreCase)) + { + indexOfCommit = numberOfObjectsInPreviousIndex + i; - // If we find the SHA, we skip all SHA listing table. - fs.Seek(20 * (totalNumberOfObjects - (indexOfCommit.Value + 1)), SeekOrigin.Current); - break; + // If we find the SHA, we skip all SHA listing table. + fs.Seek(20 * (totalNumberOfObjects - (indexOfCommit.Value + 1)), SeekOrigin.Current); + break; + } } - } - if (indexOfCommit.HasValue) - { - // Third layer: 4 byte CRC for each object. We skip it - fs.Seek(4 * totalNumberOfObjects, SeekOrigin.Current); + if (indexOfCommit.HasValue) + { + // Third layer: 4 byte CRC for each object. We skip it + fs.Seek(4 * totalNumberOfObjects, SeekOrigin.Current); - uint indexOfCommitValue = indexOfCommit.Value; + uint indexOfCommitValue = indexOfCommit.Value; - // Fourth layer: 4 byte per object of offset in pack file - fs.Seek(4 * indexOfCommitValue, SeekOrigin.Current); - uint offset = br.ReadUInt32(); + // Fourth layer: 4 byte per object of offset in pack file + fs.Seek(4 * indexOfCommitValue, SeekOrigin.Current); + uint offset = br.ReadUInt32(); - ulong packOffset; - if (((offset >> 31) & 1) == 0) - { - // offset is in the layer - packOffset = (ulong)offset; - } - else - { - // offset is not in this layer, clear first bit and look at it at the 5th layer - offset &= 0x7FFFFFFF; - // Skip complete fourth layer. - fs.Seek(4 * (totalNumberOfObjects - (indexOfCommitValue + 1)), SeekOrigin.Current); - // Use the offset from fourth layer, to find the actual pack file offset in the fifth layer. - // In this case, the offset is 8 bytes long. - fs.Seek(8 * offset, SeekOrigin.Current); - packOffset = br.ReadUInt64(); - } + ulong packOffset; + if (((offset >> 31) & 1) == 0) + { + // offset is in the layer + packOffset = (ulong)offset; + } + else + { + // offset is not in this layer, clear first bit and look at it at the 5th layer + offset &= 0x7FFFFFFF; + // Skip complete fourth layer. + fs.Seek(4 * (totalNumberOfObjects - (indexOfCommitValue + 1)), SeekOrigin.Current); + // Use the offset from fourth layer, to find the actual pack file offset in the fifth layer. + // In this case, the offset is 8 bytes long. + fs.Seek(8 * offset, SeekOrigin.Current); + packOffset = br.ReadUInt64(); + } - packageOffset = new GitPackageOffset(idxFilePath, (long)packOffset); - return true; + packageOffset = new GitPackageOffset(idxFilePath, (long)packOffset); + return true; + } } } + catch (Exception ex) + { + Log.Error(ex, ex.Message); + } return false; } From 74c2fe08a6b93c2f321920fc81cedef70aa636f1 Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Tue, 20 Apr 2021 14:10:06 +0200 Subject: [PATCH 5/6] Fixes. --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 49 ++++++++++++++++---------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index a5844bfc84c1..a4b974d6a53f 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -467,39 +467,50 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G // Extract the object size (https://codewords.recurse.com/images/three/varint.svg) int objectSize = (int)(packData[0] & 0x0F); - if (packData[0] >= 128) { - objectSize += (packData[1] & 0x7F) * 16; + int shift = 4; + objectSize += (packData[1] & 0x7F) << shift; if (packData[1] >= 128) { - int multiplier = 128; - byte pData = br.ReadByte(); - objectSize += (pData & 0x7F) * multiplier; - while (pData >= 128) + byte pData; + do { - multiplier *= 128; + shift += 7; pData = br.ReadByte(); - objectSize += (pData & 0x7F) * multiplier; + objectSize += (pData & 0x7F) << shift; } + while (pData >= 128); } } - // Advance 2 bytes to skip the zlib magic number - uint zlibMagicNumber = br.ReadUInt16(); - if ((byte)zlibMagicNumber == 0x78) + // Check if the object size is in the aceptable range + if (objectSize > 0 && objectSize < ushort.MaxValue) { - // Read the git commit object - using (var defStream = new DeflateStream(br.BaseStream, CompressionMode.Decompress)) + // Advance 2 bytes to skip the zlib magic number + uint zlibMagicNumber = br.ReadUInt16(); + if ((byte)zlibMagicNumber == 0x78) + { + // Read the git commit object + using (var defStream = new DeflateStream(br.BaseStream, CompressionMode.Decompress)) + { + byte[] buffer = new byte[objectSize]; + int readBytes = defStream.Read(buffer, 0, buffer.Length); + defStream.Close(); + string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); + commitObject = new GitCommitObject(strContent); + return true; + } + } + else { - byte[] buffer = new byte[objectSize]; - int readBytes = defStream.Read(buffer, 0, buffer.Length); - defStream.Close(); - string strContent = Encoding.UTF8.GetString(buffer, 0, readBytes); - commitObject = new GitCommitObject(strContent); - return true; + Log.Warning("The commit data doesn't have a valid zlib header magic number."); } } + else + { + Log.Warning<int>("The object size is outside of an acceptable range: {objectSize}", objectSize); + } } } } From 6f8e006cb360777b5ed3c2ae8df5b356dd5b34fb Mon Sep 17 00:00:00 2001 From: Tony Redondo <tony.redondo@datadoghq.com> Date: Wed, 21 Apr 2021 13:22:57 +0200 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com> --- src/Datadog.Trace.Ci.Shared/GitInfo.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Datadog.Trace.Ci.Shared/GitInfo.cs b/src/Datadog.Trace.Ci.Shared/GitInfo.cs index a4b974d6a53f..b6b607b531d0 100644 --- a/src/Datadog.Trace.Ci.Shared/GitInfo.cs +++ b/src/Datadog.Trace.Ci.Shared/GitInfo.cs @@ -224,7 +224,7 @@ private static GitInfo GetFrom(DirectoryInfo gitDirectory) } catch (Exception ex) { - Log.Error(ex, ex.Message); + Log.Error(ex, "Error loading git information from directory"); } return gitInfo; @@ -441,7 +441,7 @@ public static bool TryGetFromObjectFile(string filePath, out GitCommitObject com } catch (Exception ex) { - Log.Error(ex, ex.Message); + Log.Error(ex, "Error getting commit object from object file"); } return false; @@ -516,7 +516,7 @@ public static bool TryGetFromPackageOffset(GitPackageOffset packageOffset, out G } catch (Exception ex) { - Log.Error(ex, ex.Message); + Log.Error(ex, "Error loading commit information from package offset"); } return false; @@ -624,7 +624,7 @@ public static bool TryGetPackageOffset(string idxFilePath, string commitSha, out } catch (Exception ex) { - Log.Error(ex, ex.Message); + Log.Error(ex, "Error getting package offset"); } return false;