Skip to content

Git - guard against undefined paths in parseGitChangesRaw (#287615)#314614

Open
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c:fix/scm-history-undefined-path-287615
Open

Git - guard against undefined paths in parseGitChangesRaw (#287615)#314614
yogeshwaran-c wants to merge 1 commit intomicrosoft:mainfrom
yogeshwaran-c:fix/scm-history-undefined-path-287615

Conversation

@yogeshwaran-c
Copy link
Copy Markdown
Contributor

@
Fixes #287615

Root cause

parseGitChangesRaw in extensions/git/src/git.ts parses the null-delimited output of git diff --raw --numstat -z. The parser advances through the segments array using a manual index++ counter and reads file paths via segments[index++] in two places:

  1. The colon-prefixed --raw branch: the file path that immediately follows the metadata segment.
  2. The --numstat rename branch: the new path that follows the empty filename and the skipped old path.

Neither read was guarded for the case where index runs past the end of segments (or yields an empty/undefined value). When that happens, the resulting filePath is undefined, and the subsequent Uri.file(...) call throws:

The "path" argument must be of type string. Received undefined

This bubbles up through GitHistoryProvider.provideHistoryItemChanges and is logged by extHostSCM.ts as ExtHostSCM#$provideHistoryItemChanges The "path" argument must be of type string. Received undefined, leaving the multi-file diff editor with no listed files.

Fix

Add a defensive if (!filePath) break; / if (!renamePath) break; guard in both branches, mirroring the existing guard already present for the rename newPath segment a few lines below. If the segment stream is unexpectedly truncated, parsing stops cleanly instead of crashing the provider call.

Narrow change: 6 added lines in one file, no behavior change for well-formed input.

Test plan

  • Build passes locally
  • Open commit history for a commit with many changes (including renames) — no path must be of type string error in Extension Host log; changed files render in the multi-file diff editor.
    @

Copilot AI review requested due to automatic review settings May 6, 2026 01:44
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@lszomoru

Matched files:

  • extensions/git/src/git.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Git extension’s parseGitChangesRaw parser (used for git diff --raw --numstat -z) to avoid throwing when the null-delimited segment stream is unexpectedly truncated, which was surfacing as Uri.file(...) receiving undefined and breaking commit history/multi-file diff rendering.

Changes:

  • Add defensive guards to stop parsing when a --raw file path segment is missing.
  • Add defensive guards to stop parsing when a --numstat rename “new path” segment is missing.

Comment thread extensions/git/src/git.ts
Comment on lines 1138 to +1143
// Parse --raw output
const [, , , , change] = segment.split(' ');
const filePath = segments[index++];
if (!filePath) {
break;
}
Comment thread extensions/git/src/git.ts
Comment on lines +1192 to 1196
break;
}
numstatPath = path.isAbsolute(renamePath) ? renamePath : path.join(repositoryRoot, renamePath);
} else {
numstatPath = path.isAbsolute(filePath) ? filePath : path.join(repositoryRoot, filePath);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Source Control: Error path must be of type string when opening commit history

3 participants