dulwich-users team mailing list archive

[dborowitz@xxxxxxxxxx]

From: Dave Borowitz <dborowitz@xxxxxxxxxx>

Previously, bad refnames were being interpreted by _follow as broken
symlinks and being written through regardless. This caused unexpected
errors on reading refs; now we will fail faster and with a clearer error
message.

Fixes bug #653527.

Change-Id: I74fedb37572413ebdcf360b1512fa7e74bb6d4a7
---
 dulwich/errors.py                |    4 ++++
 dulwich/repo.py                  |    8 ++++++--
 dulwich/tests/test_repository.py |   20 ++++++++------------
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/dulwich/errors.py b/dulwich/errors.py
index 8fec614..bb7ce9b 100644
--- a/dulwich/errors.py
+++ b/dulwich/errors.py
@@ -166,3 +166,7 @@ class NoIndexPresent(Exception):
 
 class CommitError(Exception):
     """An error occurred while performing a commit."""
+
+
+class RefFormatError(Exception):
+    """Indicates an invalid ref name."""
diff --git a/dulwich/repo.py b/dulwich/repo.py
index 571932a..f9ebaa8 100644
--- a/dulwich/repo.py
+++ b/dulwich/repo.py
@@ -35,6 +35,7 @@ from dulwich.errors import (
     NotTagError,
     PackedRefsException,
     CommitError,
+    RefFormatError,
     )
 from dulwich.file import (
     ensure_dir_exists,
@@ -213,7 +214,7 @@ class RefsContainer(object):
         if name == 'HEAD':
             return
         if not name.startswith('refs/') or not check_ref_format(name[5:]):
-            raise KeyError(name)
+            raise RefFormatError(name)
 
     def read_ref(self, refname):
         """Read a reference without following any references.
@@ -990,7 +991,10 @@ class BaseRepo(object):
                 return self.object_store[name]
             except KeyError:
                 pass
-        return self.object_store[self.refs[name]]
+        try:
+            return self.object_store[self.refs[name]]
+        except RefFormatError:
+            raise KeyError(name)
 
     def __contains__(self, name):
         if len(name) in (20, 40):
diff --git a/dulwich/tests/test_repository.py b/dulwich/tests/test_repository.py
index 7cf1cc9..97f1677 100644
--- a/dulwich/tests/test_repository.py
+++ b/dulwich/tests/test_repository.py
@@ -641,18 +641,13 @@ class RefsContainerTests(object):
                          self._refs['refs/heads/symbolic'])
 
     def test_check_refname(self):
-        try:
-            self._refs._check_refname('HEAD')
-        except KeyError:
-            self.fail()
-
-        try:
-            self._refs._check_refname('refs/heads/foo')
-        except KeyError:
-            self.fail()
+        self._refs._check_refname('HEAD')
+        self._refs._check_refname('refs/heads/foo')
 
-        self.assertRaises(KeyError, self._refs._check_refname, 'refs')
-        self.assertRaises(KeyError, self._refs._check_refname, 'notrefs/foo')
+        self.assertRaises(errors.RefFormatError, self._refs._check_refname,
+                          'refs')
+        self.assertRaises(errors.RefFormatError, self._refs._check_refname,
+                          'notrefs/foo')
 
     def test_contains(self):
         self.assertTrue('refs/heads/master' in self._refs)
@@ -777,7 +772,8 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
         self.assertEquals(
           ('refs/heads/master', '42d06bd4b77fed026b154d16493e5deab78f02ec'),
           self._refs._follow('refs/heads/master'))
-        self.assertRaises(KeyError, self._refs._follow, 'notrefs/foo')
+        self.assertRaises(errors.RefFormatError, self._refs._follow,
+                          'notrefs/foo')
         self.assertRaises(KeyError, self._refs._follow, 'refs/heads/loop')
 
     def test_delitem(self):
-- 
1.7.3.2.168.gd6b63