From 9309e94a56f3013f98160ee1d52c02b094b36d00 Mon Sep 17 00:00:00 2001 From: vinothksekar <132795481+vinothksekar@users.noreply.github.com> Date: Fri, 3 Jul 2026 07:17:29 +0530 Subject: [PATCH] Fix shared-state/thread-safety bug in Payment.capture() capture() used a mutable default argument (data={}) and mutated it (data['amount'] = amount). The default dict is created once and shared across every call, every Client instance and every thread, so: * state leaks between calls: after capture(id, 4242) with no data, the function's default is permanently {'amount': 4242}; and * on a Client shared across threads (the documented usage) concurrent capture() calls race on the shared dict and can post one payment's amount for another payment. Fixed by using data=None and building a fresh dict per call (also avoids mutating a caller-supplied dict). Removes the # nosemgrep suppression that was hiding the mutable-default warning. Refs #134. Co-Authored-By: Claude Opus 4.8 --- razorpay/resources/payment.py | 3 ++- tests/test_client_payment.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/razorpay/resources/payment.py b/razorpay/resources/payment.py index ee5e3130..2a3cb053 100644 --- a/razorpay/resources/payment.py +++ b/razorpay/resources/payment.py @@ -34,7 +34,7 @@ def fetch(self, payment_id, data={}, **kwargs): """ return super(Payment, self).fetch(payment_id, data, **kwargs) - def capture(self, payment_id, amount, data={}, **kwargs): # nosemgrep : python.lang.correctness.common-mistakes.default-mutable-dict.default-mutable-dict + def capture(self, payment_id, amount, data=None, **kwargs): """ Capture Payment for given Id @@ -46,6 +46,7 @@ def capture(self, payment_id, amount, data={}, **kwargs): # nosemgrep : python.l Payment dict after getting captured """ url = "{}/{}/capture".format(self.base_url, payment_id) + data = dict(data) if data else {} data['amount'] = amount return self.post_url(url, data, **kwargs) diff --git a/tests/test_client_payment.py b/tests/test_client_payment.py index 0440f94f..0fb1bbc0 100644 --- a/tests/test_client_payment.py +++ b/tests/test_client_payment.py @@ -44,6 +44,36 @@ def test_payment_capture(self): self.assertEqual(self.client.payment.capture(self.payment_id, amount=5100), result) + @responses.activate + def test_payment_capture_does_not_leak_state_into_default(self): + # Regression test for issue #134: capture() used a mutable default + # argument (data={}) and mutated it, so the amount from one call + # leaked into the shared default and corrupted later/concurrent calls. + result = mock_file('fake_captured_payment') + url = '{}/{}/capture'.format(self.base_url, self.payment_id) + responses.add(responses.POST, url, status=200, + body=json.dumps(result), match_querystring=True) + + self.client.payment.capture(self.payment_id, amount=5100) + self.client.payment.capture(self.payment_id, amount=9900) + + # Without a data argument, no captured state may persist on the + # function's default between invocations. + default = self.client.payment.capture.__defaults__[0] + self.assertNotIn('amount', default or {}) + + @responses.activate + def test_payment_capture_does_not_mutate_caller_dict(self): + result = mock_file('fake_captured_payment') + url = '{}/{}/capture'.format(self.base_url, self.payment_id) + responses.add(responses.POST, url, status=200, + body=json.dumps(result), match_querystring=True) + + caller_data = {} + self.client.payment.capture(self.payment_id, amount=5100, + data=caller_data) + self.assertEqual(caller_data, {}) + @responses.activate def test_refund_create(self): result = mock_file('fake_refund')