Skip to content

Commit 994ebaa

Browse files
authored
Merge pull request #1827 from github/koesie10/filter-sarif-snippets
Limit SARIF code snippet size
2 parents 008d7b3 + 402b338 commit 994ebaa

3 files changed

Lines changed: 76 additions & 6 deletions

File tree

extensions/ql-vscode/src/pure/sarif-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as Sarif from "sarif";
2-
import { HighlightedRegion } from "../remote-queries/shared/analysis-result";
2+
import type { HighlightedRegion } from "../remote-queries/shared/analysis-result";
33
import { ResolvableLocationValue } from "./bqrs-cli-types";
44

55
export interface SarifLink {

extensions/ql-vscode/src/remote-queries/sarif-processing.ts

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as sarif from "sarif";
22
import {
3+
parseHighlightedLine,
34
parseSarifPlainTextMessage,
45
parseSarifRegion,
56
} from "../pure/sarif-utils";
@@ -15,6 +16,11 @@ import {
1516
HighlightedRegion,
1617
} from "./shared/analysis-result";
1718

19+
// A line of more than 8k characters is probably generated.
20+
const CODE_SNIPPET_LARGE_LINE_SIZE_LIMIT = 8192;
21+
// If less than 1% of the line is highlighted, we consider it a small snippet.
22+
const CODE_SNIPPET_HIGHLIGHTED_REGION_MINIMUM_RATIO = 0.01;
23+
1824
const defaultSeverity = "Warning";
1925

2026
export function extractAnalysisAlerts(
@@ -163,17 +169,44 @@ export function tryGetRule(
163169
}
164170

165171
function getCodeSnippet(
172+
contextRegion?: sarif.Region,
166173
region?: sarif.Region,
167-
alternateRegion?: sarif.Region,
168174
): CodeSnippet | undefined {
169-
region = region ?? alternateRegion;
175+
const actualRegion = contextRegion ?? region;
170176

171-
if (!region) {
177+
if (!actualRegion) {
172178
return undefined;
173179
}
174180

175-
const text = region.snippet?.text || "";
176-
const { startLine, endLine } = parseSarifRegion(region);
181+
const text = actualRegion.snippet?.text || "";
182+
const { startLine, endLine } = parseSarifRegion(actualRegion);
183+
184+
if (
185+
contextRegion &&
186+
region &&
187+
text.length > CODE_SNIPPET_LARGE_LINE_SIZE_LIMIT
188+
) {
189+
const code = text.split("\n");
190+
191+
const highlightedRegion = parseSarifRegion(region);
192+
193+
const highlightedLines = code.map((line, index) => {
194+
return parseHighlightedLine(line, startLine + index, highlightedRegion);
195+
});
196+
197+
const highlightedCharactersCount = highlightedLines.reduce(
198+
(a, line) => a + line.highlightedSection.length,
199+
0,
200+
);
201+
202+
const highlightedRatio = highlightedCharactersCount / text.length;
203+
204+
if (highlightedRatio < CODE_SNIPPET_HIGHLIGHTED_REGION_MINIMUM_RATIO) {
205+
// If not enough is highlighted and the snippet is large, it's probably generated or bundled code and
206+
// we don't want to show it.
207+
return undefined;
208+
}
209+
}
177210

178211
return {
179212
startLine,

extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,43 @@ describe("SARIF processing", () => {
637637
expect(message.tokens[2].t).toBe("text");
638638
expect(message.tokens[2].text).toBe(".");
639639
});
640+
641+
it("should not include snippets for large snippets", () => {
642+
const sarif = buildValidSarifLog();
643+
// Build string of 10 kilobytes
644+
const snippet = new Array(10 * 1024).fill("a").join("");
645+
sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion!.snippet =
646+
{
647+
text: snippet,
648+
};
649+
650+
const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix);
651+
652+
const actualCodeSnippet = result.alerts[0].codeSnippet;
653+
654+
expect(result).toBeTruthy();
655+
expectNoParsingError(result);
656+
expect(actualCodeSnippet).toBeUndefined();
657+
});
658+
659+
it("should include snippets for large snippets which are relevant", () => {
660+
const sarif = buildValidSarifLog();
661+
// Build string of 10 kilobytes
662+
const snippet = new Array(10 * 1024).fill("a").join("");
663+
sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion!.snippet =
664+
{
665+
text: snippet,
666+
};
667+
sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.region!.endColumn = 1000;
668+
669+
const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix);
670+
671+
const actualCodeSnippet = result.alerts[0].codeSnippet;
672+
673+
expect(result).toBeTruthy();
674+
expectNoParsingError(result);
675+
expect(actualCodeSnippet).not.toBeUndefined();
676+
});
640677
});
641678

642679
function expectResultParsingError(msg: string) {

0 commit comments

Comments
 (0)