# HG changeset patch
# User Mateusz Kwapich <mitrandir@fb.com>
# Date 1458691511 25200
#      Tue Mar 22 17:05:11 2016 -0700
# Branch stable
# Node ID cdda7b96afff3433eafdeeb83ded83a5b25b7a5b
# Parent  197eed39e3d5e9a8cadfd9ba5839eb14cc265caa
convert: rewrite calls to Git to use the new shelling mechanism (SEC)

CVE-2016-3069 (2/5)

One test output changed because we were ignoring git return code in numcommits
before.

---
 hgext/convert/git.py     |   51 ++++++++++++++++++++++++-----------------------
 tests/test-convert-git.t |    2 -
 2 files changed, 28 insertions(+), 25 deletions(-)

--- a/hgext/convert/git.py
+++ b/hgext/convert/git.py
@@ -102,10 +102,10 @@ class convert_git(converter_source, comm
 
     def getheads(self):
         if not self.rev:
-            heads, ret = self.gitread('git rev-parse --branches --remotes')
-            heads = heads.splitlines()
+            output, ret = self.gitrun('rev-parse', '--branches', '--remotes')
+            heads = output.splitlines()
         else:
-            heads, ret = self.gitread("git rev-parse --verify %s" % self.rev)
+            heads, ret = self.gitrun('rev-parse', '--verify', self.rev)
             heads = [heads[:-1]]
         if ret:
             raise util.Abort(_('cannot retrieve git heads'))
@@ -114,7 +114,7 @@ class convert_git(converter_source, comm
     def catfile(self, rev, type):
         if rev == hex(nullid):
             raise IOError
-        data, ret = self.gitread("git cat-file %s %s" % (type, rev))
+        data, ret = self.gitrun('cat-file', type, rev)
         if ret:
             raise util.Abort(_('cannot read %r object at %s') % (type, rev))
         return data
@@ -153,25 +153,28 @@ class convert_git(converter_source, comm
                 self.submodules.append(submodule(s['path'], '', s['url']))
 
     def retrievegitmodules(self, version):
-        modules, ret = self.gitread("git show %s:%s" % (version, '.gitmodules'))
+        modules, ret = self.gitrun('show', '%s:%s' % (version, '.gitmodules'))
         if ret:
             raise util.Abort(_('cannot read submodules config file in %s') %
                              version)
         self.parsegitmodules(modules)
         for m in self.submodules:
-            node, ret = self.gitread("git rev-parse %s:%s" % (version, m.path))
+            node, ret = self.gitrun('rev-parse', '%s:%s' % (version, m.path))
             if ret:
                 continue
             m.node = node.strip()
 
     def getchanges(self, version):
         self.modecache = {}
-        fh = self.gitopen("git diff-tree -z --root -m -r %s" % version)
+        cmd = ['diff-tree','-z', '--root', '-m', '-r'] + [version]
+        output, status = self.gitrun(*cmd)
+        if status:
+            raise util.Abort(_('cannot read changes in %s') % version)
         changes = []
         seen = set()
         entry = None
         subexists = False
-        for l in fh.read().split('\x00'):
+        for l in output.split('\x00'):
             if not entry:
                 if not l.startswith(':'):
                     continue
@@ -194,8 +197,6 @@ class convert_git(converter_source, comm
                     self.modecache[(f, h)] = (p and "x") or (s and "l") or ""
                     changes.append((f, h))
             entry = None
-        if fh.close():
-            raise util.Abort(_('cannot read changes in %s') % version)
 
         if subexists:
             self.retrievegitmodules(version)
@@ -240,12 +241,14 @@ class convert_git(converter_source, comm
     def gettags(self):
         tags = {}
         alltags = {}
-        fh = self.gitopen('git ls-remote --tags "%s"' % self.path,
-                          err=subprocess.STDOUT)
+        output, status = self.gitrunlines('ls-remote', '--tags', self.path)
+
+        if status:
+            raise util.Abort(_('cannot read tags from %s') % self.path)
         prefix = 'refs/tags/'
 
         # Build complete list of tags, both annotated and bare ones
-        for line in fh:
+        for line in output:
             line = line.strip()
             if line.startswith("error:") or line.startswith("fatal:"):
                 raise util.Abort(_('cannot read tags from %s') % self.path)
@@ -253,8 +256,6 @@ class convert_git(converter_source, comm
             if not tag.startswith(prefix):
                 continue
             alltags[tag[len(prefix):]] = node
-        if fh.close():
-            raise util.Abort(_('cannot read tags from %s') % self.path)
 
         # Filter out tag objects for annotated tag refs
         for tag in alltags:
@@ -271,18 +272,20 @@ class convert_git(converter_source, comm
     def getchangedfiles(self, version, i):
         changes = []
         if i is None:
-            fh = self.gitopen("git diff-tree --root -m -r %s" % version)
-            for l in fh:
+            output, status = self.gitrunlines('diff-tree', '--root', '-m',
+                                              '-r', version)
+            if status:
+                raise util.Abort(_('cannot read changes in %s') % version)
+            for l in output:
                 if "\t" not in l:
                     continue
                 m, f = l[:-1].split("\t")
                 changes.append(f)
         else:
-            fh = self.gitopen('git diff-tree --name-only --root -r %s '
-                              '"%s^%s" --' % (version, version, i + 1))
-            changes = [f.rstrip('\n') for f in fh]
-        if fh.close():
-            raise util.Abort(_('cannot read changes in %s') % version)
+            output, status = self.gitrunlines('diff-tree', '--name-only',
+                                              '--root', '-r', version,
+                                              '%s^%s' % (version, i + 1), '--')
+            changes = [f.rstrip('\n') for f in output]
 
         return changes
 
@@ -302,8 +305,8 @@ class convert_git(converter_source, comm
         ])
 
         try:
-            fh = self.gitopen('git show-ref', err=subprocess.PIPE)
-            for line in fh:
+            output, status = self.gitrunlines('show-ref')
+            for line in output:
                 line = line.strip()
                 rev, name = line.split(None, 1)
                 # Process each type of branch
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -421,7 +421,7 @@ damage git repository by renaming a comm
   $ COMMIT_OBJ=1c/0ce3c5886f83a1d78a7b517cdff5cf9ca17bdd
   $ mv git-repo4/.git/objects/$COMMIT_OBJ git-repo4/.git/objects/$COMMIT_OBJ.tmp
   $ hg convert git-repo4 git-repo4-broken-hg 2>&1 | grep 'abort:'
-  abort: cannot read tags from git-repo4/.git
+  abort: cannot retrieve number of commits in git-repo4/.git
   $ mv git-repo4/.git/objects/$COMMIT_OBJ.tmp git-repo4/.git/objects/$COMMIT_OBJ
 damage git repository by renaming a blob object
 
