diff --git a/.changes/next-release/bugfix-crt-96245.json b/.changes/next-release/bugfix-crt-96245.json new file mode 100644 index 000000000000..d102de8b3bde --- /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..efd91b490217 100644 --- a/awscli/s3transfer/crt.py +++ b/awscli/s3transfer/crt.py @@ -55,6 +55,13 @@ CRT_S3_PROCESS_LOCK = None +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 @@ -430,6 +437,22 @@ 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 + + @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. @@ -479,6 +502,14 @@ def __init__(self, session, client_kwargs=None): 'before-call.s3.*', self._remove_checksum_context ) + @property + 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(): @@ -718,6 +749,8 @@ 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 + self._service_model = crt_request_serializer.service_model def get_make_request_args( self, request_type, call_args, coordinator, future, on_done_after_calls @@ -779,9 +812,8 @@ 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( + request_type, call_args.extra_args ): checksum_algorithm = call_args.extra_args.pop( 'ChecksumAlgorithm', 'CRC64NVME' @@ -819,7 +851,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 +880,45 @@ def _get_make_request_args_get_object( make_request_args['checksum_config'] = checksum_config return make_request_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 + ): + 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 + 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, @@ -869,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() 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]