From bdcf42279db33e3fc68270641432e5c9027f376e Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Wed, 2 Sep 2026 16:13:41 -0400 Subject: [PATCH 1/3] Pass response_checksum_validation and request_checksum_calculation to CRT --- .changes/next-release/bugfix-crt-96245.json | 5 + awscli/s3transfer/crt.py | 53 +++++++++- tests/functional/s3transfer/test_crt.py | 111 ++++++++++++++++++++ 3 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 .changes/next-release/bugfix-crt-96245.json diff --git a/.changes/next-release/bugfix-crt-96245.json b/.changes/next-release/bugfix-crt-96245.json new file mode 100644 index 000000000000..65f4beeec51b --- /dev/null +++ b/.changes/next-release/bugfix-crt-96245.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "crt", + "description": "Pass configured `response_checksum_validation` and `request_checksum_calculation` options to CRT transfers." +} diff --git a/awscli/s3transfer/crt.py b/awscli/s3transfer/crt.py index ae4e5195a4b2..ee9dbce53d2e 100644 --- a/awscli/s3transfer/crt.py +++ b/awscli/s3transfer/crt.py @@ -55,6 +55,9 @@ CRT_S3_PROCESS_LOCK = None +WHEN_REQUIRED = 'when_required' + + def acquire_crt_s3_process_lock(name): # Currently, the CRT S3 client performs best when there is only one # instance of it running on a host. This lock allows an application to @@ -430,6 +433,14 @@ def set_exception(self, exception): class BaseCRTRequestSerializer: + @property + def client_config(self): + """Resolved botocore configuration, if this serializer has any. + + :rtype: Optional[botocore.config.Config] + """ + return None + def serialize_http_request(self, transfer_type, future): """Serialize CRT HTTP requests. @@ -479,6 +490,10 @@ def __init__(self, session, client_kwargs=None): 'before-call.s3.*', self._remove_checksum_context ) + @property + def client_config(self): + return self._client.meta.config + def _resolve_client_config(self, session, client_kwargs): user_provided_config = None if session.get_default_client_config(): @@ -718,6 +733,7 @@ class S3ClientArgsCreator: def __init__(self, crt_request_serializer, os_utils): self._request_serializer = crt_request_serializer self._os_utils = os_utils + self._client_config = crt_request_serializer.client_config def get_make_request_args( self, request_type, call_args, coordinator, future, on_done_after_calls @@ -779,10 +795,7 @@ def _get_make_request_args_put_object( call_args.extra_args["Body"] = call_args.fileobj checksum_config = None - if not any( - checksum_arg in call_args.extra_args - for checksum_arg in FULL_OBJECT_CHECKSUM_ARGS - ): + if self._should_calculate_upload_checksum(call_args.extra_args): checksum_algorithm = call_args.extra_args.pop( 'ChecksumAlgorithm', 'CRC64NVME' ).upper() @@ -819,7 +832,11 @@ def _get_make_request_args_get_object( ): recv_filepath = None on_body = None - checksum_config = awscrt.s3.S3ChecksumConfig(validate_response=True) + checksum_config = awscrt.s3.S3ChecksumConfig( + validate_response=self._should_validate_download_checksum( + call_args.extra_args + ) + ) if isinstance(call_args.fileobj, str): final_filepath = call_args.fileobj recv_filepath = self._os_utils.get_temp_filename(final_filepath) @@ -844,6 +861,32 @@ def _get_make_request_args_get_object( make_request_args['checksum_config'] = checksum_config return make_request_args + def _should_calculate_upload_checksum(self, extra_args): + if any( + checksum_arg in extra_args + for checksum_arg in FULL_OBJECT_CHECKSUM_ARGS + ): + return False + if 'ChecksumAlgorithm' in extra_args: + return True + return ( + self._get_client_config('request_checksum_calculation') + != WHEN_REQUIRED + ) + + def _should_validate_download_checksum(self, extra_args): + if 'ChecksumMode' in extra_args: + return True + return ( + self._get_client_config('response_checksum_validation') + != WHEN_REQUIRED + ) + + def _get_client_config(self, name): + if self._client_config is None: + return None + return getattr(self._client_config, name) + def _default_get_make_request_args( self, request_type, diff --git a/tests/functional/s3transfer/test_crt.py b/tests/functional/s3transfer/test_crt.py index d092982f082c..790d80c46452 100644 --- a/tests/functional/s3transfer/test_crt.py +++ b/tests/functional/s3transfer/test_crt.py @@ -214,6 +214,117 @@ def _get_expected_download_checksum_config(self, **overrides): checksum_config_kwargs.update(overrides) return awscrt.s3.S3ChecksumConfig(**checksum_config_kwargs) + def _set_checksum_config_variables( + self, request_calculation=None, response_validation=None + ): + if request_calculation is not None: + self.session.set_config_variable( + 'request_checksum_calculation', request_calculation + ) + if response_validation is not None: + self.session.set_config_variable( + 'response_checksum_validation', response_validation + ) + # The serializer resolves these when it creates its client, so it has + # to be rebuilt after changing them. + self.request_serializer = s3transfer.crt.BotocoreCRTRequestSerializer( + self.session + ) + self.transfer_manager = s3transfer.crt.CRTTransferManager( + crt_s3_client=self.s3_crt_client, + crt_request_serializer=self.request_serializer, + ) + + def _get_checksum_config_from_make_request(self): + return self.s3_crt_client.make_request.call_args[1]['checksum_config'] + + def test_upload_calculates_checksum_when_supported(self): + self._set_checksum_config_variables( + request_calculation='when_supported' + ) + future = self.transfer_manager.upload( + self.filename, self.bucket, self.key, {}, [] + ) + future.result() + self.assertEqual( + self._get_checksum_config_from_make_request(), + self._get_expected_upload_checksum_config(), + ) + + def test_upload_skips_checksum_when_required(self): + self._set_checksum_config_variables( + request_calculation='when_required' + ) + future = self.transfer_manager.upload( + self.filename, self.bucket, self.key, {}, [] + ) + future.result() + self.assertIsNone(self._get_checksum_config_from_make_request()) + + def test_upload_uses_requested_algorithm_when_required(self): + self._set_checksum_config_variables( + request_calculation='when_required' + ) + future = self.transfer_manager.upload( + self.filename, + self.bucket, + self.key, + {'ChecksumAlgorithm': 'CRC32'}, + [], + ) + future.result() + self.assertEqual( + self._get_checksum_config_from_make_request(), + self._get_expected_upload_checksum_config( + algorithm=awscrt.s3.S3ChecksumAlgorithm.CRC32 + ), + ) + + def test_download_validates_checksum_when_supported(self): + self._set_checksum_config_variables( + response_validation='when_supported' + ) + future = self.transfer_manager.download( + self.bucket, self.key, self.filename, {}, [] + ) + future.result() + self.assertEqual( + self._get_checksum_config_from_make_request(), + self._get_expected_download_checksum_config(), + ) + + def test_download_skips_validation_when_required(self): + self._set_checksum_config_variables( + response_validation='when_required' + ) + future = self.transfer_manager.download( + self.bucket, self.key, self.filename, {}, [] + ) + future.result() + self.assertEqual( + self._get_checksum_config_from_make_request(), + self._get_expected_download_checksum_config( + validate_response=False + ), + ) + + def test_download_validates_when_checksum_mode_requested(self): + self._set_checksum_config_variables( + response_validation='when_required' + ) + future = self.transfer_manager.download( + self.bucket, + self.key, + self.filename, + {'ChecksumMode': 'ENABLED'}, + [], + ) + future.result() + self.assertEqual( + self._get_checksum_config_from_make_request(), + self._get_expected_download_checksum_config(), + ) + def _invoke_done_callbacks(self, **kwargs): callargs = self.s3_crt_client.make_request.call_args callargs_kwargs = callargs[1] From 4f4dabc02996a862d6cd24ccf0c863d5c88f445d Mon Sep 17 00:00:00 2001 From: Steve <106777148+hssyoo@users.noreply.github.com> Date: Wed, 9 Sep 2026 10:01:36 -0400 Subject: [PATCH 2/3] Apply suggestion from @ashovlin Co-authored-by: Alex Shovlin --- .changes/next-release/bugfix-crt-96245.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/bugfix-crt-96245.json b/.changes/next-release/bugfix-crt-96245.json index 65f4beeec51b..d102de8b3bde 100644 --- a/.changes/next-release/bugfix-crt-96245.json +++ b/.changes/next-release/bugfix-crt-96245.json @@ -1,5 +1,5 @@ { "type": "bugfix", "category": "crt", - "description": "Pass configured `response_checksum_validation` and `request_checksum_calculation` options to CRT transfers." + "description": "Pass configured ``response_checksum_validation`` and ``request_checksum_calculation`` options to CRT transfers." } From e21388fc498c97292c567b553bd7abd9c6e7d470 Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Wed, 9 Sep 2026 10:10:03 -0400 Subject: [PATCH 3/3] Check model for checksum requirement --- awscli/s3transfer/crt.py | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/awscli/s3transfer/crt.py b/awscli/s3transfer/crt.py index ee9dbce53d2e..efd91b490217 100644 --- a/awscli/s3transfer/crt.py +++ b/awscli/s3transfer/crt.py @@ -58,6 +58,10 @@ WHEN_REQUIRED = 'when_required' +def _get_operation_name(request_type): + return ''.join(part.title() for part in request_type.split('_')) + + def acquire_crt_s3_process_lock(name): # Currently, the CRT S3 client performs best when there is only one # instance of it running on a host. This lock allows an application to @@ -441,6 +445,14 @@ def client_config(self): """ return None + @property + def service_model(self): + """Model of the service being serialized, if there is one. + + :rtype: Optional[botocore.model.ServiceModel] + """ + return None + def serialize_http_request(self, transfer_type, future): """Serialize CRT HTTP requests. @@ -494,6 +506,10 @@ def __init__(self, session, client_kwargs=None): def client_config(self): return self._client.meta.config + @property + def service_model(self): + return self._client.meta.service_model + def _resolve_client_config(self, session, client_kwargs): user_provided_config = None if session.get_default_client_config(): @@ -734,6 +750,7 @@ def __init__(self, crt_request_serializer, os_utils): self._request_serializer = crt_request_serializer self._os_utils = os_utils self._client_config = crt_request_serializer.client_config + self._service_model = crt_request_serializer.service_model def get_make_request_args( self, request_type, call_args, coordinator, future, on_done_after_calls @@ -795,7 +812,9 @@ def _get_make_request_args_put_object( call_args.extra_args["Body"] = call_args.fileobj checksum_config = None - if self._should_calculate_upload_checksum(call_args.extra_args): + if self._should_calculate_upload_checksum( + request_type, call_args.extra_args + ): checksum_algorithm = call_args.extra_args.pop( 'ChecksumAlgorithm', 'CRC64NVME' ).upper() @@ -861,7 +880,7 @@ def _get_make_request_args_get_object( make_request_args['checksum_config'] = checksum_config return make_request_args - def _should_calculate_upload_checksum(self, extra_args): + def _should_calculate_upload_checksum(self, request_type, extra_args): if any( checksum_arg in extra_args for checksum_arg in FULL_OBJECT_CHECKSUM_ARGS @@ -869,11 +888,24 @@ def _should_calculate_upload_checksum(self, extra_args): return False if 'ChecksumAlgorithm' in extra_args: return True + if self._is_request_checksum_required(request_type): + return True return ( self._get_client_config('request_checksum_calculation') != WHEN_REQUIRED ) + def _is_request_checksum_required(self, request_type): + if self._service_model is None: + return False + operation_model = self._service_model.operation_model( + _get_operation_name(request_type) + ) + return bool( + operation_model.http_checksum_required + or operation_model.http_checksum.get('requestChecksumRequired') + ) + def _should_validate_download_checksum(self, extra_args): if 'ChecksumMode' in extra_args: return True @@ -912,8 +944,8 @@ def _default_get_make_request_args( # For DEFAULT requests, CRT requires the official S3 operation name. # So transform string like "delete_object" -> "DeleteObject". if make_request_args['type'] == S3RequestType.DEFAULT: - make_request_args['operation_name'] = ''.join( - x.title() for x in request_type.split('_') + make_request_args['operation_name'] = _get_operation_name( + request_type ) arn_handler = _S3ArnParamHandler()