Skip to content

Commit d9e3342

Browse files
eamodioeamodio
committed
Fixes #724 - catastrophic backtracking regex
Reworks branch parsing to use for-each-ref
1 parent 8a64a06 commit d9e3342

File tree

6 files changed

+77
-55
lines changed

6 files changed

+77
-55
lines changed

‎src/git/git.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { Logger } from '../logger';
88
import { Objects, Strings } from '../system';
99
import { findGitPath, GitLocation } from './locator';
1010
import { run, RunOptions } from './shell';
11-
import { GitLogParser, GitStashParser } from './parsers/parsers';
11+
import { GitBranchParser, GitLogParser, GitStashParser } from './parsers/parsers';
1212
import { GitFileStatus } from './models/file';
1313

1414
export { GitLocation } from './locator';
@@ -391,12 +391,12 @@ export class Git {
391391
}
392392

393393
static branch(repoPath: string, options: { all: boolean } = { all: false }) {
394-
const params = ['branch', '-vv', '--abbrev=40'];
394+
const params = ['for-each-ref', `--format=${GitBranchParser.defaultFormat}`, 'refs/heads'];
395395
if (options.all) {
396-
params.push('-a');
396+
params.push('refs/remotes');
397397
}
398398

399-
return git<string>({ cwd: repoPath, configs: ['-c', 'color.branch=false'] }, ...params);
399+
return git<string>({ cwd: repoPath }, ...params);
400400
}
401401

