Преглед изворни кода

Fix DictRefsContainer.set_if_equals() to only update requested ref

The set_if_equals() method was incorrectly updating all refs in a
symbolic reference chain instead of just the requested ref. This was
because it used follow() to get all refs in the chain and then updated
all of them.

This caused issues where updating a symbolic ref would incorrectly
modify the ref it pointed to.
Jelmer Vernooij пре 1 месец
родитељ
комит
5095e574d7
2 измењених фајлова са 51 додато и 15 уклоњено
  1. 14 15
      dulwich/refs.py
  2. 37 0
      tests/test_refs.py

+ 14 - 15
dulwich/refs.py

@@ -507,21 +507,20 @@ class DictRefsContainer(RefsContainer):
     ) -> bool:
         if old_ref is not None and self._refs.get(name, ZERO_SHA) != old_ref:
             return False
-        realnames, _ = self.follow(name)
-        for realname in realnames:
-            self._check_refname(realname)
-            old = self._refs.get(realname)
-            self._refs[realname] = new_ref
-            self._notify(realname, new_ref)
-            self._log(
-                realname,
-                old,
-                new_ref,
-                committer=committer,
-                timestamp=timestamp,
-                timezone=timezone,
-                message=message,
-            )
+        # Only update the specific ref requested, not the whole chain
+        self._check_refname(name)
+        old = self._refs.get(name)
+        self._refs[name] = new_ref
+        self._notify(name, new_ref)
+        self._log(
+            name,
+            old,
+            new_ref,
+            committer=committer,
+            timestamp=timestamp,
+            timezone=timezone,
+            message=message,
+        )
         return True
 
     def add_if_new(

+ 37 - 0
tests/test_refs.py

@@ -381,6 +381,43 @@ class DictRefsContainerTests(RefsContainerTests, TestCase):
         expected_refs[b"refs/stash"] = b"00" * 20
         self.assertEqual(expected_refs, self._refs.as_dict())
 
+    def test_set_if_equals_with_symbolic_ref(self) -> None:
+        # Test that set_if_equals only updates the requested ref,
+        # not all refs in a symbolic reference chain
+
+        # The bug in the original implementation was that when follow()
+        # was called on a ref, it would return all refs in the chain,
+        # and set_if_equals would update ALL of them instead of just the
+        # requested ref.
+
+        # Set up refs
+        master_sha = b"1" * 40
+        feature_sha = b"2" * 40
+        new_sha = b"3" * 40
+
+        self._refs[b"refs/heads/master"] = master_sha
+        self._refs[b"refs/heads/feature"] = feature_sha
+        # Create a second symbolic ref pointing to feature
+        self._refs.set_symbolic_ref(b"refs/heads/other", b"refs/heads/feature")
+
+        # Update refs/heads/other through set_if_equals
+        # With the bug, this would update BOTH refs/heads/other AND refs/heads/feature
+        # Without the bug, only refs/heads/other should be updated
+        # Note: old_ref needs to be the actual stored value (the symref)
+        self.assertTrue(
+            self._refs.set_if_equals(
+                b"refs/heads/other", b"ref: refs/heads/feature", new_sha
+            )
+        )
+
+        # refs/heads/other should now directly point to new_sha
+        self.assertEqual(self._refs.read_ref(b"refs/heads/other"), new_sha)
+
+        # refs/heads/feature should remain unchanged
+        # With the bug, refs/heads/feature would also be incorrectly updated to new_sha
+        self.assertEqual(self._refs[b"refs/heads/feature"], feature_sha)
+        self.assertEqual(self._refs[b"refs/heads/master"], master_sha)
+
 
 class DiskRefsContainerTests(RefsContainerTests, TestCase):
     def setUp(self) -> None: