Skip to content

Commit

Permalink
Revert "Enable peekable-based binary sniffing on ZipArchiveArtifacts (#…
Browse files Browse the repository at this point in the history
…2773)"

This reverts commit edbd822.
  • Loading branch information
rwoll committed Feb 9, 2024
1 parent b759fa1 commit 84a4782
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 28 deletions.
1 change: 0 additions & 1 deletion ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
* BUG: Fix `UnsupportedOperationException` in `ZipArchiveArtifact`.
* BUG: Fix `MultithreadedAnalyzeCommandBase` to return rich return code with the `--rich-return-code` option.
* NEW: Add `IsBinary` property to `IEnumeratedArtifact` and implement the property in `ZipArchiveArtifact`.
* NEW: Switch to content-based `IsBinary` categorization for `ZipArchiveArtifact`s.
* PRF: Change default `max-file-size-in-kb` parameter to 10 megabytes.
* PRF: Add support for efficiently peeking into non-seekable streams for binary/text categorization.
* NEW: Add a new `--timeout-in-seconds` parameter to `AnalyzeOptionsBase`, which will override the `TimeoutInMilliseconds` property in `AnalyzeContextBase`.
Expand Down
25 changes: 8 additions & 17 deletions src/Sarif/ZipArchiveArtifact.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public bool IsBinary
{
get
{
GetArtifactData();
return this.bytes != null;
string extension = Path.GetExtension(Uri.ToString());
return this.binaryExtensions.Contains(extension);
}
}

Expand Down Expand Up @@ -91,20 +91,8 @@ public byte[] Bytes
{
if (this.contents == null && this.bytes == null)
{
const int PeekWindowBytes = 1024;
var peekable = new PeekableStream(this.Stream, PeekWindowBytes);

byte[] header = new byte[PeekWindowBytes];
int length = this.Stream.Read(header, 0, header.Length);
bool isText = FileEncoding.IsTextualData(header, 0, length);

peekable.Rewind();

if (isText)
{
this.contents = new StreamReader(Stream).ReadToEnd();
}
else
string extension = Path.GetExtension(Uri.ToString());
if (this.binaryExtensions.Contains(extension))
{
// The underlying System.IO.Compression.DeflateStream throws on reads to get_Length.
using var ms = new MemoryStream((int)SizeInBytes.Value);
Expand All @@ -125,9 +113,12 @@ public byte[] Bytes
ms.Read(this.bytes, 0, this.bytes.Length);
}
}
else
{
this.contents = new StreamReader(Stream).ReadToEnd();
}
}
}

this.entry = null;
}

Expand Down
16 changes: 6 additions & 10 deletions src/Test.UnitTests.Sarif/ArtifactProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.IO;
using System.IO.Compression;
using System.Linq;
using System.Text;

using FluentAssertions;

Expand All @@ -31,19 +32,14 @@ public void MultithreadedZipArchiveArtifactProvider_RetrieveSizeInBytesBeforeRet
[Fact]
public void MultithreadedZipArchiveArtifactProvider_RetrieveSizeInBytesBeforeRetrievingBytes()
{
string filePath = this.GetType().Assembly.Location;
using FileStream reader = File.OpenRead(filePath);

int headerSize = 1024;
byte[] data = new byte[headerSize];
reader.Read(data, 0, data.Length);
string entryContents = $"{Guid.NewGuid()}";

// Note that even thought we populate an archive with binary contents, the extension
// of the archive entry indicates a text file. We still expect binary data on expansion.
ZipArchive zip = CreateZipArchiveWithBinaryContents("test.txt", data);
// Note that even thought we populate an archive with text contents, the extension
// of the archive entry indicates a binary file. So we expect binary data on expansion.
ZipArchive zip = CreateZipArchiveWithTextContents("test.exe", entryContents);
var artifactProvider = new MultithreadedZipArchiveArtifactProvider(zip, FileSystem.Instance);

ValidateBinaryContents(artifactProvider.Artifacts, data);
ValidateBinaryContents(artifactProvider.Artifacts, Encoding.UTF8.GetBytes(entryContents));
}

[Fact]
Expand Down

0 comments on commit 84a4782

Please sign in to comment.