Rev 2502: Test that we properly verify the size and position strings. in http://bzr.arbash-meinel.com/branches/bzr/0.17-dev/knit_index_pyrex

John Arbash Meinel john at arbash-meinel.com
Fri Jun 29 17:00:20 BST 2007


At http://bzr.arbash-meinel.com/branches/bzr/0.17-dev/knit_index_pyrex

------------------------------------------------------------
revno: 2502
revision-id: john at arbash-meinel.com-20070629160006-sdhws6bdttdbgua8
parent: john at arbash-meinel.com-20070629154156-qqtkp5u94qiwl5ua
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: knit_index_pyrex
timestamp: Fri 2007-06-29 11:00:06 -0500
message:
  Test that we properly verify the size and position strings.
modified:
  bzrlib/_knit_load_data_c.pyx   knit_c.pyx-20070509143944-u42gy8w387a10m0j-1
  bzrlib/_knit_load_data_py.py   _knit_load_data_py.p-20070629000948-9a0nh4s118bi5y8n-1
  bzrlib/tests/test_knit.py      test_knit.py-20051212171302-95d4c00dd5f11f2b
-------------- next part --------------
=== modified file 'bzrlib/_knit_load_data_c.pyx'
--- a/bzrlib/_knit_load_data_c.pyx	2007-06-29 15:39:01 +0000
+++ b/bzrlib/_knit_load_data_c.pyx	2007-06-29 16:00:06 +0000
@@ -60,6 +60,31 @@
     void *memchr(void *s, int c, size_t n)
 
 
+cdef int string_to_int_safe(char *s, char *end, int *out) except -1:
+    """Convert a base10 string to an integer.
+
+    This makes sure the whole string is consumed, or it raises ValueError.
+    This is similar to how int(s) works, except you don't need a Python
+    String object.
+
+    :param s: The string to convert
+    :param end: The character after the integer. So if the string is '12\0',
+        this should be pointing at the '\0'. If the string was '12 ' then this
+        should point at the ' '.
+    :param out: This is the integer that will be returned
+    :return: -1 if an exception is raised. 0 otherwise
+    """
+    cdef char *integer_end
+
+    # We can't just return the integer because of how pyrex determines when
+    # there is an exception.
+    out[0] = strtol(s, &integer_end, 10)
+    if integer_end != end:
+        py_s = PyString_FromStringAndSize(s, end-s)
+        raise ValueError('%r is not a valid integer' % (py_s,))
+    return 0
+
+
 cdef class KnitIndexReader:
 
     cdef object kndx
@@ -149,27 +174,12 @@
                                                     next - parent_str)
             else:
                 # This in an integer mapping to original
-                # TODO: ensure that we are actually parsing the string
-                int_parent = strtol(parent_str, &parent_end, 10)
+                string_to_int_safe(parent_str, next, &int_parent)
 
-                # Can the parent be decoded to get its parent row? This
-                # at a minimum will cause this row to have wrong parents, or
-                # even to apply a delta to the wrong base and decode
-                # incorrectly. its therefore not usable, and because we have
-                # encountered a situation where a new knit index had this
-                # corrupt we can't asssume that no other rows referring to the
-                # index of this record actually mean the subsequent uncorrupt
-                # one, so we error.
                 if int_parent >= self.history_len:
                     raise IndexError('Parent index refers to a revision which'
                         ' does not exist yet.'
                         ' %d > %d' % (int_parent, self.history_len))
-                if parent_end < next:
-                    # We didn't process all of the string, which means it isn't
-                    # a complete integer.
-                    py_parent = PyString_FromStringAndSize(parent_str,
-                                                           next - parent_str)
-                    raise ValueError('%r is not a valid integer' % (py_parent,))
                 parent = PyList_GET_ITEM(self.history, int_parent)
                 # PyList_GET_ITEM steals a reference
                 Py_INCREF(parent)
@@ -213,7 +223,6 @@
             return 0
         size_str = size_str + 1
 
-        # TODO: Make sure this works when there are no parents
         parent_str = <char*>memchr(size_str, c' ', end - size_str)
         if parent_str == NULL or parent_str >= end:
             # Missing parents
@@ -224,11 +233,9 @@
                                                 version_id_size)
         options = self.process_options(option_str, option_end)
 
-        # TODO: Check that we are actually reading integers
-        pos = strtol(pos_str, NULL, 10)
-        size = strtol(size_str, NULL, 10)
-
         try:
+            string_to_int_safe(pos_str, size_str - 1, &pos)
+            string_to_int_safe(size_str, parent_str - 1, &size)
             parents = self.process_parents(parent_str, end)
         except (ValueError, IndexError), e:
             py_line = PyString_FromStringAndSize(start, end - start)

=== modified file 'bzrlib/_knit_load_data_py.py'
--- a/bzrlib/_knit_load_data_py.py	2007-06-29 00:31:00 +0000
+++ b/bzrlib/_knit_load_data_py.py	2007-06-29 16:00:06 +0000
@@ -64,6 +64,18 @@
 
         version_id, options, pos, size = rec[:4]
         version_id = version_id
+        try:
+            pos = int(pos)
+        except ValueError, e:
+            raise errors.KnitCorrupt(kndx._filename,
+                                     "invalid position on line %r: %s"
+                                     % (rec, e))
+        try:
+            size = int(size)
+        except ValueError, e:
+            raise errors.KnitCorrupt(kndx._filename,
+                                     "invalid size on line %r: %s"
+                                     % (rec, e))
 
         # See kndx._cache_version
         # only want the _history index to reference the 1st
@@ -76,8 +88,8 @@
             index = cache[version_id][5]
         cache[version_id] = (version_id,
                              options.split(','),
-                             int(pos),
-                             int(size),
+                             pos,
+                             size,
                              parents,
                              index)
         # end kndx._cache_version

=== modified file 'bzrlib/tests/test_knit.py'
--- a/bzrlib/tests/test_knit.py	2007-06-29 15:41:56 +0000
+++ b/bzrlib/tests/test_knit.py	2007-06-29 16:00:06 +0000
@@ -753,6 +753,38 @@
                                   ' raising new style exceptions with python'
                                   ' >=2.5')
 
+    def test_invalid_position(self):
+        transport = MockTransport([
+            _KnitIndex.HEADER,
+            "a option 1v 1 :",
+            ])
+        try:
+            self.assertRaises(errors.KnitCorrupt,
+                              self.get_knit_index, transport, 'filename', 'r')
+        except TypeError, e:
+            if (str(e) == ('exceptions must be strings, classes, or instances,'
+                           ' not exceptions.ValueError')
+                and sys.version_info[0:2] >= (2,5)):
+                self.knownFailure('Pyrex <0.9.5 fails with TypeError when'
+                                  ' raising new style exceptions with python'
+                                  ' >=2.5')
+
+    def test_invalid_size(self):
+        transport = MockTransport([
+            _KnitIndex.HEADER,
+            "a option 1 1v :",
+            ])
+        try:
+            self.assertRaises(errors.KnitCorrupt,
+                              self.get_knit_index, transport, 'filename', 'r')
+        except TypeError, e:
+            if (str(e) == ('exceptions must be strings, classes, or instances,'
+                           ' not exceptions.ValueError')
+                and sys.version_info[0:2] >= (2,5)):
+                self.knownFailure('Pyrex <0.9.5 fails with TypeError when'
+                                  ' raising new style exceptions with python'
+                                  ' >=2.5')
+
 
 class LowLevelKnitIndexTests_c(LowLevelKnitIndexTests):
 



More information about the bazaar-commits mailing list