From 87e19d2bf4f8e856c260adcc0f5a4d97ca155d72 Mon Sep 17 00:00:00 2001
From: Red4Ru <39802734+Red4Ru@users.noreply.github.com>
Date: Wed, 13 Nov 2024 11:20:38 +0300
Subject: [PATCH] gh-104745: Limit starting a patcher more than once without
 stopping it (GH-126649)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, this would cause an `AttributeError` if the patch stopped more than once after this, and would also disrupt the original patched object.

---------

(cherry picked from commit 1e40c5ba47780ddd91868abb3aa064f5ba3015e4)

Co-authored-by: Red4Ru <39802734+Red4Ru@users.noreply.github.com>
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
---
 Lib/test/test_unittest/testmock/testpatch.py  | 52 ++++++++++++++++++-
 Lib/unittest/mock.py                          |  9 ++++
 ...-11-10-18-14-51.gh-issue-104745.zAa5Ke.rst |  3 ++
 3 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Library/2024-11-10-18-14-51.gh-issue-104745.zAa5Ke.rst

diff --git a/Lib/test/test_unittest/testmock/testpatch.py b/Lib/test/test_unittest/testmock/testpatch.py
index f26e74ce0bc1ba..037c021e6eafcf 100644
--- a/Lib/test/test_unittest/testmock/testpatch.py
+++ b/Lib/test/test_unittest/testmock/testpatch.py
@@ -745,6 +745,54 @@ def test_stop_idempotent(self):
         self.assertIsNone(patcher.stop())
 
 
+    def test_exit_idempotent(self):
+        patcher = patch(foo_name, 'bar', 3)
+        with patcher:
+            patcher.stop()
+
+
+    def test_second_start_failure(self):
+        patcher = patch(foo_name, 'bar', 3)
+        patcher.start()
+        try:
+            self.assertRaises(RuntimeError, patcher.start)
+        finally:
+            patcher.stop()
+
+
+    def test_second_enter_failure(self):
+        patcher = patch(foo_name, 'bar', 3)
+        with patcher:
+            self.assertRaises(RuntimeError, patcher.start)
+
+
+    def test_second_start_after_stop(self):
+        patcher = patch(foo_name, 'bar', 3)
+        patcher.start()
+        patcher.stop()
+        patcher.start()
+        patcher.stop()
+
+
+    def test_property_setters(self):
+        mock_object = Mock()
+        mock_bar = mock_object.bar
+        patcher = patch.object(mock_object, 'bar', 'x')
+        with patcher:
+            self.assertEqual(patcher.is_local, False)
+            self.assertIs(patcher.target, mock_object)
+            self.assertEqual(patcher.temp_original, mock_bar)
+            patcher.is_local = True
+            patcher.target = mock_bar
+            patcher.temp_original = mock_object
+            self.assertEqual(patcher.is_local, True)
+            self.assertIs(patcher.target, mock_bar)
+            self.assertEqual(patcher.temp_original, mock_object)
+        # if changes are left intact, they may lead to disruption as shown below (it might be what someone needs though)
+        self.assertEqual(mock_bar.bar, mock_object)
+        self.assertEqual(mock_object.bar, 'x')
+
+
     def test_patchobject_start_stop(self):
         original = something
         patcher = patch.object(PTModule, 'something', 'foo')
@@ -1098,7 +1146,7 @@ def test_new_callable_patch(self):
 
         self.assertIsNot(m1, m2)
         for mock in m1, m2:
-            self.assertNotCallable(m1)
+            self.assertNotCallable(mock)
 
 
     def test_new_callable_patch_object(self):
@@ -1111,7 +1159,7 @@ def test_new_callable_patch_object(self):
 
         self.assertIsNot(m1, m2)
         for mock in m1, m2:
-            self.assertNotCallable(m1)
+            self.assertNotCallable(mock)
 
 
     def test_new_callable_keyword_arguments(self):
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 3e9791b22dc450..c4ce8f8a3eb930 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -1329,6 +1329,7 @@ def __init__(
         self.autospec = autospec
         self.kwargs = kwargs
         self.additional_patchers = []
+        self.is_started = False
 
 
     def copy(self):
@@ -1441,6 +1442,9 @@ def get_original(self):
 
     def __enter__(self):
         """Perform the patch."""
+        if self.is_started:
+            raise RuntimeError("Patch is already started")
+
         new, spec, spec_set = self.new, self.spec, self.spec_set
         autospec, kwargs = self.autospec, self.kwargs
         new_callable = self.new_callable
@@ -1572,6 +1576,7 @@ def __enter__(self):
         self.temp_original = original
         self.is_local = local
         self._exit_stack = contextlib.ExitStack()
+        self.is_started = True
         try:
             setattr(self.target, self.attribute, new_attr)
             if self.attribute_name is not None:
@@ -1591,6 +1596,9 @@ def __enter__(self):
 
     def __exit__(self, *exc_info):
         """Undo the patch."""
+        if not self.is_started:
+            return
+
         if self.is_local and self.temp_original is not DEFAULT:
             setattr(self.target, self.attribute, self.temp_original)
         else:
@@ -1607,6 +1615,7 @@ def __exit__(self, *exc_info):
         del self.target
         exit_stack = self._exit_stack
         del self._exit_stack
+        self.is_started = False
         return exit_stack.__exit__(*exc_info)
 
 
diff --git a/Misc/NEWS.d/next/Library/2024-11-10-18-14-51.gh-issue-104745.zAa5Ke.rst b/Misc/NEWS.d/next/Library/2024-11-10-18-14-51.gh-issue-104745.zAa5Ke.rst
new file mode 100644
index 00000000000000..c83a10769820cf
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-11-10-18-14-51.gh-issue-104745.zAa5Ke.rst
@@ -0,0 +1,3 @@
+Limit starting a patcher (from :func:`unittest.mock.patch` or
+:func:`unittest.mock.patch.object`) more than
+once without stopping it