402402
static branch_contains(repoPath: string, ref: string, options: { remote: boolean } = { remote: false }) {

‎src/git/gitService.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,15 +1088,18 @@ export class GitService implements Disposable {
10881088
if (data === undefined) return undefined;
10891089

10901090
const branch = data[0].split('\n');
1091-
return new GitBranch(repoPath, branch[0], true, data[1], branch[1]);
1091+
return new GitBranch(repoPath, branch[0], false, true, data[1], branch[1]);
10921092
}
10931093

10941094
@log()
10951095
async getBranches(repoPath: string | undefined): Promise<GitBranch[]> {
10961096
if (repoPath === undefined) return [];
10971097

1098-
let branches = this._branchesCache.get(repoPath);
1099-
if (branches !== undefined) return branches;
1098+
let branches;
1099+
if (this.useCaching) {
1100+
branches = this._branchesCache.get(repoPath);
1101+
if (branches !== undefined) return branches;
1102+
}
11001103

11011104
const data = await Git.branch(repoPath, { all: true });
11021105
// If we don't get any data, assume the repo doesn't have any commits yet so check if we have a current branch
@@ -1105,12 +1108,14 @@ export class GitService implements Disposable {
11051108
branches = current !== undefined ? [current] : [];
11061109
}
11071110
else {
1108-
branches = GitBranchParser.parse(data, repoPath) || [];
1111+
branches = GitBranchParser.parse(data, repoPath);
11091112
}
11101113

1111-
const repo = await this.getRepository(repoPath);
1112-
if (repo !== undefined && repo.supportsChangeEvents) {
1113-
this._branchesCache.set(repoPath, branches);
1114+
if (this.useCaching) {
1115+
const repo = await this.getRepository(repoPath);
1116+
if (repo !== undefined && repo.supportsChangeEvents) {
1117+
this._branchesCache.set(repoPath, branches);
1118+
}
11141119
}
11151120
return branches;
11161121
}

‎src/git/models/branch.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,38 +12,26 @@ export interface GitTrackingState {
1212
export class GitBranch {
1313
readonly detached: boolean;
1414
readonly id: string;
15-
readonly name: string;
16-
readonly remote: boolean;
1715
readonly tracking?: string;
1816
readonly state: GitTrackingState;
1917

2018
constructor(
2119
public readonly repoPath: string,
22-
name: string,
23-
public readonly current: boolean = false,
20+
public readonly name: string,
21+
public readonly remote: boolean,
22+
public readonly current: boolean,
2423
public readonly sha?: string,
2524
tracking?: string,
2625
ahead: number = 0,
2726
behind: number = 0,
2827
detached: boolean = false
2928
) {
30-
this.id = `${repoPath}|${name}`;
31-
32-
if (name.startsWith('remotes/')) {
33-
name = name.substring(8);
34-
this.remote = true;
35-
}
36-
else {
37-
this.remote = false;
38-
}
29+
this.id = `${repoPath}|${remote ? 'remotes/' : 'heads/'}${name}`;
3930

4031
this.detached = detached || (this.current ? GitBranch.isDetached(name) : false);
4132
if (this.detached) {
4233
this.name = GitBranch.formatDetached(this.sha!);
4334
}
44-
else {
45-
this.name = name;
46-
}
4735

4836
this.tracking = tracking == null || tracking.length === 0 ? undefined : tracking;
4937
this.state = {

‎src/git/parsers/branchParser.ts

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,86 @@
11
'use strict';
2-
import { GitBranch } from '../git';
2+
import { GitBranch } from '../models/branch';
3+
import { debug } from '../../system';
34

4-
const branchWithTrackingRegex = /^(\*?)\s+(.+?)\s+([0-9,a-f]+)\s+(?:\[(.*?\/.*?)(?::\s(.*)\]|\]))?/gm;
5-
const branchWithTrackingStateRegex = /^(?:ahead\s([0-9]+))?[,\s]*(?:behind\s([0-9]+))?/;
5+
const branchWithTrackingRegex = /^<h>(.+)<n>(.+)<u>(.*)<t>(?:\[(?:ahead ([0-9]+))?[,\s]*(?:behind ([0-9]+))?]|\[gone])?<r>(.*)$/gm;
6+
7+
// Using %x00 codes because some shells seem to try to expand things if not
8+
const lb = '%3c'; // `%${'<'.charCodeAt(0).toString(16)}`;
9+
const rb = '%3e'; // `%${'>'.charCodeAt(0).toString(16)}`;
610

711
export class GitBranchParser {
8-
static parse(data: string, repoPath: string): GitBranch[] | undefined {
9-
if (!data) return undefined;
12+
static defaultFormat = [
13+
`${lb}h${rb}%(HEAD)`, // HEAD indicator
14+
`${lb}n${rb}%(refname:lstrip=1)`, // branch name
15+
`${lb}u${rb}%(upstream:short)`, // branch upstream
16+
`${lb}t${rb}%(upstream:track)`, // branch upstream tracking state
17+
`${lb}r${rb}%(objectname)` // ref
18+
].join('');
1019

20+
@debug({ args: false })
21+
static parse(data: string, repoPath: string): GitBranch[] {
1122
const branches: GitBranch[] = [];
1223

24+
if (!data) return branches;
25+
1326
let match: RegExpExecArray | null;
1427
let ahead;
28+
let aheadStr;
1529
let behind;
30+
let behindStr;
1631
let current;
1732
let name;
18-
let sha;
19-
let state;
33+
let ref;
34+
let remote;
2035
let tracking;
2136
do {
2237
match = branchWithTrackingRegex.exec(data);
2338
if (match == null) break;
2439

25-
[, current, name, sha, tracking, state] = match;
26-
[ahead, behind] = this.parseState(state);
40+
[, current, name, tracking, aheadStr, behindStr, ref] = match;
41+
if (aheadStr !== undefined && aheadStr.length !== 0) {
42+
ahead = parseInt(aheadStr, 10);
43+
ahead = isNaN(ahead) ? 0 : ahead;
44+
}
45+
else {
46+
ahead = 0;
47+
}
48+
49+
if (behindStr !== undefined && behindStr.length !== 0) {
50+
behind = parseInt(behindStr, 10);
51+
behind = isNaN(behind) ? 0 : behind;
52+
}
53+
else {
54+
behind = 0;
55+
}
56+
57+
if (name.startsWith('remotes/')) {
58+
// Strip off remotes/
59+
name = name.substr(8);
60+
remote = true;
61+
}
62+
else {
63+
// Strip off heads/
64+
name = name.substr(6);
65+
remote = false;
66+
}
67+
2768
branches.push(
2869
new GitBranch(
2970
repoPath,
71+
name,
72+
remote,
73+
current.charCodeAt(0) === 42, // '*',
3074
// Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
31-
` ${name}`.substr(1),
32-
current === '*',
33-
// Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
34-
sha === undefined ? undefined : ` ${sha}`.substr(1),
75+
ref === undefined || ref.length === 0 ? undefined : ` ${ref}`.substr(1),
3576
// Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
36-
tracking === undefined ? undefined : ` ${tracking}`.substr(1),
77+
tracking === undefined || tracking.length === 0 ? undefined : ` ${tracking}`.substr(1),
3778
ahead,
3879
behind
3980
)
4081
);
4182
} while (match != null);
4283

43-
if (!branches.length) return undefined;
44-
4584
return branches;
4685
}
47-
48-
static parseState(state: string): [number, number] {
49-
if (state == null) return [0, 0];
50-
51-
const match = branchWithTrackingStateRegex.exec(state);
52-
if (match == null) return [0, 0];
53-
54-
const ahead = parseInt(match[1], 10);
55-
const behind = parseInt(match[2], 10);
56-
return [isNaN(ahead) ? 0 : ahead, isNaN(behind) ? 0 : behind];
57-
}
5886
}

‎src/git/parsers/logParser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export class GitLogParser {
102102
// Since log --reverse doesn't properly honor a max count -- enforce it here
103103
if (reverse && maxCount && i >= maxCount) break;
104104

105-
// <<1-char token>> <data>
105+
// <1-char token> data
106106
// e.g. <r> bd1452a2dc
107107
token = line.charCodeAt(1);
108108

‎src/views/nodes/repositoryNode.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export class RepositoryNode extends SubscribeableViewNode<RepositoriesView> {
5050
const branch = new GitBranch(
5151
status.repoPath,
5252
status.branch,
53+
false,
5354
true,
5455
status.sha,
5556
status.upstream,

0 commit comments

Comments
 (0)