Only set default checksum if request_checksum_calculation config …

…is set to ``when_supported`` (#329)

* Only set default checksum if ``request_checksum_calculation`` config is set to ``when_supported``

* Add changelog entry

.changes/next-release/bugfix-upload-80266.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "upload",
+  "description": "Only set a default checksum if the ``request_checksum_calculation`` config is set to ``when_supported``. Fixes `boto/s3transfer#327 <https://github.com/boto/s3transfer/issues/327>`__."
+}

s3transfer/manager.py

@@ -519,7 +519,11 @@ def _validate_all_known_args(self, actual, allowed):
                 )
 
     def _add_operation_defaults(self, extra_args):
-        set_default_checksum_algorithm(extra_args)
+        if (
+            self.client.meta.config.request_checksum_calculation
+            == "when_supported"
+        ):
+            set_default_checksum_algorithm(extra_args)
 
     def _submit_transfer(
         self, call_args, submission_task_cls, extra_main_kwargs=None

tests/functional/test_upload.py

@@ -19,6 +19,7 @@
 from botocore.awsrequest import AWSRequest
 from botocore.client import Config
 from botocore.exceptions import ClientError
+from botocore.httpchecksum import DEFAULT_CHECKSUM_ALGORITHM
 from botocore.stub import ANY
 
 from s3transfer.manager import TransferConfig, TransferManager
@@ -143,7 +144,7 @@ class TestNonMultipartUpload(BaseUploadTest):
     __test__ = True
 
     def add_put_object_response_with_default_expected_params(
-        self, extra_expected_params=None, bucket=None
+        self, extra_expected_params=None, bucket=None, include_checksum=True
     ):
         if bucket is None:
             bucket = self.bucket
@@ -152,8 +153,9 @@ def add_put_object_response_with_default_expected_params(
             'Body': ANY,
             'Bucket': bucket,
             'Key': self.key,
-            'ChecksumAlgorithm': 'CRC32',
         }
+        if include_checksum:
+            expected_params["ChecksumAlgorithm"] = DEFAULT_CHECKSUM_ALGORITHM
         if extra_expected_params:
             expected_params.update(extra_expected_params)
         upload_response = self.create_stubbed_responses()[0]
@@ -187,6 +189,46 @@ def test_upload_with_checksum(self):
         self.assert_expected_client_calls_were_correct()
         self.assert_put_object_body_was_correct()
 
+    def test_upload_with_default_checksum_when_supported(self):
+        # Reset client to configure `request_checksum_calculation` to "when_supported".
+        self.reset_stubber_with_new_client(
+            {'config': Config(request_checksum_calculation="when_supported")}
+        )
+        self.client.meta.events.register(
+            'before-parameter-build.s3.*', self.collect_body
+        )
+        self._manager = TransferManager(self.client, self.config)
+
+        self.add_put_object_response_with_default_expected_params(
+            include_checksum=True
+        )
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        self.assert_expected_client_calls_were_correct()
+        self.assert_put_object_body_was_correct()
+
+    def test_upload_with_default_checksum_when_required(self):
+        # Reset client to configure `request_checksum_calculation` to "when_required".
+        self.reset_stubber_with_new_client(
+            {'config': Config(request_checksum_calculation="when_required")}
+        )
+        self.client.meta.events.register(
+            'before-parameter-build.s3.*', self.collect_body
+        )
+        self._manager = TransferManager(self.client, self.config)
+
+        self.add_put_object_response_with_default_expected_params(
+            include_checksum=False
+        )
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        self.assert_expected_client_calls_were_correct()
+        self.assert_put_object_body_was_correct()
+
     def test_upload_with_s3express_default_checksum(self):
         s3express_bucket = "mytestbucket--usw2-az6--x-s3"
         self.assertFalse("ChecksumAlgorithm" in self.extra_args)
@@ -370,28 +412,25 @@ def assert_upload_part_bodies_were_correct(self):
         self.assertEqual(self.sent_bodies, expected_contents)
 
     def add_create_multipart_response_with_default_expected_params(
-        self,
-        extra_expected_params=None,
-        bucket=None,
+        self, extra_expected_params=None, bucket=None, include_checksum=True
     ):
         if bucket is None:
             bucket = self.bucket
 
         expected_params = {
             'Bucket': bucket,
             'Key': self.key,
-            'ChecksumAlgorithm': 'CRC32',
         }
+        if include_checksum:
+            expected_params["ChecksumAlgorithm"] = DEFAULT_CHECKSUM_ALGORITHM
         if extra_expected_params:
             expected_params.update(extra_expected_params)
         response = self.create_stubbed_responses()[0]
         response['expected_params'] = expected_params
         self.stubber.add_response(**response)
 
     def add_upload_part_responses_with_default_expected_params(
-        self,
-        extra_expected_params=None,
-        bucket=None,
+        self, extra_expected_params=None, bucket=None, include_checksum=True
     ):
         if bucket is None:
             bucket = self.bucket
@@ -406,50 +445,48 @@ def add_upload_part_responses_with_default_expected_params(
                 'UploadId': self.multipart_id,
                 'Body': ANY,
                 'PartNumber': i + 1,
-                'ChecksumAlgorithm': 'CRC32',
             }
+            if include_checksum:
+                expected_params["ChecksumAlgorithm"] = (
+                    DEFAULT_CHECKSUM_ALGORITHM
+                )
             if extra_expected_params:
                 expected_params.update(extra_expected_params)
 
-            name = expected_params['ChecksumAlgorithm']
-            checksum_member = f'Checksum{name.upper()}'
-            response = upload_part_response['service_response']
-            response[checksum_member] = f'sum{i+1}=='
+            # If ChecksumAlgorithm is in expected parameters, add checksum to the response
+            checksum_algorithm = expected_params.get('ChecksumAlgorithm')
+            if checksum_algorithm:
+                checksum_member = f'Checksum{checksum_algorithm.upper()}'
+                response = upload_part_response['service_response']
+                response[checksum_member] = f'sum{i+1}=='
 
             upload_part_response['expected_params'] = expected_params
             self.stubber.add_response(**upload_part_response)
 
     def add_complete_multipart_response_with_default_expected_params(
-        self,
-        extra_expected_params=None,
-        bucket=None,
+        self, extra_expected_params=None, bucket=None, include_checksum=True
     ):
         if bucket is None:
             bucket = self.bucket
 
+        num_parts = 3
+        parts = []
+        for part_num in range(1, num_parts + 1):
+            part = {
+                'ETag': f'etag-{part_num}',
+                'PartNumber': part_num,
+            }
+            if include_checksum:
+                part[f"Checksum{DEFAULT_CHECKSUM_ALGORITHM}"] = (
+                    f"sum{part_num}=="
+                )
+            parts.append(part)
+
         expected_params = {
             'Bucket': bucket,
             'Key': self.key,
             'UploadId': self.multipart_id,
-            'MultipartUpload': {
-                'Parts': [
-                    {
-                        'ETag': 'etag-1',
-                        'PartNumber': 1,
-                        'ChecksumCRC32': 'sum1==',
-                    },
-                    {
-                        'ETag': 'etag-2',
-                        'PartNumber': 2,
-                        'ChecksumCRC32': 'sum2==',
-                    },
-                    {
-                        'ETag': 'etag-3',
-                        'PartNumber': 3,
-                        'ChecksumCRC32': 'sum3==',
-                    },
-                ]
-            },
+            'MultipartUpload': {'Parts': parts},
         }
         if extra_expected_params:
             expected_params.update(extra_expected_params)
@@ -761,3 +798,53 @@ def test_multipart_upload_with_full_object_checksum_args(self):
         )
         future.result()
         self.assert_expected_client_calls_were_correct()
