Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only set default checksum if request_checksum_calculation config is set to when_supported #9238

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion awscli/s3transfer/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
159 changes: 123 additions & 36 deletions tests/functional/s3transfer/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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': '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]
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
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Loading