From e165279f1207827562a6ee018370cd5335d48a3d Mon Sep 17 00:00:00 2001 From: jonathan343 Date: Tue, 21 Jan 2025 11:35:34 -0500 Subject: [PATCH] Only set default checksum if request_checksum_calculation config is set to when_supported --- awscli/s3transfer/manager.py | 6 +- tests/functional/s3transfer/test_upload.py | 159 ++++++++++++++++----- 2 files changed, 128 insertions(+), 37 deletions(-) diff --git a/awscli/s3transfer/manager.py b/awscli/s3transfer/manager.py index ea484a04bec7..862250330de0 100644 --- a/awscli/s3transfer/manager.py +++ b/awscli/s3transfer/manager.py @@ -515,7 +515,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 diff --git a/tests/functional/s3transfer/test_upload.py b/tests/functional/s3transfer/test_upload.py index 033fa27cb67e..b0fe39862122 100644 --- a/tests/functional/s3transfer/test_upload.py +++ b/tests/functional/s3transfer/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': 'CRC64NVME', } + 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,9 +412,7 @@ 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 @@ -380,8 +420,9 @@ def add_create_multipart_response_with_default_expected_params( expected_params = { 'Bucket': bucket, 'Key': self.key, - 'ChecksumAlgorithm': 'CRC64NVME', } + 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] @@ -389,9 +430,7 @@ def add_create_multipart_response_with_default_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': 'CRC64NVME', } + 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, - 'ChecksumCRC64NVME': 'sum1==', - }, - { - 'ETag': 'etag-2', - 'PartNumber': 2, - 'ChecksumCRC64NVME': 'sum2==', - }, - { - 'ETag': 'etag-3', - 'PartNumber': 3, - 'ChecksumCRC64NVME': '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()