From f63b26552e1bc4e384d94e3d79b5170f0c75adc3 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Wed, 18 Oct 2023 13:18:00 -0700 Subject: [PATCH 1/4] APIRequestor: don't mutate incoming multipart headers --- stripe/api_requestor.py | 16 ++++++++++------ tests/test_api_requestor.py | 1 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/stripe/api_requestor.py b/stripe/api_requestor.py index 3070ba062..33a39776e 100644 --- a/stripe/api_requestor.py +++ b/stripe/api_requestor.py @@ -3,7 +3,7 @@ import json import platform import time -from typing import Optional, Tuple +from typing import Dict, Optional, Tuple import uuid import warnings from collections import OrderedDict @@ -296,6 +296,10 @@ def request_raw( Mechanism for issuing an API call """ + supplied_headers_dict: Optional[Dict[str, str]] = ( + dict(supplied_headers) if supplied_headers is not None else None + ) + if self.api_key: my_api_key = self.api_key else: @@ -327,14 +331,14 @@ def request_raw( post_data = None elif method == "post": if ( - supplied_headers is not None - and supplied_headers.get("Content-Type") + supplied_headers_dict is not None + and supplied_headers_dict.get("Content-Type") == "multipart/form-data" ): generator = MultipartDataGenerator() generator.add_params(params or {}) post_data = generator.get_post_data() - supplied_headers[ + supplied_headers_dict[ "Content-Type" ] = "multipart/form-data; boundary=%s" % (generator.boundary,) else: @@ -347,8 +351,8 @@ def request_raw( ) headers = self.request_headers(my_api_key, method) - if supplied_headers is not None: - for key, value in supplied_headers.items(): + if supplied_headers_dict is not None: + for key, value in supplied_headers_dict.items(): headers[key] = value util.log_info("Request to Stripe api", method=method, path=abs_url) diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index 240359f21..0e8fb501d 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -761,6 +761,7 @@ def test_raw_request_with_file_param(self, requestor, mock_response): supplied_headers = {"Content-Type": "multipart/form-data"} mock_response("{}", 200) requestor.request("post", "/v1/files", params, supplied_headers) + assert supplied_headers["Content-Type"] == "multipart/form-data" class TestDefaultClient(object): From e432959a816851f09456cb6ebd0ed9bc61ad6cd6 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Wed, 18 Oct 2023 14:37:51 -0700 Subject: [PATCH 2/4] Fix file tests --- tests/api_resources/test_file.py | 26 +++++++---- tests/api_resources/test_file_upload.py | 57 ------------------------- 2 files changed, 17 insertions(+), 66 deletions(-) delete mode 100644 tests/api_resources/test_file_upload.py diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index ebfb13aa2..c781dcedd 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -28,7 +28,11 @@ def test_is_retrievable(self, request_mock): request_mock.assert_requested("get", "/v1/files/%s" % TEST_RESOURCE_ID) assert isinstance(resource, stripe.File) - def test_is_creatable(self, setup_upload_api_base, request_mock): + def test_is_creatable(self, mocker): + hc = mocker.Mock(stripe.http_client.HTTPClient) + hc.name = "mockclient" + stripe.default_http_client = hc + hc.request_with_retries.return_value = ("{}", 200, {}) stripe.multipart_data_generator.MultipartDataGenerator._initialize_boundary = ( lambda self: 1234567890 ) @@ -38,15 +42,19 @@ def test_is_creatable(self, setup_upload_api_base, request_mock): file=test_file, file_link_data={"create": True}, ) - request_mock.assert_api_base(stripe.upload_api_base) - request_mock.assert_requested( - "post", - "/v1/files", - headers={ - "Content-Type": "multipart/form-data; boundary=1234567890" - }, + method, url, headers, body = hc.request_with_retries.call_args[0] + assert method == "post" + assert url.endswith("/v1/files") + assert ( + headers["Content-Type"] + == "multipart/form-data; boundary=1234567890" ) - assert isinstance(resource, stripe.File) + parts = body.split(b"--1234567890") + assert len(parts) == 5 + assert parts[0] == b"" + assert parts[1].find(b'name="purpose"') > 0 + assert parts[2].find(b'name="file"') > 0 + assert parts[3].find(b'name="file_link_data[create]"') > 0 def test_create_respects_stripe_version( self, setup_upload_api_base, request_mock diff --git a/tests/api_resources/test_file_upload.py b/tests/api_resources/test_file_upload.py deleted file mode 100644 index 829a85638..000000000 --- a/tests/api_resources/test_file_upload.py +++ /dev/null @@ -1,57 +0,0 @@ -import tempfile - -import pytest - -import stripe - - -TEST_RESOURCE_ID = "file_123" - - -class TestFileUpload(object): - @pytest.fixture(scope="function") - def setup_upload_api_base(self): - stripe.upload_api_base = stripe.api_base - stripe.api_base = None - yield - stripe.api_base = stripe.upload_api_base - stripe.upload_api_base = "https://files.stripe.com" - - def test_is_listable(self, request_mock): - resources = stripe.FileUpload.list() - request_mock.assert_requested("get", "/v1/files") - assert isinstance(resources.data, list) - assert isinstance(resources.data[0], stripe.FileUpload) - - def test_is_retrievable(self, request_mock): - resource = stripe.FileUpload.retrieve(TEST_RESOURCE_ID) - request_mock.assert_requested("get", "/v1/files/%s" % TEST_RESOURCE_ID) - assert isinstance(resource, stripe.FileUpload) - - def test_is_creatable(self, setup_upload_api_base, request_mock): - stripe.multipart_data_generator.MultipartDataGenerator._initialize_boundary = ( - lambda self: 1234567890 - ) - test_file = tempfile.TemporaryFile() - resource = stripe.FileUpload.create( - purpose="dispute_evidence", - file=test_file, - file_link_data={"create": True}, - ) - request_mock.assert_api_base(stripe.upload_api_base) - request_mock.assert_requested( - "post", - "/v1/files", - headers={ - "Content-Type": "multipart/form-data; boundary=1234567890" - }, - ) - assert isinstance(resource, stripe.FileUpload) - - def test_deserializes_from_file(self): - obj = stripe.util.convert_to_stripe_object({"object": "file"}) - assert isinstance(obj, stripe.FileUpload) - - def test_deserializes_from_file_upload(self): - obj = stripe.util.convert_to_stripe_object({"object": "file_upload"}) - assert isinstance(obj, stripe.FileUpload) From db27b66c4888d4e82bea756e94b1dffec57db284 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Wed, 18 Oct 2023 14:40:41 -0700 Subject: [PATCH 3/4] Fix lint --- tests/api_resources/test_file.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index c781dcedd..366ee56c4 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -32,7 +32,7 @@ def test_is_creatable(self, mocker): hc = mocker.Mock(stripe.http_client.HTTPClient) hc.name = "mockclient" stripe.default_http_client = hc - hc.request_with_retries.return_value = ("{}", 200, {}) + hc.request_with_retries.return_value = ('{"object": "file"}', 200, {}) stripe.multipart_data_generator.MultipartDataGenerator._initialize_boundary = ( lambda self: 1234567890 ) @@ -55,6 +55,7 @@ def test_is_creatable(self, mocker): assert parts[1].find(b'name="purpose"') > 0 assert parts[2].find(b'name="file"') > 0 assert parts[3].find(b'name="file_link_data[create]"') > 0 + assert isinstance(resource, stripe.File) def test_create_respects_stripe_version( self, setup_upload_api_base, request_mock From d9e64151ed3ea0af3ed011b852ae99f6fd46ecc3 Mon Sep 17 00:00:00 2001 From: Richard Marmorstein Date: Wed, 18 Oct 2023 14:54:21 -0700 Subject: [PATCH 4/4] Use in, not find --- tests/api_resources/test_file.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/api_resources/test_file.py b/tests/api_resources/test_file.py index 366ee56c4..9a2c94bab 100644 --- a/tests/api_resources/test_file.py +++ b/tests/api_resources/test_file.py @@ -52,9 +52,9 @@ def test_is_creatable(self, mocker): parts = body.split(b"--1234567890") assert len(parts) == 5 assert parts[0] == b"" - assert parts[1].find(b'name="purpose"') > 0 - assert parts[2].find(b'name="file"') > 0 - assert parts[3].find(b'name="file_link_data[create]"') > 0 + assert b'name="purpose"' in parts[1] + assert b'name="file"' in parts[2] + assert b'name="file_link_data[create]"' in parts[3] assert isinstance(resource, stripe.File) def test_create_respects_stripe_version(