From 565b1780c682d02bd13e76741b8fa3f8b4f23c46 Mon Sep 17 00:00:00 2001
From: "borenet@google.com"
 <borenet@google.com@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 11 Jun 2014 20:37:30 +0000
Subject: [PATCH] scm.GIT.IsValidRevision: Only return True if the given object
 is a commit

BUG=383476

Review URL: https://codereview.chromium.org/329823002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@276487 0039d316-1c4b-4281-b951-d872f2087c98
---
 scm.py                |  9 ++++++---
 tests/scm_unittest.py | 12 ++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/scm.py b/scm.py
index eb5c52403..d60453dce 100644
--- a/scm.py
+++ b/scm.py
@@ -491,12 +491,15 @@ class GIT(object):
       sha = GIT.Capture(['rev-parse', lookup_rev], cwd=cwd).lower()
       if lookup_rev != rev:
         # Make sure we get the original 40 chars back.
-        return rev.lower() == sha
+        if rev.lower() != sha:
+          return False
       if sha_only:
-        return sha.startswith(rev.lower())
-      return True
+        if not sha.startswith(rev.lower()):
+          return False
     except subprocess2.CalledProcessError:
       return False
+    obj_type = GIT.Capture(['cat-file', '-t', rev], cwd=cwd).strip()
+    return obj_type == 'commit'
 
   @classmethod
   def AssertVersion(cls, min_version):
diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py
index 4e97c73ee..0b03152e9 100755
--- a/tests/scm_unittest.py
+++ b/tests/scm_unittest.py
@@ -8,6 +8,7 @@
 import logging
 import os
 import sys
+import tempfile
 import unittest
 
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
@@ -141,6 +142,17 @@ class RealGitTest(fake_repos.FakeReposTestBase):
     self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev=first_rev))
     self.assertTrue(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev='HEAD'))
 
+    # Verify that IsValidRevision returns False for non-commit objects.
+    tmp = tempfile.NamedTemporaryFile(delete=False)
+    try:
+      tmp.write('This is not a commit')
+      tmp.close()
+      hashval = scm.GIT.Capture(['hash-object', '-w', tmp.name],
+                                cwd=self.clone_dir)
+    finally:
+      os.remove(tmp.name)
+    self.assertFalse(scm.GIT.IsValidRevision(cwd=self.clone_dir, rev=hashval))
+
 
 class RealGitSvnTest(fake_repos.FakeReposTestBase):
   def setUp(self):