+
+    def test_multipart_upload_with_default_checksum_when_supported(self):
+        # Reset client to configure `request_checksum_calculation` to "when_supported".
+        self.reset_stubber_with_new_client(
+            {'config': Config(request_checksum_calculation="when_supported")}
+        )
+        self.client.meta.events.register(
+            'before-parameter-build.s3.*', self.collect_body
+        )
+        self._manager = TransferManager(self.client, self.config)
+
+        self.add_create_multipart_response_with_default_expected_params(
+            include_checksum=True
+        )
+        self.add_upload_part_responses_with_default_expected_params(
+            include_checksum=True
+        )
+        self.add_complete_multipart_response_with_default_expected_params()
+
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        self.assert_expected_client_calls_were_correct()
+
+    def test_multipart_upload_with_default_checksum_when_required(self):
+        # Reset client to configure `request_checksum_calculation` to "when_required".
+        self.reset_stubber_with_new_client(
+            {'config': Config(request_checksum_calculation="when_required")}
+        )
+        self.client.meta.events.register(
+            'before-parameter-build.s3.*', self.collect_body
+        )
+        self._manager = TransferManager(self.client, self.config)
+
+        self.add_create_multipart_response_with_default_expected_params(
+            include_checksum=False
+        )
+        self.add_upload_part_responses_with_default_expected_params(
+            include_checksum=False
+        )
+        self.add_complete_multipart_response_with_default_expected_params(
+            include_checksum=False
+        )
+
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        self.assert_expected_client_calls_were_correct()