Fixes #724 - catastrophic backtracking regex

Reworks branch parsing to use for-each-ref

src/git/git.ts

@@ -8,7 +8,7 @@ import { Logger } from '../logger';
 import { Objects, Strings } from '../system';
 import { findGitPath, GitLocation } from './locator';
 import { run, RunOptions } from './shell';
-import { GitLogParser, GitStashParser } from './parsers/parsers';
+import { GitBranchParser, GitLogParser, GitStashParser } from './parsers/parsers';
 import { GitFileStatus } from './models/file';
 
 export { GitLocation } from './locator';
@@ -391,12 +391,12 @@ export class Git {
     }
 
     static branch(repoPath: string, options: { all: boolean } = { all: false }) {
-        const params = ['branch', '-vv', '--abbrev=40'];
+        const params = ['for-each-ref', `--format=${GitBranchParser.defaultFormat}`, 'refs/heads'];
         if (options.all) {
-            params.push('-a');
+            params.push('refs/remotes');
         }
 
-        return git<string>({ cwd: repoPath, configs: ['-c', 'color.branch=false'] }, ...params);
+        return git<string>({ cwd: repoPath }, ...params);
     }
 
     static branch_contains(repoPath: string, ref: string, options: { remote: boolean } = { remote: false }) {

src/git/gitService.ts

@@ -1088,15 +1088,18 @@ export class GitService implements Disposable {
         if (data === undefined) return undefined;
 
         const branch = data[0].split('\n');
-        return new GitBranch(repoPath, branch[0], true, data[1], branch[1]);
+        return new GitBranch(repoPath, branch[0], false, true, data[1], branch[1]);
     }
 
     @log()
     async getBranches(repoPath: string | undefined): Promise<GitBranch[]> {
         if (repoPath === undefined) return [];
 
-        let branches = this._branchesCache.get(repoPath);
-        if (branches !== undefined) return branches;
+        let branches;
+        if (this.useCaching) {
+            branches = this._branchesCache.get(repoPath);
+            if (branches !== undefined) return branches;
+        }
 
         const data = await Git.branch(repoPath, { all: true });
         // 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 {
             branches = current !== undefined ? [current] : [];
         }
         else {
-            branches = GitBranchParser.parse(data, repoPath) || [];
+            branches = GitBranchParser.parse(data, repoPath);
         }
 
-        const repo = await this.getRepository(repoPath);
-        if (repo !== undefined && repo.supportsChangeEvents) {
-            this._branchesCache.set(repoPath, branches);
+        if (this.useCaching) {
+            const repo = await this.getRepository(repoPath);
+            if (repo !== undefined && repo.supportsChangeEvents) {
+                this._branchesCache.set(repoPath, branches);
+            }
         }
         return branches;
     }

src/git/models/branch.ts

@@ -12,38 +12,26 @@ export interface GitTrackingState {
 export class GitBranch {
     readonly detached: boolean;
     readonly id: string;
-    readonly name: string;
-    readonly remote: boolean;
     readonly tracking?: string;
     readonly state: GitTrackingState;
 
     constructor(
         public readonly repoPath: string,
-        name: string,
-        public readonly current: boolean = false,
+        public readonly name: string,
+        public readonly remote: boolean,
+        public readonly current: boolean,
         public readonly sha?: string,
         tracking?: string,
         ahead: number = 0,
         behind: number = 0,
         detached: boolean = false
     ) {
-        this.id = `${repoPath}|${name}`;
-
-        if (name.startsWith('remotes/')) {
-            name = name.substring(8);
-            this.remote = true;
-        }
-        else {
-            this.remote = false;
-        }
+        this.id = `${repoPath}|${remote ? 'remotes/' : 'heads/'}${name}`;
 
         this.detached = detached || (this.current ? GitBranch.isDetached(name) : false);
         if (this.detached) {
             this.name = GitBranch.formatDetached(this.sha!);
         }
-        else {
-            this.name = name;
-        }
 
         this.tracking = tracking == null || tracking.length === 0 ? undefined : tracking;
         this.state = {

src/git/parsers/branchParser.ts

@@ -1,58 +1,86 @@
 'use strict';
-import { GitBranch } from '../git';
+import { GitBranch } from '../models/branch';
+import { debug } from '../../system';
 
-const branchWithTrackingRegex = /^(\*?)\s+(.+?)\s+([0-9,a-f]+)\s+(?:\[(.*?\/.*?)(?::\s(.*)\]|\]))?/gm;
-const branchWithTrackingStateRegex = /^(?:ahead\s([0-9]+))?[,\s]*(?:behind\s([0-9]+))?/;
+const branchWithTrackingRegex = /^<h>(.+)<n>(.+)<u>(.*)<t>(?:\[(?:ahead ([0-9]+))?[,\s]*(?:behind ([0-9]+))?]|\[gone])?<r>(.*)$/gm;
+
+// Using %x00 codes because some shells seem to try to expand things if not
+const lb = '%3c'; // `%${'<'.charCodeAt(0).toString(16)}`;
+const rb = '%3e'; // `%${'>'.charCodeAt(0).toString(16)}`;
 
 export class GitBranchParser {
-    static parse(data: string, repoPath: string): GitBranch[] | undefined {
-        if (!data) return undefined;
+    static defaultFormat = [
+        `${lb}h${rb}%(HEAD)`, // HEAD indicator
+        `${lb}n${rb}%(refname:lstrip=1)`, // branch name
+        `${lb}u${rb}%(upstream:short)`, // branch upstream
+        `${lb}t${rb}%(upstream:track)`, // branch upstream tracking state
+        `${lb}r${rb}%(objectname)` // ref
+    ].join('');
 
+    @debug({ args: false })
+    static parse(data: string, repoPath: string): GitBranch[] {
         const branches: GitBranch[] = [];
 
+        if (!data) return branches;
+
         let match: RegExpExecArray | null;
         let ahead;
+        let aheadStr;
         let behind;
+        let behindStr;
         let current;
         let name;
-        let sha;
-        let state;
+        let ref;
+        let remote;
         let tracking;
         do {
             match = branchWithTrackingRegex.exec(data);
             if (match == null) break;
 
-            [, current, name, sha, tracking, state] = match;
-            [ahead, behind] = this.parseState(state);
+            [, current, name, tracking, aheadStr, behindStr, ref] = match;
+            if (aheadStr !== undefined && aheadStr.length !== 0) {
+                ahead = parseInt(aheadStr, 10);
+                ahead = isNaN(ahead) ? 0 : ahead;
+            }
+            else {
+                ahead = 0;
+            }
+
+            if (behindStr !== undefined && behindStr.length !== 0) {
+                behind = parseInt(behindStr, 10);
+                behind = isNaN(behind) ? 0 : behind;
+            }
+            else {
+                behind = 0;
+            }
+
+            if (name.startsWith('remotes/')) {
+                // Strip off remotes/
+                name = name.substr(8);
+                remote = true;
+            }
+            else {
+                // Strip off heads/
+                name = name.substr(6);
+                remote = false;
+            }
+
             branches.push(
                 new GitBranch(
                     repoPath,
+                    name,
+                    remote,
+                    current.charCodeAt(0) === 42, // '*',
                     // Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
-                    ` ${name}`.substr(1),
-                    current === '*',
-                    // Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
-                    sha === undefined ? undefined : ` ${sha}`.substr(1),
+                    ref === undefined || ref.length === 0 ? undefined : ` ${ref}`.substr(1),
                     // Stops excessive memory usage -- https://bugs.chromium.org/p/v8/issues/detail?id=2869
-                    tracking === undefined ? undefined : ` ${tracking}`.substr(1),
+                    tracking === undefined || tracking.length === 0 ? undefined : ` ${tracking}`.substr(1),
                     ahead,
                     behind
                 )
             );
         } while (match != null);
 
-        if (!branches.length) return undefined;
-
         return branches;
     }
-
-    static parseState(state: string): [number, number] {
-        if (state == null) return [0, 0];
-
-        const match = branchWithTrackingStateRegex.exec(state);
-        if (match == null) return [0, 0];
-
-        const ahead = parseInt(match[1], 10);
-        const behind = parseInt(match[2], 10);
-        return [isNaN(ahead) ? 0 : ahead, isNaN(behind) ? 0 : behind];
-    }
 }

src/git/parsers/logParser.ts

@@ -102,7 +102,7 @@ export class GitLogParser {
             // Since log --reverse doesn't properly honor a max count -- enforce it here
             if (reverse && maxCount && i >= maxCount) break;
 
-            // <<1-char token>> <data>
+            // <1-char token> data
             // e.g. <r> bd1452a2dc
             token = line.charCodeAt(1);
 

src/views/nodes/repositoryNode.ts

@@ -50,6 +50,7 @@ export class RepositoryNode extends SubscribeableViewNode<RepositoriesView> {
                 const branch = new GitBranch(
                     status.repoPath,
                     status.branch,
+                    false,
                     true,
                     status.sha,
                     status.upstream,