Skip to content

Commit 287c30e

Browse files
wenytang-msCopilot
andcommitted
fix: address stack trace review feedback
Cap clipboard prefill content, clarify command registration readiness, and add regression coverage for shared stack frame parsing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e50dc26-84c4-4532-a7d3-88f6fe016780
1 parent 867f458 commit 287c30e

2 files changed

Lines changed: 51 additions & 6 deletions

File tree

src/stackTraceLinkProvider.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ const MAX_LINKS_PER_DOCUMENT = 2000;
2727
// Only resolve to source locations the language server is expected to return.
2828
const ALLOWED_SOURCE_SCHEMES = new Set<string>(["file", "jdt"]);
2929

30-
// Cap how much of the clipboard we scan when deciding whether to prefill (ReDoS hygiene).
31-
const MAX_CLIPBOARD_SCAN_LENGTH = 20000;
30+
// Bound both stack-trace detection and scratch-document prefill so a large clipboard cannot create
31+
// an expensive untitled document (and keeps the detection regex input bounded).
32+
const MAX_CLIPBOARD_PREFILL_LENGTH = 20000;
3233

3334
interface IStackFrameLinkArgs {
3435
stackTrace: string;
@@ -131,15 +132,16 @@ async function analyzeStackTrace(): Promise<void> {
131132
// The command itself is auto-instrumented via instrumentOperationAsVsCodeCommand, so no
132133
// manual telemetry is needed here to track invocations.
133134
const clipboard = await env.clipboard.readText();
134-
const looksLikeTrace = parseJavaStackFrame(clipboard.slice(0, MAX_CLIPBOARD_SCAN_LENGTH)) !== undefined;
135-
const content = looksLikeTrace ? clipboard : "";
135+
const clipboardContent = clipboard.slice(0, MAX_CLIPBOARD_PREFILL_LENGTH);
136+
const looksLikeTrace = parseJavaStackFrame(clipboardContent) !== undefined;
137+
const content = looksLikeTrace ? clipboardContent : "";
136138
const document = await workspace.openTextDocument({ language: "log", content });
137139
await window.showTextDocument(document);
138140
}
139141

140142
export function registerStackTraceLinkProvider(context: ExtensionContext): void {
141-
// The commands are always available: the palette command must not depend on server
142-
// readiness, and the click handler is only ever reached from links the provider creates.
143+
// Register handlers immediately for programmatic invocations and existing command links.
144+
// Palette visibility and creation of new links are gated on language-server readiness elsewhere.
143145
context.subscriptions.push(
144146
commands.registerCommand(NAVIGATE_TO_STACK_FRAME_COMMAND, navigateToStackFrame),
145147
instrumentOperationAsVsCodeCommand(ANALYZE_STACK_TRACE_COMMAND, analyzeStackTrace),

test/stackFrameParser.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
import * as assert from "assert";
5+
6+
import { parseJavaStackFrame } from "../src/stackFrameParser";
7+
8+
suite("parseJavaStackFrame", () => {
9+
test("parses a tab-indented stack frame", () => {
10+
const stackTrace = "com.example.App.main(App.java:42)";
11+
12+
assert.deepStrictEqual(parseJavaStackFrame(`\tat ${stackTrace}`), {
13+
stackTrace,
14+
methodName: "com.example.App.main",
15+
lineNumber: 42,
16+
startIndex: 4,
17+
length: stackTrace.length,
18+
});
19+
});
20+
21+
test("preserves a module prefix", () => {
22+
const stackTrace = "java.base/java.util.ArrayList.forEach(ArrayList.java:1511)";
23+
24+
assert.deepStrictEqual(parseJavaStackFrame(`\tat ${stackTrace}`), {
25+
stackTrace,
26+
methodName: "java.util.ArrayList.forEach",
27+
lineNumber: 1511,
28+
startIndex: 4,
29+
length: stackTrace.length,
30+
});
31+
});
32+
33+
test("calculates the link range within prefixed output", () => {
34+
const stackTrace = "com.example.Worker.run(Worker.java:7)";
35+
const line = `[stderr] \tat ${stackTrace} ~[app.jar:1.0]`;
36+
const frame = parseJavaStackFrame(line);
37+
38+
assert.ok(frame);
39+
assert.strictEqual(frame.startIndex, line.indexOf(stackTrace));
40+
assert.strictEqual(frame.length, stackTrace.length);
41+
assert.strictEqual(line.substring(frame.startIndex, frame.startIndex + frame.length), stackTrace);
42+
});
43+
});

0 commit comments

Comments
 (0)