Rev 4548: Check fileids in inventory deltas are not None and are strings. in http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

[Robert Collins]

At http://bazaar.launchpad.net/~lifeless/bzr/apply-inventory-delta

------------------------------------------------------------
revno: 4548
revision-id: robertc at robertcollins.net-20090720051953-rohq200jtq2vyhhj
parent: robertc at robertcollins.net-20090720051304-j6p9eea55kknii25
committer: Robert Collins <robertc at robertcollins.net>
branch nick: apply-inventory-delta
timestamp: Mon 2009-07-20 15:19:53 +1000
message:
  Check fileids in inventory deltas are not None and are strings.
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2009-07-20 04:43:06 +0000
+++ b/bzrlib/dirstate.py	2009-07-20 05:19:53 +0000
@@ -1302,7 +1302,8 @@
             inventory._check_delta_unique_old_paths(
             inventory._check_delta_unique_new_paths(
             inventory._check_delta_ids_match_entry(
-            inventory._check_delta_new_path_entry_both_or_None(delta)))),
+            inventory._check_delta_ids_are_valid(
+            inventory._check_delta_new_path_entry_both_or_None(delta))))),
             reverse=True):
             if (file_id in insertions) or (file_id in removals):
                 raise errors.InconsistentDelta(old_path or new_path, file_id,

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2009-07-20 04:42:36 +0000
+++ b/bzrlib/inventory.py	2009-07-20 05:19:53 +0000
@@ -1132,8 +1132,9 @@
         # facility.
         list(_check_delta_unique_ids(_check_delta_unique_new_paths(
             _check_delta_unique_old_paths(_check_delta_ids_match_entry(
+            _check_delta_ids_are_valid(
             _check_delta_new_path_entry_both_or_None(
-            delta))))))
+            delta)))))))
 
         children = {}
         # Remove all affected items which were in the original inventory,
@@ -1646,6 +1647,8 @@
         inventory_delta = _check_delta_unique_new_paths(inventory_delta)
         # Check for entries that don't match the fileid
         inventory_delta = _check_delta_ids_match_entry(inventory_delta)
+        # Check for nonsense fileids
+        inventory_delta = _check_delta_ids_are_valid(inventory_delta)
         # Check for new_path <-> entry consistency
         inventory_delta = _check_delta_new_path_entry_both_or_None(
             inventory_delta)
@@ -2231,6 +2234,22 @@
         yield item
 
 
+def _check_delta_ids_are_valid(delta):
+    """Decorate a delta and check that the ids in it are valid.
+
+    :return: A generator over delta.
+    """
+    for item in delta:
+        entry = item[3]
+        if item[2] is None:
+            raise errors.InconsistentDelta(item[0] or item[1], item[2],
+                "entry with file_id None %r" % entry)
+        if type(item[2]) != str:
+            raise errors.InconsistentDelta(item[0] or item[1], item[2],
+                "entry with non bytes file_id %r" % entry)
+        yield item
+
+
 def _check_delta_ids_match_entry(delta):
     """Decorate a delta and check that the ids in it match the entry.file_id.
 

=== modified file 'bzrlib/tests/test_inv.py'
--- a/bzrlib/tests/test_inv.py	2009-07-17 02:03:31 +0000
+++ b/bzrlib/tests/test_inv.py	2009-07-20 05:19:53 +0000
@@ -277,6 +277,22 @@
         inv2 = self.get_empty_inventory(inv)
         self.assertEqual([], inv2._make_delta(inv))
 
+    def test_None_file_id(self):
+        inv = self.get_empty_inventory()
+        dir1 = inventory.InventoryDirectory(None, 'dir1', inv.root.file_id)
+        dir1.revision = 'result'
+        delta = [(None, u'dir1', None, dir1)]
+        self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+            inv, delta)
+
+    def test_unicode_file_id(self):
+        inv = self.get_empty_inventory()
+        dir1 = inventory.InventoryDirectory(u'dirid', 'dir1', inv.root.file_id)
+        dir1.revision = 'result'
+        delta = [(None, u'dir1', dir1.file_id, dir1)]
+        self.assertRaises(errors.InconsistentDelta, self.apply_delta, self,
+            inv, delta)
+
     def test_repeated_file_id(self):
         inv = self.get_empty_inventory()
         file1 = inventory.InventoryFile('id', 'path1', inv.root.file_id)