Skip to content

Commit

Permalink
Add Exception Handling Around More Recursive Extractor Calls (#599)
Browse files Browse the repository at this point in the history
* Add Catch around Recursive Extractor calls

Was able to reproduce overflow exceptions thrown here causing end to execution, I think this is potentially the same root cause of issue #598's report of null streams causing a crash as well.

* Fix Capitalization

* Clamp GetLocation to end of file
  • Loading branch information
gfs authored Jan 10, 2025
1 parent 92a437d commit 567ac2f
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 14 deletions.
4 changes: 2 additions & 2 deletions AppInspector.RulesEngine/RuleProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,11 @@ internal static string ExtractExcerpt(TextContainer text, Location start, Locati

int startLineNumber =
start.Line < 0 ? 0 : start.Line > text.LineEnds.Count ? text.LineEnds.Count - 1 : start.Line;
int endLineNUmber =
int endLineNumber =
end.Line < 0 ? 0 : end.Line > text.LineEnds.Count ? text.LineEnds.Count - 1 : end.Line;
// First we try to include the number of lines of context requested
var excerptStartLine = Math.Max(0, startLineNumber - context);
var excerptEndLine = Math.Min(text.LineEnds.Count - 1, endLineNUmber + context);
var excerptEndLine = Math.Min(text.LineEnds.Count - 1, endLineNumber + context);
var startIndex = text.LineStarts[excerptStartLine];
var endIndex = text.LineEnds[excerptEndLine] + 1;
// Maximum number of characters to capture on each side
Expand Down
13 changes: 13 additions & 0 deletions AppInspector.RulesEngine/TextContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,14 @@ public string GetLineContent(int line)

/// <summary>
/// Returns location (Line, Column) for given index in text
/// If the index is beyond the end of the file, clamps to the end
/// </summary>
/// <param name="index"> Position in text (line is one-indexed)</param>
/// <returns> Location </returns>
public Location GetLocation(int index)
{
for (var i = 1; i < LineEnds.Count; i++)
{
if (LineEnds[i] >= index)
{
return new Location
Expand All @@ -463,6 +465,17 @@ public Location GetLocation(int index)
Line = i
};
}
}

// If the index is beyond the end of the file, clamp to the end of the file
if (index > LineEnds[^1])
{
return new Location()
{
Column = LineEnds[^1] - LineStarts[^1],
Line = LineEnds.Count
};
}

return new Location();
}
Expand Down
42 changes: 30 additions & 12 deletions AppInspector/Commands/AnalyzeCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Globalization;
using System.IO;
Expand Down Expand Up @@ -771,21 +772,38 @@ private IEnumerable<FileEntry> EnumerateFileEntries()

if (contents != null)
{
if (_options.DisableCrawlArchives)
IList<FileEntry> entriesToYield = new List<FileEntry>();
try
{
yield return new FileEntry(srcFile, contents);
if (_options.DisableCrawlArchives)
{
entriesToYield.Add(new FileEntry(srcFile, contents));
}
else
{
// Use MemoryStreamCutoff = 1 to force using FileStream with DeleteOnClose for backing, and avoid memory exhaustion.
ExtractorOptions opts = new()
{
Parallel = false, DenyFilters = _options.FilePathExclusions, MemoryStreamCutoff = 1
};
// This works if the contents contain any kind of file.
// If the file is an archive this gets all the entries it contains.
// If the file is not an archive, the stream is wrapped in a FileEntry container and yielded
entriesToYield = extractor.Extract(srcFile, contents, opts).ToImmutableList();
}
}
else
catch (Exception e)
{
// Use MemoryStreamCutoff = 1 to force using FileStream with DeleteOnClose for backing, and avoid memory exhaustion.
ExtractorOptions opts = new()
{
Parallel = false, DenyFilters = _options.FilePathExclusions, MemoryStreamCutoff = 1
};
// This works if the contents contain any kind of file.
// If the file is an archive this gets all the entries it contains.
// If the file is not an archive, the stream is wrapped in a FileEntry container and yielded
foreach (var entry in extractor.Extract(srcFile, contents, opts)) yield return entry;
_logger.LogDebug(
"Failed to analyze file {Path}. {Type}:{Message}. ({StackTrace})",
srcFile, e.GetType(), e.Message, e.StackTrace);
_metaDataHelper?.Metadata.Files.Add(new FileRecord
{ FileName = srcFile, Status = ScanState.Error });
}

foreach (var entry in entriesToYield)
{
yield return entry;
}
}

Expand Down

0 comments on commit 567ac2f

Please sign in to comment.