From 4208cb351e3268818b19a3aaea9dd4a4ef08ae10 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 6 Jun 2023 11:39:28 -0400 Subject: [PATCH 01/37] request compression --- botocore/args.py | 21 +++ botocore/awsrequest.py | 90 +++++++++++- botocore/config.py | 17 +++ botocore/configprovider.py | 14 +- botocore/endpoint.py | 19 +-- botocore/handlers.py | 58 ++++++-- botocore/model.py | 8 + botocore/signers.py | 1 - tests/functional/test_docdb.py | 4 +- tests/functional/test_neptune.py | 4 +- tests/functional/test_rds.py | 4 +- tests/unit/test_awsrequest.py | 244 ++++++++++++++++++++++++++++++- tests/unit/test_client.py | 62 ++++++++ tests/unit/test_endpoint.py | 3 + tests/unit/test_model.py | 10 +- 15 files changed, 526 insertions(+), 33 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 73c8ab45e0..a7cb8c1e7d 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -260,10 +260,17 @@ def compute_client_args( tcp_keepalive=client_config.tcp_keepalive, user_agent_extra=client_config.user_agent_extra, user_agent_appid=client_config.user_agent_appid, + request_min_compression_size_bytes=( + client_config.request_min_compression_size_bytes + ), + disable_request_compression=( + client_config.disable_request_compression + ), ) self._compute_retry_config(config_kwargs) self._compute_connect_timeout(config_kwargs) self._compute_user_agent_appid_config(config_kwargs) + self._compute_request_compression_config(config_kwargs) s3_config = self.compute_s3_config(client_config) is_s3_service = self._is_s3_service(service_name) @@ -543,6 +550,20 @@ def _compute_connect_timeout(self, config_kwargs): if connect_timeout: config_kwargs['connect_timeout'] = connect_timeout + def _compute_request_compression_config(self, config_kwargs): + min_size = config_kwargs.get('request_min_compression_size_bytes') + disabled = config_kwargs.get('disable_request_compression') + if min_size is None: + min_size = self._config_store.get_config_variable( + 'request_min_compression_size_bytes' + ) + config_kwargs['request_min_compression_size_bytes'] = min_size + if disabled is None: + disabled = self._config_store.get_config_variable( + 'disable_request_compression' + ) + config_kwargs['disable_request_compression'] = disabled + def _ensure_boolean(self, val): if isinstance(val, bool): return val diff --git a/botocore/awsrequest.py b/botocore/awsrequest.py index f00a0dde57..45b408443c 100644 --- a/botocore/awsrequest.py +++ b/botocore/awsrequest.py @@ -12,6 +12,8 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import functools +import gzip +import io import logging from collections.abc import Mapping @@ -477,7 +479,7 @@ def prepare(self): @property def body(self): - body = self.prepare().body + body = self._request_preparer._prepare_body(self) if isinstance(body, str): body = body.encode('utf-8') return body @@ -534,6 +536,92 @@ def reset_stream(self): raise UnseekableStreamError(stream_object=self.body) +class AWSRequestCompressor: + """A class that can compress the body of an ``AWSRequest``.""" + + def compress(self, config, request_dict, operation_model): + """Compresses the request body using the specified encodings if conditions + are met. + + Check if the request should be compressed based on the contents of its + body and config settings. Set or append the `Content-Encoding` header + with the matched encoding if not present. + """ + body = request_dict['body'] + if self._should_compress_request(config, body, operation_model): + encodings = operation_model.request_compression['encodings'] + headers = request_dict.get('headers', {}) + for encoding in encodings: + encoder = getattr(self, f'_{encoding}_compress_body', None) + if encoder is not None: + if 'Content-Encoding' not in headers: + headers['Content-Encoding'] = encoding + elif encoding not in headers['Content-Encoding'].split( + ',' + ): + headers[ + 'Content-Encoding' + ] = f'{headers["Content-Encoding"]},{encoding}' + request_dict['body'] = encoder(body) + if 'headers' not in request_dict: + request_dict['headers'] = headers + else: + logger.debug( + 'Unsupported compression encoding: %s' % encoding + ) + + def _gzip_compress_body(self, body): + if isinstance(body, str): + return gzip.compress(body.encode('utf-8')) + elif isinstance(body, (bytes, bytearray)): + return gzip.compress(body) + elif hasattr(body, 'read'): + if hasattr(body, 'seek') and hasattr(body, 'tell'): + current_position = body.tell() + compressed_obj = self._gzip_compress_fileobj(body) + body.seek(current_position) + return compressed_obj + return self._gzip_compress_fileobj(body) + + def _gzip_compress_fileobj(self, body): + compressed_obj = io.BytesIO() + with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: + while True: + chunk = body.read(8192) + if not chunk: + break + if isinstance(chunk, str): + chunk = chunk.encode('utf-8') + gz.write(chunk) + compressed_obj.seek(0) + return compressed_obj + + def _should_compress_request(self, config, body, operation_model): + if config.disable_request_compression is not True: + # Request is compressed no matter the content length if it has a streaming input. + # However, if the stream has the `requiresLength` trait it is NOT compressed. + if operation_model.has_streaming_input: + return ( + 'requiresLength' + not in operation_model.get_streaming_input().metadata + ) + return ( + config.request_min_compression_size_bytes + <= self._get_body_size(body) + ) + return False + + def _get_body_size(self, body): + size = botocore.utils.determine_content_length(body) + if size is None: + logger.debug( + 'Unable to get length of the request body: %s. Not compressing.' + % body + ) + return -1 + return size + + class AWSResponse: """A data class representing an HTTP response. diff --git a/botocore/config.py b/botocore/config.py index be3a475fa7..de31321d85 100644 --- a/botocore/config.py +++ b/botocore/config.py @@ -206,6 +206,21 @@ class Config: creating new connections if set to True. Defaults to False. + + :type request_min_compression_size_bytes: int + :param request_min_compression_bytes: The minimum size in bytes that a + request body should be to trigger compression. A request is compressed + regardless of the size of the body if the operation has streaming input, + but if the operation also contains the `requiresLength` trait, the request + is not compressed. + + Defaults to None. + + :type disable_request_compression: bool + :param disable_request_compression: Disables the request body compression + if set to True. + + Defaults to None. """ OPTION_DEFAULTS = OrderedDict( @@ -231,6 +246,8 @@ class Config: ('ignore_configured_endpoint_urls', None), ('defaults_mode', None), ('tcp_keepalive', None), + ('request_min_compression_size_bytes', None), + ('disable_request_compression', None), ] ) diff --git a/botocore/configprovider.py b/botocore/configprovider.py index d7b2e19de8..f8a0b072f2 100644 --- a/botocore/configprovider.py +++ b/botocore/configprovider.py @@ -10,7 +10,7 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. -"""This module contains the inteface for controlling how configuration +"""This module contains the interface for controlling how configuration is loaded. """ import copy @@ -147,6 +147,18 @@ # whatever the defaults are in _retry.json. 'max_attempts': ('max_attempts', 'AWS_MAX_ATTEMPTS', None, int), 'user_agent_appid': ('sdk_ua_app_id', 'AWS_SDK_UA_APP_ID', None, None), + 'request_min_compression_size_bytes': ( + 'request_min_compression_size_bytes', + 'AWS_REQUEST_MIN_COMPRESSION_SIZE_BYTES', + 10240, + int, + ), + 'disable_request_compression': ( + 'disable_request_compression', + 'AWS_DISABLE_REQUEST_COMPRESSION', + False, + utils.ensure_boolean, + ), } # A mapping for the s3 specific configuration vars. These are the configuration # vars that typically go in the s3 section of the config file. This mapping diff --git a/botocore/endpoint.py b/botocore/endpoint.py index adc622c25a..2230254bed 100644 --- a/botocore/endpoint.py +++ b/botocore/endpoint.py @@ -20,7 +20,7 @@ import uuid from botocore import parsers -from botocore.awsrequest import create_request_object +from botocore.awsrequest import AWSRequestCompressor, create_request_object from botocore.exceptions import HTTPClientError from botocore.history import get_global_history_recorder from botocore.hooks import first_non_none_response @@ -75,14 +75,8 @@ def convert_to_response_dict(http_response, operation_model): class Endpoint: - """ - Represents an endpoint for a particular service in a specific - region. Only an endpoint can make requests. - - :ivar service: The Service object that describes this endpoints - service. - :ivar host: The fully qualified endpoint hostname. - :ivar session: The session object. + """Represents an endpoint for a particular service in a specific region. + Only an endpoint can make requests. """ def __init__( @@ -103,6 +97,7 @@ def __init__( self.http_session = http_session if self.http_session is None: self.http_session = URLLib3Session() + self._request_compressor = AWSRequestCompressor() def __repr__(self): return f'{self._endpoint_prefix}({self.host})' @@ -121,6 +116,12 @@ def make_request(self, operation_model, request_dict): def create_request(self, params, operation_model=None): request = create_request_object(params) if operation_model: + if operation_model.request_compression: + self._request_compressor.compress( + params['context']['client_config'], + request, + operation_model, + ) request.stream_output = any( [ operation_model.has_streaming_output, diff --git a/botocore/handlers.py b/botocore/handlers.py index e3eb5c886b..92258d212b 100644 --- a/botocore/handlers.py +++ b/botocore/handlers.py @@ -28,6 +28,7 @@ import botocore import botocore.auth from botocore import utils +from botocore.awsrequest import AWSRequestCompressor from botocore.compat import ( ETree, OrderedDict, @@ -38,6 +39,7 @@ quote, unquote, unquote_str, + urlencode, urlsplit, urlunsplit, ) @@ -1148,6 +1150,34 @@ def remove_content_type_header_for_presigning(request, **kwargs): del request.headers['Content-Type'] +def urlencode_body(model, params, context, **kwargs): + """Urlencode the request body if it is a dictionary. + + This is used for services like S3 that require the body to be urlencoded + when the body is a dictionary. + """ + body = params.get('body') + if ( + context['client_config'].signature_version != 'v2' + and model.service_model.protocol == 'query' + and isinstance(body, dict) + ): + params['body'] = urlencode(body, doseq=True, encoding='utf-8').encode( + 'utf-8' + ) + + +def compress_request(model, params, context, **kwargs): + body = params.get('body') + config = context['client_config'] + if config.signature_version == 'v2': + logger.debug( + "Request compression is not supported for signature version 2" + ) + elif model.request_compression and body is not None: + AWS_REQUEST_COMPRESSOR.compress(body, context['client_config'], model) + + # This is a list of (event_name, handler). # When a Session is created, everything in this list will be # automatically registered with that Session. @@ -1207,13 +1237,27 @@ def remove_content_type_header_for_presigning(request, **kwargs): ('before-call.s3', add_expect_header), ('before-call.glacier', add_glacier_version), ('before-call.apigateway', add_accept_header), + ('before-call.ec2.CopySnapshot', inject_presigned_url_ec2), + ('before-call.rds.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.rds.CreateDBCluster', inject_presigned_url_rds), + ('before-call.rds.CopyDBSnapshot', inject_presigned_url_rds), + ('before-call.rds.CreateDBInstanceReadReplica', inject_presigned_url_rds), + ( + 'before-call.rds.StartDBInstanceAutomatedBackupsReplication', + inject_presigned_url_rds, + ), + ('before-call.neptune.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.neptune.CreateDBCluster', inject_presigned_url_rds), + ('before-call.docdb.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.docdb.CreateDBCluster', inject_presigned_url_rds), + ('before-call', urlencode_body), + ('before-call', compress_request), ('before-call.s3.PutObject', conditionally_calculate_md5), ('before-call.s3.UploadPart', conditionally_calculate_md5), ('before-call.s3.DeleteObjects', escape_xml_payload), ('before-call.s3.PutBucketLifecycleConfiguration', escape_xml_payload), ('before-call.glacier.UploadArchive', add_glacier_checksums), ('before-call.glacier.UploadMultipartPart', add_glacier_checksums), - ('before-call.ec2.CopySnapshot', inject_presigned_url_ec2), ('request-created', add_retry_headers), ('request-created.machinelearning.Predict', switch_host_machinelearning), ('needs-retry.s3.UploadPartCopy', check_for_200_error, REGISTER_FIRST), @@ -1348,14 +1392,6 @@ def remove_content_type_header_for_presigning(request, **kwargs): # RDS ############# ('creating-client-class.rds', add_generate_db_auth_token), - ('before-call.rds.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.rds.CreateDBCluster', inject_presigned_url_rds), - ('before-call.rds.CopyDBSnapshot', inject_presigned_url_rds), - ('before-call.rds.CreateDBInstanceReadReplica', inject_presigned_url_rds), - ( - 'before-call.rds.StartDBInstanceAutomatedBackupsReplication', - inject_presigned_url_rds, - ), # RDS PresignedUrl documentation customizations ( 'docs.*.rds.CopyDBClusterSnapshot.complete-section', @@ -1380,8 +1416,6 @@ def remove_content_type_header_for_presigning(request, **kwargs): ############# # Neptune ############# - ('before-call.neptune.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.neptune.CreateDBCluster', inject_presigned_url_rds), # Neptune PresignedUrl documentation customizations ( 'docs.*.neptune.CopyDBClusterSnapshot.complete-section', @@ -1394,8 +1428,6 @@ def remove_content_type_header_for_presigning(request, **kwargs): ############# # DocDB ############# - ('before-call.docdb.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.docdb.CreateDBCluster', inject_presigned_url_rds), # DocDB PresignedUrl documentation customizations ( 'docs.*.docdb.CopyDBClusterSnapshot.complete-section', diff --git a/botocore/model.py b/botocore/model.py index ea888ce1bb..dec3e95eb5 100644 --- a/botocore/model.py +++ b/botocore/model.py @@ -87,6 +87,7 @@ class Shape: 'union', 'contextParam', 'clientContextParams', + 'requiresLength', ] MAP_TYPE = OrderedDict @@ -172,6 +173,9 @@ def metadata(self): * idempotencyToken * document * union + * contextParam + * clientContextParams + * requiresLength :rtype: dict :return: Metadata about the shape. @@ -614,6 +618,10 @@ def context_parameters(self): and 'name' in shape.metadata['contextParam'] ] + @CachedProperty + def request_compression(self): + return self._operation_model.get('requestcompression') + @CachedProperty def auth_type(self): return self._operation_model.get('authtype') diff --git a/botocore/signers.py b/botocore/signers.py index 0acbf68463..0e00c094a8 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -185,7 +185,6 @@ def sign( ) else: raise e - auth.add_auth(request) def _choose_signer(self, operation_name, signing_type, context): diff --git a/tests/functional/test_docdb.py b/tests/functional/test_docdb.py index 11c02574bf..e64bc7f42a 100644 --- a/tests/functional/test_docdb.py +++ b/tests/functional/test_docdb.py @@ -20,8 +20,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn('PreSignedUrl', body) - self.assertNotIn('SourceRegion', body) + self.assertIn(b'PreSignedUrl', body) + self.assertNotIn(b'SourceRegion', body) def test_create_db_cluster(self): params = { diff --git a/tests/functional/test_neptune.py b/tests/functional/test_neptune.py index 2ada9421c9..37000d9f13 100644 --- a/tests/functional/test_neptune.py +++ b/tests/functional/test_neptune.py @@ -20,8 +20,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn('PreSignedUrl', body) - self.assertNotIn('SourceRegion', body) + self.assertIn(b'PreSignedUrl', body) + self.assertNotIn(b'SourceRegion', body) def test_create_db_cluster(self): params = { diff --git a/tests/functional/test_rds.py b/tests/functional/test_rds.py index cd4cedc219..d680db2709 100644 --- a/tests/functional/test_rds.py +++ b/tests/functional/test_rds.py @@ -21,8 +21,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn('PreSignedUrl', body) - self.assertNotIn('SourceRegion', body) + self.assertIn(b'PreSignedUrl', body) + self.assertNotIn(b'SourceRegion', body) def test_copy_snapshot(self): params = { diff --git a/tests/unit/test_awsrequest.py b/tests/unit/test_awsrequest.py index a29a846116..a39b60915d 100644 --- a/tests/unit/test_awsrequest.py +++ b/tests/unit/test_awsrequest.py @@ -25,12 +25,14 @@ AWSHTTPConnection, AWSHTTPSConnection, AWSRequest, + AWSRequestCompressor, AWSResponse, HeadersDict, create_request_object, prepare_request_dict, ) -from botocore.compat import file_type +from botocore.compat import HTTPHeaders, file_type +from botocore.config import Config from botocore.exceptions import UnseekableStreamError from tests import mock, unittest @@ -107,6 +109,100 @@ def tell(self): return self._stream.tell() +def _op_with_compression(): + op = mock.Mock() + op.request_compression = {'encodings': ['gzip']} + op.has_streaming_input = False + return op + + +def _op_unknown_compression(): + op = mock.Mock() + op.request_compression = {'encodings': ['foo']} + op.has_streaming_input = None + return op + + +def _op_without_compression(): + op = mock.Mock() + op.request_compression = None + op.has_streaming_input = False + return op + + +def _streaming_op_with_compression(): + op = _op_with_compression() + op.has_streaming_input = True + streaming_shape = mock.Mock() + streaming_shape.metadata = {} + op.get_streaming_input.return_value = streaming_shape + return op + + +def _streaming_op_with_compression_requires_length(): + op = _streaming_op_with_compression() + streaming_shape = mock.Mock() + streaming_shape.metadata = {'requiresLength': True} + op.get_streaming_input.return_value = streaming_shape + return op + + +OP_WITH_COMPRESSION = _op_with_compression() +OP_UNKNOWN_COMPRESSION = _op_unknown_compression() +STREAMING_OP_WITH_COMPRESSION = _streaming_op_with_compression() +REQUEST_BODY = ( + b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' + b'&MetricData.member.1.MetricName=metric&MetricData.member.1.Unit=Bytes' + b'&MetricData.member.1.Value=128' +) + +DEFAULT_COMPRESSION_CONFIG = Config( + disable_request_compression=False, + request_min_compression_size_bytes=10420, +) +COMPRESSION_CONFIG_128_BYTES = Config( + disable_request_compression=False, + request_min_compression_size_bytes=128, +) +COMPRESSION_CONFIG_0_BYTES = Config( + disable_request_compression=False, + request_min_compression_size_bytes=0, +) +GZIP_ENCODINGS = ['gzip'] + + +def request_dict(): + return { + 'body': REQUEST_BODY, + 'headers': {'foo': 'bar'}, + } + + +def request_dict_with_content_encoding_header(): + return { + 'body': REQUEST_BODY, + 'headers': {'foo': 'bar', 'Content-Encoding': 'identity'}, + } + + +@pytest.fixture(scope="module") +def aws_request_compressor(): + return AWSRequestCompressor() + + +@pytest.fixture(scope="module") +def headers(): + return HTTPHeaders() + + +def _assert_compression(is_compressed, body): + if hasattr(body, 'read'): + header = body.read(2) + else: + header = body[:2] + assert is_compressed == (header == b'\x1f\x8b') + + class TestAWSRequest(unittest.TestCase): def setUp(self): self.tempdir = tempfile.mkdtemp() @@ -798,5 +894,151 @@ def test_iteration(self): self.assertIn(('dead', 'beef'), headers_items) +@pytest.mark.parametrize( + 'config, request_dict, operation_model, is_compressed, encoding', + [ + ( + Config( + disable_request_compression=True, + request_min_compression_size_bytes=1000, + ), + {'body': b'foo'}, + OP_WITH_COMPRESSION, + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict(), + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + Config( + disable_request_compression=False, + request_min_compression_size_bytes=256, + ), + request_dict(), + OP_WITH_COMPRESSION, + False, + None, + ), + ( + Config(request_min_compression_size_bytes=128), + request_dict(), + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + STREAMING_OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + _streaming_op_with_compression_requires_length(), + False, + None, + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + _op_without_compression(), + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict(), + OP_UNKNOWN_COMPRESSION, + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': REQUEST_BODY.decode()}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': bytearray(REQUEST_BODY)}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': io.BytesIO(REQUEST_BODY)}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': io.StringIO(REQUEST_BODY.decode())}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict_with_content_encoding_header(), + OP_UNKNOWN_COMPRESSION, + False, + "foo", + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict_with_content_encoding_header(), + OP_WITH_COMPRESSION, + True, + "gzip", + ), + ], +) +def test_compress( + aws_request_compressor, + config, + request_dict, + operation_model, + is_compressed, + encoding, +): + aws_request_compressor.compress(config, request_dict, operation_model) + _assert_compression(is_compressed, request_dict['body']) + assert ( + 'headers' in request_dict + and 'Content-Encoding' in request_dict['headers'] + and encoding in request_dict['headers']['Content-Encoding'] + ) == is_compressed + + +@pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) +def test_compress_bad_types(aws_request_compressor, body): + request_dict = {'body': body} + aws_request_compressor.compress( + COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION + ) + assert request_dict['body'] == body + + +@pytest.mark.parametrize( + 'body', + [io.StringIO("foo"), io.BytesIO(b"foo")], +) +def test_body_streams_position_reset(aws_request_compressor, body): + aws_request_compressor.compress( + COMPRESSION_CONFIG_0_BYTES, {'body': body}, OP_WITH_COMPRESSION + ) + assert body.tell() == 0 + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index c868615b8d..6447077bb0 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1689,6 +1689,68 @@ def test_client_close_context_manager(self): self.endpoint.close.assert_called_once_with() + def test_request_compression_client_config(self): + creator = self.create_client_creator() + service_client = creator.create_client( + 'myservice', + 'us-west-2', + credentials=self.credentials, + client_config=botocore.config.Config( + disable_request_compression=True, + request_min_compression_size_bytes=100, + ), + ) + self.assertEqual( + service_client.meta.config.request_min_compression_size_bytes, 100 + ) + self.assertEqual( + service_client.meta.config.disable_request_compression, True + ) + + def test_request_compression_config_store(self): + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 100 + ) + self.config_store.set_config_variable( + 'disable_request_compression', True + ) + creator = self.create_client_creator() + service_client = creator.create_client( + 'myservice', + 'us-west-2', + credentials=self.credentials, + ) + self.assertEqual( + service_client.meta.config.request_min_compression_size_bytes, 100 + ) + self.assertEqual( + service_client.meta.config.disable_request_compression, True + ) + + def test_request_compression_client_config_overrides_config_store(self): + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 100 + ) + self.config_store.set_config_variable( + 'disable_request_compression', True + ) + creator = self.create_client_creator() + service_client = creator.create_client( + 'myservice', + 'us-west-2', + credentials=self.credentials, + client_config=botocore.config.Config( + disable_request_compression=False, + request_min_compression_size_bytes=0, + ), + ) + self.assertEqual( + service_client.meta.config.request_min_compression_size_bytes, 0 + ) + self.assertEqual( + service_client.meta.config.disable_request_compression, False + ) + class TestClientErrors(TestAutoGeneratedClient): def add_error_response(self, error_response): diff --git a/tests/unit/test_endpoint.py b/tests/unit/test_endpoint.py index a2dc45755a..33930b7746 100644 --- a/tests/unit/test_endpoint.py +++ b/tests/unit/test_endpoint.py @@ -59,6 +59,7 @@ def setUp(self): self.op = mock.Mock() self.op.has_streaming_output = False self.op.has_event_stream_output = False + self.op.request_compression = None self.op.metadata = {'protocol': 'json'} self.event_emitter = mock.Mock() self.event_emitter.emit.return_value = [] @@ -216,6 +217,7 @@ def setUp(self): self._operation.service_model.service_id = ServiceId('EC2') self._operation.has_streaming_output = False self._operation.has_event_stream_output = False + self._operation.request_compression = None def assert_events_emitted(self, event_emitter, expected_events): self.assertEqual( @@ -306,6 +308,7 @@ def test_reset_stream_on_retry(self): op.name = 'PutObject' op.has_streaming_output = True op.has_event_stream_output = False + op.request_compression = None op.metadata = {'protocol': 'rest-xml'} request = request_dict() request['body'] = body diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 62c8f568d9..303ed1a31a 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -208,6 +208,7 @@ def setUp(self): }, 'errors': [{'shape': 'NoSuchResourceException'}], 'documentation': 'Docs for PayloadOperation', + 'requestcompression': {'encodings': ['gzip']}, }, 'NoBodyOperation': { 'http': { @@ -521,12 +522,19 @@ def test_static_context_parameter_present(self): self.assertEqual(static_ctx_param2.name, 'booleanStaticContextParam') self.assertEqual(static_ctx_param2.value, True) - def test_static_context_parameter_abent(self): + def test_static_context_parameter_absent(self): service_model = model.ServiceModel(self.model) operation = service_model.operation_model('OperationTwo') self.assertIsInstance(operation.static_context_parameters, list) self.assertEqual(len(operation.static_context_parameters), 0) + def test_request_compression(self): + service_model = model.ServiceModel(self.model) + operation = service_model.operation_model('PayloadOperation') + self.assertEqual( + operation.request_compression, {'encodings': ['gzip']} + ) + class TestOperationModelEventStreamTypes(unittest.TestCase): def setUp(self): From 8fb9de74d440483c05fff4222221b569b9d90f1f Mon Sep 17 00:00:00 2001 From: davidlm Date: Mon, 26 Jun 2023 11:02:39 -0400 Subject: [PATCH 02/37] refactor --- botocore/args.py | 25 ++++- botocore/awsrequest.py | 22 ++-- botocore/client.py | 6 +- botocore/configprovider.py | 2 +- botocore/endpoint.py | 19 ++-- botocore/handlers.py | 47 +++------ tests/functional/test_compression.py | 132 ++++++++++++++++++++++++ tests/functional/test_context_params.py | 1 - tests/unit/test_awsrequest.py | 31 +++--- tests/unit/test_client.py | 14 ++- tests/unit/test_endpoint.py | 3 - 11 files changed, 226 insertions(+), 76 deletions(-) create mode 100644 tests/functional/test_compression.py diff --git a/botocore/args.py b/botocore/args.py index a7cb8c1e7d..db7cab0371 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -558,11 +558,34 @@ def _compute_request_compression_config(self, config_kwargs): 'request_min_compression_size_bytes' ) config_kwargs['request_min_compression_size_bytes'] = min_size + # conversion func is skipped so input validation must be done here + # regardless if the value is coming from the config store or the + # config object + self._validate_min_compression_size(min_size) if disabled is None: disabled = self._config_store.get_config_variable( 'disable_request_compression' ) - config_kwargs['disable_request_compression'] = disabled + else: + # if the user provided a value we must check if its a boolean + disabled = ensure_boolean(disabled) + config_kwargs['disable_request_compression'] = disabled + + def _validate_min_compression_size(self, min_size): + if min_size is not None: + try: + min_size = int(min_size) + except ValueError: + raise botocore.exceptions.InvalidConfigError( + error_msg=f"Invalid value '{min_size}' for " + "request_min_compression_size_bytes. Value must be an integer." + ) + if not 0 <= min_size <= 1048576: + raise botocore.exceptions.InvalidConfigError( + error_msg=f"Invalid value '{min_size}' for " + "request_min_compression_size_bytes. Value must be between 0 " + "and 1048576." + ) def _ensure_boolean(self, val): if isinstance(val, bool): diff --git a/botocore/awsrequest.py b/botocore/awsrequest.py index 45b408443c..f8b973f1af 100644 --- a/botocore/awsrequest.py +++ b/botocore/awsrequest.py @@ -536,12 +536,11 @@ def reset_stream(self): raise UnseekableStreamError(stream_object=self.body) -class AWSRequestCompressor: +class RequestCompressor: """A class that can compress the body of an ``AWSRequest``.""" def compress(self, config, request_dict, operation_model): - """Compresses the request body using the specified encodings if conditions - are met. + """Compresses the request body using the specified encodings. Check if the request should be compressed based on the contents of its body and config settings. Set or append the `Content-Encoding` header @@ -554,14 +553,11 @@ def compress(self, config, request_dict, operation_model): for encoding in encodings: encoder = getattr(self, f'_{encoding}_compress_body', None) if encoder is not None: - if 'Content-Encoding' not in headers: + ce_header = headers.get('Content-Encoding') + if ce_header is None: headers['Content-Encoding'] = encoding - elif encoding not in headers['Content-Encoding'].split( - ',' - ): - headers[ - 'Content-Encoding' - ] = f'{headers["Content-Encoding"]},{encoding}' + elif encoding not in ce_header.split(','): + headers['Content-Encoding'] = f'{ce_header},{encoding}' request_dict['body'] = encoder(body) if 'headers' not in request_dict: request_dict['headers'] = headers @@ -597,7 +593,11 @@ def _gzip_compress_fileobj(self, body): return compressed_obj def _should_compress_request(self, config, body, operation_model): - if config.disable_request_compression is not True: + if ( + config.disable_request_compression is not True + and config.signature_version != 'v2' + and operation_model.request_compression is not None + ): # Request is compressed no matter the content length if it has a streaming input. # However, if the stream has the `requiresLength` trait it is NOT compressed. if operation_model.has_streaming_input: diff --git a/botocore/client.py b/botocore/client.py index ec34bf7c0f..b884bff0af 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -15,7 +15,7 @@ from botocore import waiter, xform_name from botocore.args import ClientArgsCreator from botocore.auth import AUTH_TYPE_MAPS -from botocore.awsrequest import prepare_request_dict +from botocore.awsrequest import RequestCompressor, prepare_request_dict from botocore.config import Config from botocore.discovery import ( EndpointDiscoveryHandler, @@ -72,6 +72,7 @@ 's3v4', ) ) +REQUEST_COMPRESSOR = RequestCompressor() logger = logging.getLogger(__name__) @@ -955,6 +956,9 @@ def _make_api_call(self, operation_name, api_params): if event_response is not None: http, parsed_response = event_response else: + REQUEST_COMPRESSOR.compress( + self.meta.config, request_dict, operation_model + ) apply_request_checksum(request_dict) http, parsed_response = self._make_request( operation_model, request_dict, request_context diff --git a/botocore/configprovider.py b/botocore/configprovider.py index f8a0b072f2..1f9155e320 100644 --- a/botocore/configprovider.py +++ b/botocore/configprovider.py @@ -151,7 +151,7 @@ 'request_min_compression_size_bytes', 'AWS_REQUEST_MIN_COMPRESSION_SIZE_BYTES', 10240, - int, + None, ), 'disable_request_compression': ( 'disable_request_compression', diff --git a/botocore/endpoint.py b/botocore/endpoint.py index 2230254bed..adc622c25a 100644 --- a/botocore/endpoint.py +++ b/botocore/endpoint.py @@ -20,7 +20,7 @@ import uuid from botocore import parsers -from botocore.awsrequest import AWSRequestCompressor, create_request_object +from botocore.awsrequest import create_request_object from botocore.exceptions import HTTPClientError from botocore.history import get_global_history_recorder from botocore.hooks import first_non_none_response @@ -75,8 +75,14 @@ def convert_to_response_dict(http_response, operation_model): class Endpoint: - """Represents an endpoint for a particular service in a specific region. - Only an endpoint can make requests. + """ + Represents an endpoint for a particular service in a specific + region. Only an endpoint can make requests. + + :ivar service: The Service object that describes this endpoints + service. + :ivar host: The fully qualified endpoint hostname. + :ivar session: The session object. """ def __init__( @@ -97,7 +103,6 @@ def __init__( self.http_session = http_session if self.http_session is None: self.http_session = URLLib3Session() - self._request_compressor = AWSRequestCompressor() def __repr__(self): return f'{self._endpoint_prefix}({self.host})' @@ -116,12 +121,6 @@ def make_request(self, operation_model, request_dict): def create_request(self, params, operation_model=None): request = create_request_object(params) if operation_model: - if operation_model.request_compression: - self._request_compressor.compress( - params['context']['client_config'], - request, - operation_model, - ) request.stream_output = any( [ operation_model.has_streaming_output, diff --git a/botocore/handlers.py b/botocore/handlers.py index 92258d212b..6f4b7a4eab 100644 --- a/botocore/handlers.py +++ b/botocore/handlers.py @@ -28,7 +28,6 @@ import botocore import botocore.auth from botocore import utils -from botocore.awsrequest import AWSRequestCompressor from botocore.compat import ( ETree, OrderedDict, @@ -1151,10 +1150,10 @@ def remove_content_type_header_for_presigning(request, **kwargs): def urlencode_body(model, params, context, **kwargs): - """Urlencode the request body if it is a dictionary. + """Url encode the request body if it is a dictionary. - This is used for services like S3 that require the body to be urlencoded - when the body is a dictionary. + This is used for services using the query protocol. The body must be serialized + as a urlencoded string before it can be compressed. """ body = params.get('body') if ( @@ -1167,17 +1166,6 @@ def urlencode_body(model, params, context, **kwargs): ) -def compress_request(model, params, context, **kwargs): - body = params.get('body') - config = context['client_config'] - if config.signature_version == 'v2': - logger.debug( - "Request compression is not supported for signature version 2" - ) - elif model.request_compression and body is not None: - AWS_REQUEST_COMPRESSOR.compress(body, context['client_config'], model) - - # This is a list of (event_name, handler). # When a Session is created, everything in this list will be # automatically registered with that Session. @@ -1237,27 +1225,13 @@ def compress_request(model, params, context, **kwargs): ('before-call.s3', add_expect_header), ('before-call.glacier', add_glacier_version), ('before-call.apigateway', add_accept_header), - ('before-call.ec2.CopySnapshot', inject_presigned_url_ec2), - ('before-call.rds.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.rds.CreateDBCluster', inject_presigned_url_rds), - ('before-call.rds.CopyDBSnapshot', inject_presigned_url_rds), - ('before-call.rds.CreateDBInstanceReadReplica', inject_presigned_url_rds), - ( - 'before-call.rds.StartDBInstanceAutomatedBackupsReplication', - inject_presigned_url_rds, - ), - ('before-call.neptune.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.neptune.CreateDBCluster', inject_presigned_url_rds), - ('before-call.docdb.CopyDBClusterSnapshot', inject_presigned_url_rds), - ('before-call.docdb.CreateDBCluster', inject_presigned_url_rds), - ('before-call', urlencode_body), - ('before-call', compress_request), ('before-call.s3.PutObject', conditionally_calculate_md5), ('before-call.s3.UploadPart', conditionally_calculate_md5), ('before-call.s3.DeleteObjects', escape_xml_payload), ('before-call.s3.PutBucketLifecycleConfiguration', escape_xml_payload), ('before-call.glacier.UploadArchive', add_glacier_checksums), ('before-call.glacier.UploadMultipartPart', add_glacier_checksums), + ('before-call.ec2.CopySnapshot', inject_presigned_url_ec2), ('request-created', add_retry_headers), ('request-created.machinelearning.Predict', switch_host_machinelearning), ('needs-retry.s3.UploadPartCopy', check_for_200_error, REGISTER_FIRST), @@ -1392,6 +1366,14 @@ def compress_request(model, params, context, **kwargs): # RDS ############# ('creating-client-class.rds', add_generate_db_auth_token), + ('before-call.rds.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.rds.CreateDBCluster', inject_presigned_url_rds), + ('before-call.rds.CopyDBSnapshot', inject_presigned_url_rds), + ('before-call.rds.CreateDBInstanceReadReplica', inject_presigned_url_rds), + ( + 'before-call.rds.StartDBInstanceAutomatedBackupsReplication', + inject_presigned_url_rds, + ), # RDS PresignedUrl documentation customizations ( 'docs.*.rds.CopyDBClusterSnapshot.complete-section', @@ -1416,6 +1398,8 @@ def compress_request(model, params, context, **kwargs): ############# # Neptune ############# + ('before-call.neptune.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.neptune.CreateDBCluster', inject_presigned_url_rds), # Neptune PresignedUrl documentation customizations ( 'docs.*.neptune.CopyDBClusterSnapshot.complete-section', @@ -1428,6 +1412,8 @@ def compress_request(model, params, context, **kwargs): ############# # DocDB ############# + ('before-call.docdb.CopyDBClusterSnapshot', inject_presigned_url_rds), + ('before-call.docdb.CreateDBCluster', inject_presigned_url_rds), # DocDB PresignedUrl documentation customizations ( 'docs.*.docdb.CopyDBClusterSnapshot.complete-section', @@ -1438,5 +1424,6 @@ def compress_request(model, params, context, **kwargs): AutoPopulatedParam('PreSignedUrl').document_auto_populated_param, ), ('before-call', inject_api_version_header_if_needed), + ('before-call', urlencode_body), ] _add_parameter_aliases(BUILTIN_HANDLERS) diff --git a/tests/functional/test_compression.py b/tests/functional/test_compression.py new file mode 100644 index 0000000000..97f02ba4ae --- /dev/null +++ b/tests/functional/test_compression.py @@ -0,0 +1,132 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +import gzip + +import pytest + +from botocore.config import Config +from tests import ALL_SERVICES, ClientHTTPStubber, patch_load_service_model + +KNOWN_COMPRESSION_ENCODINGS = 'gzip' + +FAKE_MODEL = { + "version": "2.0", + "documentation": "", + "metadata": { + "apiVersion": "2020-02-02", + "endpointPrefix": "otherservice", + "protocol": "query", + "serviceFullName": "Other Service", + "serviceId": "Other Service", + "signatureVersion": "v4", + "signingName": "otherservice", + "uid": "otherservice-2020-02-02", + }, + "operations": { + "MockOperation": { + "name": "MockOperation", + "http": {"method": "POST", "requestUri": "/"}, + "input": {"shape": "MockOperationRequest"}, + "documentation": "", + "requestcompression": { + "encodings": ["gzip"], + }, + }, + }, + "shapes": { + "MockOpParamList": { + "type": "list", + "member": {"shape": "MockOpParam"}, + }, + "MockOpParam": { + "type": "structure", + "members": {"MockOpParam": {"shape": "MockOpParamValue"}}, + }, + "MockOpParamValue": { + "type": "string", + }, + "MockOperationRequest": { + "type": "structure", + "required": ["MockOpParamList"], + "members": { + "MockOpParamList": { + "shape": "MockOpParamList", + "documentation": "", + }, + }, + }, + }, +} + +FAKE_RULESET = { + "version": "1.0", + "parameters": {}, + "rules": [ + { + "conditions": [], + "type": "endpoint", + "endpoint": { + "url": "https://foo.bar", + "properties": {}, + "headers": {}, + }, + } + ], +} + + +def _all_compression_operations(): + for service_model in ALL_SERVICES: + for operation_name in service_model.operation_names: + operation_model = service_model.operation_model(operation_name) + if operation_model.request_compression is not None: + yield operation_model + + +@pytest.mark.parametrize("operation_model", _all_compression_operations()) +def test_no_unknown_compression_encodings(operation_model): + for encoding in operation_model.request_compression["encodings"]: + assert ( + encoding in KNOWN_COMPRESSION_ENCODINGS + ), f"Found unknown compression encoding '{encoding}' in operation {operation_model.name}" + + +def test_compression(patched_session, monkeypatch): + patch_load_service_model( + patched_session, monkeypatch, FAKE_MODEL, FAKE_RULESET + ) + client = patched_session.create_client( + 'otherservice', + region_name='us-west-2', + config=Config(request_min_compression_size_bytes=100), + ) + with ClientHTTPStubber(client, strict=True) as http_stubber: + http_stubber.add_response( + status=200, + body=b'successRequest processed successfully', + ) + client.mock_operation( + MockOpParamList=[ + {"MockOpParam": f"MockOpParamValue{i}"} for i in range(1, 21) + ] + ) + serialized_body = b"&".join( + [ + f"MockOpParamList.member.{i}.MockOpParam=MockOpParamValue{i}".encode() + for i in range(1, 21) + ] + ) + assert serialized_body in gzip.decompress( + http_stubber.requests[0].body + ) diff --git a/tests/functional/test_context_params.py b/tests/functional/test_context_params.py index 03eb9ae965..e3c501ead9 100644 --- a/tests/functional/test_context_params.py +++ b/tests/functional/test_context_params.py @@ -398,7 +398,6 @@ def test_static_context_param_sent_to_endpoint_resolver( client = patched_session.create_client( service_name, region_name='us-east-1' ) - with ClientHTTPStubber(client, strict=True) as http_stubber: http_stubber.add_response(status=200) with mock.patch.object( diff --git a/tests/unit/test_awsrequest.py b/tests/unit/test_awsrequest.py index a39b60915d..53979b6b70 100644 --- a/tests/unit/test_awsrequest.py +++ b/tests/unit/test_awsrequest.py @@ -25,13 +25,13 @@ AWSHTTPConnection, AWSHTTPSConnection, AWSRequest, - AWSRequestCompressor, AWSResponse, HeadersDict, + RequestCompressor, create_request_object, prepare_request_dict, ) -from botocore.compat import HTTPHeaders, file_type +from botocore.compat import file_type from botocore.config import Config from botocore.exceptions import UnseekableStreamError from tests import mock, unittest @@ -168,7 +168,6 @@ def _streaming_op_with_compression_requires_length(): disable_request_compression=False, request_min_compression_size_bytes=0, ) -GZIP_ENCODINGS = ['gzip'] def request_dict(): @@ -186,21 +185,19 @@ def request_dict_with_content_encoding_header(): @pytest.fixture(scope="module") -def aws_request_compressor(): - return AWSRequestCompressor() +def request_compressor(): + return RequestCompressor() -@pytest.fixture(scope="module") -def headers(): - return HTTPHeaders() +COMPRESSION_HEADERS = {'gzip': b'\x1f\x8b'} -def _assert_compression(is_compressed, body): +def _assert_compression(is_compressed, body, encoding): if hasattr(body, 'read'): header = body.read(2) else: header = body[:2] - assert is_compressed == (header == b'\x1f\x8b') + assert is_compressed == (header == COMPRESSION_HEADERS.get(encoding)) class TestAWSRequest(unittest.TestCase): @@ -1004,15 +1001,15 @@ def test_iteration(self): ], ) def test_compress( - aws_request_compressor, + request_compressor, config, request_dict, operation_model, is_compressed, encoding, ): - aws_request_compressor.compress(config, request_dict, operation_model) - _assert_compression(is_compressed, request_dict['body']) + request_compressor.compress(config, request_dict, operation_model) + _assert_compression(is_compressed, request_dict['body'], encoding) assert ( 'headers' in request_dict and 'Content-Encoding' in request_dict['headers'] @@ -1021,9 +1018,9 @@ def test_compress( @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) -def test_compress_bad_types(aws_request_compressor, body): +def test_compress_bad_types(request_compressor, body): request_dict = {'body': body} - aws_request_compressor.compress( + request_compressor.compress( COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION ) assert request_dict['body'] == body @@ -1033,8 +1030,8 @@ def test_compress_bad_types(aws_request_compressor, body): 'body', [io.StringIO("foo"), io.BytesIO(b"foo")], ) -def test_body_streams_position_reset(aws_request_compressor, body): - aws_request_compressor.compress( +def test_body_streams_position_reset(request_compressor, body): + request_compressor.compress( COMPRESSION_CONFIG_0_BYTES, {'body': body}, OP_WITH_COMPRESSION ) assert body.tell() == 0 diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 6447077bb0..b0b57f1128 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -27,6 +27,7 @@ from botocore.endpoint import DEFAULT_TIMEOUT from botocore.errorfactory import ClientExceptionsFactory from botocore.exceptions import ( + InvalidConfigError, InvalidMaxRetryAttemptsError, InvalidRetryConfigurationError, InvalidRetryModeError, @@ -1309,7 +1310,6 @@ def test_events_are_per_client(self): second_client = creator.create_client( 'myservice', 'us-west-2', credentials=self.credentials ) - first_client.meta.events.register('before-call', first_handler) second_client.meta.events.register('before-call', second_handler) @@ -1751,6 +1751,18 @@ def test_request_compression_client_config_overrides_config_store(self): service_client.meta.config.disable_request_compression, False ) + def test_bad_request_min_compression_size_bytes(self): + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 'foo' + ) + creator = self.create_client_creator() + with self.assertRaises(InvalidConfigError): + creator.create_client( + 'myservice', + 'us-west-2', + credentials=self.credentials, + ) + class TestClientErrors(TestAutoGeneratedClient): def add_error_response(self, error_response): diff --git a/tests/unit/test_endpoint.py b/tests/unit/test_endpoint.py index 33930b7746..a2dc45755a 100644 --- a/tests/unit/test_endpoint.py +++ b/tests/unit/test_endpoint.py @@ -59,7 +59,6 @@ def setUp(self): self.op = mock.Mock() self.op.has_streaming_output = False self.op.has_event_stream_output = False - self.op.request_compression = None self.op.metadata = {'protocol': 'json'} self.event_emitter = mock.Mock() self.event_emitter.emit.return_value = [] @@ -217,7 +216,6 @@ def setUp(self): self._operation.service_model.service_id = ServiceId('EC2') self._operation.has_streaming_output = False self._operation.has_event_stream_output = False - self._operation.request_compression = None def assert_events_emitted(self, event_emitter, expected_events): self.assertEqual( @@ -308,7 +306,6 @@ def test_reset_stream_on_retry(self): op.name = 'PutObject' op.has_streaming_output = True op.has_event_stream_output = False - op.request_compression = None op.metadata = {'protocol': 'rest-xml'} request = request_dict() request['body'] = body From b689db83b11d179698882e51ccc228fada511409 Mon Sep 17 00:00:00 2001 From: davidlm Date: Mon, 26 Jun 2023 14:55:09 -0400 Subject: [PATCH 03/37] add handler test --- botocore/args.py | 14 +++++++++----- botocore/awsrequest.py | 2 +- botocore/handlers.py | 6 +++--- botocore/signers.py | 1 + tests/functional/test_compression.py | 11 +++++++---- tests/functional/test_context_params.py | 1 + tests/unit/test_awsrequest.py | 8 ++++---- tests/unit/test_client.py | 1 + tests/unit/test_handlers.py | 19 +++++++++++++++++++ 9 files changed, 46 insertions(+), 17 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index db7cab0371..7164e8975b 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -573,18 +573,22 @@ def _compute_request_compression_config(self, config_kwargs): def _validate_min_compression_size(self, min_size): if min_size is not None: + error_msg_base = ( + f'Invalid value "{min_size}" for ' + 'request_min_compression_size_bytes. ' + ) try: min_size = int(min_size) except ValueError: raise botocore.exceptions.InvalidConfigError( - error_msg=f"Invalid value '{min_size}' for " - "request_min_compression_size_bytes. Value must be an integer." + error_msg=f'{error_msg_base}Value must be an integer.' ) if not 0 <= min_size <= 1048576: raise botocore.exceptions.InvalidConfigError( - error_msg=f"Invalid value '{min_size}' for " - "request_min_compression_size_bytes. Value must be between 0 " - "and 1048576." + error_msg=( + f'{error_msg_base}Value must ' + 'be between 0 and 1048576.' + ) ) def _ensure_boolean(self, val): diff --git a/botocore/awsrequest.py b/botocore/awsrequest.py index f8b973f1af..d12f935945 100644 --- a/botocore/awsrequest.py +++ b/botocore/awsrequest.py @@ -479,7 +479,7 @@ def prepare(self): @property def body(self): - body = self._request_preparer._prepare_body(self) + body = self.prepare().body if isinstance(body, str): body = body.encode('utf-8') return body diff --git a/botocore/handlers.py b/botocore/handlers.py index 6f4b7a4eab..cfaf401e08 100644 --- a/botocore/handlers.py +++ b/botocore/handlers.py @@ -1150,10 +1150,10 @@ def remove_content_type_header_for_presigning(request, **kwargs): def urlencode_body(model, params, context, **kwargs): - """Url encode the request body if it is a dictionary. + """URL-encode the request body if it is a dictionary. - This is used for services using the query protocol. The body must be serialized - as a urlencoded string before it can be compressed. + This is used for services using the query protocol. The body must be + serialized as a URL-encoded string before it can be compressed. """ body = params.get('body') if ( diff --git a/botocore/signers.py b/botocore/signers.py index 0e00c094a8..0acbf68463 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -185,6 +185,7 @@ def sign( ) else: raise e + auth.add_auth(request) def _choose_signer(self, operation_name, signing_type, context): diff --git a/tests/functional/test_compression.py b/tests/functional/test_compression.py index 97f02ba4ae..b5289fe07e 100644 --- a/tests/functional/test_compression.py +++ b/tests/functional/test_compression.py @@ -18,7 +18,7 @@ from botocore.config import Config from tests import ALL_SERVICES, ClientHTTPStubber, patch_load_service_model -KNOWN_COMPRESSION_ENCODINGS = 'gzip' +KNOWN_COMPRESSION_ENCODINGS = ("gzip",) FAKE_MODEL = { "version": "2.0", @@ -107,14 +107,17 @@ def test_compression(patched_session, monkeypatch): patched_session, monkeypatch, FAKE_MODEL, FAKE_RULESET ) client = patched_session.create_client( - 'otherservice', - region_name='us-west-2', + "otherservice", + region_name="us-west-2", config=Config(request_min_compression_size_bytes=100), ) with ClientHTTPStubber(client, strict=True) as http_stubber: http_stubber.add_response( status=200, - body=b'successRequest processed successfully', + body=( + b"success" + b"Request processed successfully" + ), ) client.mock_operation( MockOpParamList=[ diff --git a/tests/functional/test_context_params.py b/tests/functional/test_context_params.py index e3c501ead9..03eb9ae965 100644 --- a/tests/functional/test_context_params.py +++ b/tests/functional/test_context_params.py @@ -398,6 +398,7 @@ def test_static_context_param_sent_to_endpoint_resolver( client = patched_session.create_client( service_name, region_name='us-east-1' ) + with ClientHTTPStubber(client, strict=True) as http_stubber: http_stubber.add_response(status=200) with mock.patch.object( diff --git a/tests/unit/test_awsrequest.py b/tests/unit/test_awsrequest.py index 53979b6b70..bba00332c1 100644 --- a/tests/unit/test_awsrequest.py +++ b/tests/unit/test_awsrequest.py @@ -184,7 +184,7 @@ def request_dict_with_content_encoding_header(): } -@pytest.fixture(scope="module") +@pytest.fixture(scope='module') def request_compressor(): return RequestCompressor() @@ -989,14 +989,14 @@ def test_iteration(self): request_dict_with_content_encoding_header(), OP_UNKNOWN_COMPRESSION, False, - "foo", + 'foo', ), ( COMPRESSION_CONFIG_128_BYTES, request_dict_with_content_encoding_header(), OP_WITH_COMPRESSION, True, - "gzip", + 'gzip', ), ], ) @@ -1028,7 +1028,7 @@ def test_compress_bad_types(request_compressor, body): @pytest.mark.parametrize( 'body', - [io.StringIO("foo"), io.BytesIO(b"foo")], + [io.StringIO('foo'), io.BytesIO(b'foo')], ) def test_body_streams_position_reset(request_compressor, body): request_compressor.compress( diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index b0b57f1128..3a9529665d 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1310,6 +1310,7 @@ def test_events_are_per_client(self): second_client = creator.create_client( 'myservice', 'us-west-2', credentials=self.credentials ) + first_client.meta.events.register('before-call', first_handler) second_client.meta.events.register('before-call', second_handler) diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py index 2701c32efe..361389e921 100644 --- a/tests/unit/test_handlers.py +++ b/tests/unit/test_handlers.py @@ -1089,6 +1089,25 @@ def test_set_operation_specific_signer_s3v4(auth_type, expected_response): assert response == expected_response +@pytest.mark.parametrize( + 'protocol, signature_version, params, expected_body', + [ + ('query', 'v4', {'body': {'foo': 'bar'}}, b'foo=bar'), + ('query', 'v4', {}, None), + ('json', 'v4', {'body': {'foo': 'bar'}}, {'foo': 'bar'}), + ('query', 'v2', {'body': {'foo': 'bar'}}, {'foo': 'bar'}), + ('query', 'v4', {'body': 'foo=bar'}, 'foo=bar'), + ], +) +def test_urlencode_body(protocol, signature_version, params, expected_body): + operation_def = {'name': 'CreateFoo'} + service_def = {'metadata': {'protocol': protocol}, 'shapes': {}} + model = OperationModel(operation_def, ServiceModel(service_def)) + context = {'client_config': Config(signature_version=signature_version)} + handlers.urlencode_body(model, params, context) + assert params.get('body') == expected_body + + class TestConvertStringBodyToFileLikeObject(BaseSessionTest): def assert_converts_to_file_like_object_with_bytes(self, body, body_bytes): params = {'Body': body} From 6cdd1ed6593b863d132e6511dada36d7b9bda12b Mon Sep 17 00:00:00 2001 From: davidlm Date: Fri, 30 Jun 2023 11:32:27 -0400 Subject: [PATCH 04/37] Another round of refactoring. Move compression to a separate file and query body serialization to a utility --- botocore/awsrequest.py | 88 ------ botocore/client.py | 10 +- botocore/compress.py | 114 ++++++++ botocore/configprovider.py | 2 + botocore/handlers.py | 19 -- botocore/utils.py | 14 + .../{test_compression.py => test_compress.py} | 7 +- tests/unit/test_awsrequest.py | 239 ----------------- tests/unit/test_client.py | 6 +- tests/unit/test_compress.py | 250 ++++++++++++++++++ tests/unit/test_handlers.py | 19 -- tests/unit/test_utils.py | 23 ++ 12 files changed, 417 insertions(+), 374 deletions(-) create mode 100644 botocore/compress.py rename tests/functional/{test_compression.py => test_compress.py} (95%) create mode 100644 tests/unit/test_compress.py diff --git a/botocore/awsrequest.py b/botocore/awsrequest.py index d12f935945..f00a0dde57 100644 --- a/botocore/awsrequest.py +++ b/botocore/awsrequest.py @@ -12,8 +12,6 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import functools -import gzip -import io import logging from collections.abc import Mapping @@ -536,92 +534,6 @@ def reset_stream(self): raise UnseekableStreamError(stream_object=self.body) -class RequestCompressor: - """A class that can compress the body of an ``AWSRequest``.""" - - def compress(self, config, request_dict, operation_model): - """Compresses the request body using the specified encodings. - - Check if the request should be compressed based on the contents of its - body and config settings. Set or append the `Content-Encoding` header - with the matched encoding if not present. - """ - body = request_dict['body'] - if self._should_compress_request(config, body, operation_model): - encodings = operation_model.request_compression['encodings'] - headers = request_dict.get('headers', {}) - for encoding in encodings: - encoder = getattr(self, f'_{encoding}_compress_body', None) - if encoder is not None: - ce_header = headers.get('Content-Encoding') - if ce_header is None: - headers['Content-Encoding'] = encoding - elif encoding not in ce_header.split(','): - headers['Content-Encoding'] = f'{ce_header},{encoding}' - request_dict['body'] = encoder(body) - if 'headers' not in request_dict: - request_dict['headers'] = headers - else: - logger.debug( - 'Unsupported compression encoding: %s' % encoding - ) - - def _gzip_compress_body(self, body): - if isinstance(body, str): - return gzip.compress(body.encode('utf-8')) - elif isinstance(body, (bytes, bytearray)): - return gzip.compress(body) - elif hasattr(body, 'read'): - if hasattr(body, 'seek') and hasattr(body, 'tell'): - current_position = body.tell() - compressed_obj = self._gzip_compress_fileobj(body) - body.seek(current_position) - return compressed_obj - return self._gzip_compress_fileobj(body) - - def _gzip_compress_fileobj(self, body): - compressed_obj = io.BytesIO() - with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: - while True: - chunk = body.read(8192) - if not chunk: - break - if isinstance(chunk, str): - chunk = chunk.encode('utf-8') - gz.write(chunk) - compressed_obj.seek(0) - return compressed_obj - - def _should_compress_request(self, config, body, operation_model): - if ( - config.disable_request_compression is not True - and config.signature_version != 'v2' - and operation_model.request_compression is not None - ): - # Request is compressed no matter the content length if it has a streaming input. - # However, if the stream has the `requiresLength` trait it is NOT compressed. - if operation_model.has_streaming_input: - return ( - 'requiresLength' - not in operation_model.get_streaming_input().metadata - ) - return ( - config.request_min_compression_size_bytes - <= self._get_body_size(body) - ) - return False - - def _get_body_size(self, body): - size = botocore.utils.determine_content_length(body) - if size is None: - logger.debug( - 'Unable to get length of the request body: %s. Not compressing.' - % body - ) - return -1 - return size - - class AWSResponse: """A data class representing an HTTP response. diff --git a/botocore/client.py b/botocore/client.py index b884bff0af..b03c26df97 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -15,7 +15,8 @@ from botocore import waiter, xform_name from botocore.args import ClientArgsCreator from botocore.auth import AUTH_TYPE_MAPS -from botocore.awsrequest import RequestCompressor, prepare_request_dict +from botocore.awsrequest import prepare_request_dict +from botocore.compress import RequestCompressor from botocore.config import Config from botocore.discovery import ( EndpointDiscoveryHandler, @@ -47,6 +48,7 @@ S3RegionRedirectorv2, ensure_boolean, get_service_module_name, + urlencode_query_body, ) # Keep these imported. There's pre-existing code that uses: @@ -72,7 +74,6 @@ 's3v4', ) ) -REQUEST_COMPRESSOR = RequestCompressor() logger = logging.getLogger(__name__) @@ -956,7 +957,10 @@ def _make_api_call(self, operation_name, api_params): if event_response is not None: http, parsed_response = event_response else: - REQUEST_COMPRESSOR.compress( + urlencode_query_body( + request_dict, operation_model, self.meta.config + ) + RequestCompressor.compress( self.meta.config, request_dict, operation_model ) apply_request_checksum(request_dict) diff --git a/botocore/compress.py b/botocore/compress.py new file mode 100644 index 0000000000..52eaa86b88 --- /dev/null +++ b/botocore/compress.py @@ -0,0 +1,114 @@ +# Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +import gzip +import io +import logging + +from botocore.utils import determine_content_length + +logger = logging.getLogger(__name__) + + +class RequestCompressor: + """A class that can compress the body of an ``AWSRequest``.""" + + @classmethod + def compress(cls, config, request_dict, operation_model): + """Compresses the request body using the specified encodings. + + Check if the request should be compressed based on the contents of its + body and config settings. Set or append the `Content-Encoding` header + with the matched encoding if not present. + """ + body = request_dict['body'] + if cls._should_compress_request(config, body, operation_model): + encodings = operation_model.request_compression['encodings'] + headers = request_dict.get('headers', {}) + for encoding in encodings: + encoder = getattr(cls, f'_{encoding}_compress_body', None) + if encoder is not None: + ce_header = headers.get('Content-Encoding') + if ce_header is None: + headers['Content-Encoding'] = encoding + elif encoding not in ce_header.split(','): + headers['Content-Encoding'] = f'{ce_header},{encoding}' + logger.debug( + 'Compressing request with %s encoding', encoding + ) + request_dict['body'] = encoder(body) + if 'headers' not in request_dict: + request_dict['headers'] = headers + else: + logger.debug( + 'Unsupported compression encoding: %s' % encoding + ) + + @classmethod + def _gzip_compress_body(cls, body): + if isinstance(body, str): + return gzip.compress(body.encode('utf-8')) + elif isinstance(body, (bytes, bytearray)): + return gzip.compress(body) + elif hasattr(body, 'read'): + if hasattr(body, 'seek') and hasattr(body, 'tell'): + current_position = body.tell() + compressed_obj = cls._gzip_compress_fileobj(body) + body.seek(current_position) + return compressed_obj + return cls._gzip_compress_fileobj(body) + + @staticmethod + def _gzip_compress_fileobj(body): + compressed_obj = io.BytesIO() + with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: + while True: + chunk = body.read(8192) + if not chunk: + break + if isinstance(chunk, str): + chunk = chunk.encode('utf-8') + gz.write(chunk) + compressed_obj.seek(0) + return compressed_obj + + @classmethod + def _should_compress_request(cls, config, body, operation_model): + if ( + config.disable_request_compression is not True + and config.signature_version != 'v2' + and operation_model.request_compression is not None + ): + # Request is compressed no matter the content length if it has a streaming input. + # However, if the stream has the `requiresLength` trait it is NOT compressed. + if operation_model.has_streaming_input: + return ( + 'requiresLength' + not in operation_model.get_streaming_input().metadata + ) + return ( + config.request_min_compression_size_bytes + <= cls._get_body_size(body) + ) + return False + + @staticmethod + def _get_body_size(body): + size = determine_content_length(body) + if size is None: + logger.debug( + 'Unable to get length of the request body: %s. Not compressing.' + % body + ) + return -1 + return size diff --git a/botocore/configprovider.py b/botocore/configprovider.py index 1f9155e320..778507775d 100644 --- a/botocore/configprovider.py +++ b/botocore/configprovider.py @@ -147,6 +147,8 @@ # whatever the defaults are in _retry.json. 'max_attempts': ('max_attempts', 'AWS_MAX_ATTEMPTS', None, int), 'user_agent_appid': ('sdk_ua_app_id', 'AWS_SDK_UA_APP_ID', None, None), + # This must be a parsable integer between 0 and 1048576, but validation + # is performed during client initialization instead of here. 'request_min_compression_size_bytes': ( 'request_min_compression_size_bytes', 'AWS_REQUEST_MIN_COMPRESSION_SIZE_BYTES', diff --git a/botocore/handlers.py b/botocore/handlers.py index cfaf401e08..e3eb5c886b 100644 --- a/botocore/handlers.py +++ b/botocore/handlers.py @@ -38,7 +38,6 @@ quote, unquote, unquote_str, - urlencode, urlsplit, urlunsplit, ) @@ -1149,23 +1148,6 @@ def remove_content_type_header_for_presigning(request, **kwargs): del request.headers['Content-Type'] -def urlencode_body(model, params, context, **kwargs): - """URL-encode the request body if it is a dictionary. - - This is used for services using the query protocol. The body must be - serialized as a URL-encoded string before it can be compressed. - """ - body = params.get('body') - if ( - context['client_config'].signature_version != 'v2' - and model.service_model.protocol == 'query' - and isinstance(body, dict) - ): - params['body'] = urlencode(body, doseq=True, encoding='utf-8').encode( - 'utf-8' - ) - - # This is a list of (event_name, handler). # When a Session is created, everything in this list will be # automatically registered with that Session. @@ -1424,6 +1406,5 @@ def urlencode_body(model, params, context, **kwargs): AutoPopulatedParam('PreSignedUrl').document_auto_populated_param, ), ('before-call', inject_api_version_header_if_needed), - ('before-call', urlencode_body), ] _add_parameter_aliases(BUILTIN_HANDLERS) diff --git a/botocore/utils.py b/botocore/utils.py index 266d204629..9d35c61211 100644 --- a/botocore/utils.py +++ b/botocore/utils.py @@ -55,6 +55,7 @@ get_tzinfo_options, json, quote, + urlencode, urlparse, urlsplit, urlunsplit, @@ -3429,3 +3430,16 @@ def _serialize_if_needed(self, value, iso=False): 'stepfunctions': 'sfn', 'storagegateway': 'storage-gateway', } + + +def urlencode_query_body(request_dict, operation_model, config): + """URL encode a request's body if it using the query protocol.""" + body = request_dict['body'] + if ( + operation_model.service_model.protocol == 'query' + and isinstance(body, dict) + and config.signature_version != 'v2' + ): + request_dict['body'] = urlencode( + body, doseq=True, encoding='utf-8' + ).encode('utf-8') diff --git a/tests/functional/test_compression.py b/tests/functional/test_compress.py similarity index 95% rename from tests/functional/test_compression.py rename to tests/functional/test_compress.py index b5289fe07e..3ad0465240 100644 --- a/tests/functional/test_compression.py +++ b/tests/functional/test_compress.py @@ -97,9 +97,10 @@ def _all_compression_operations(): @pytest.mark.parametrize("operation_model", _all_compression_operations()) def test_no_unknown_compression_encodings(operation_model): for encoding in operation_model.request_compression["encodings"]: - assert ( - encoding in KNOWN_COMPRESSION_ENCODINGS - ), f"Found unknown compression encoding '{encoding}' in operation {operation_model.name}" + assert encoding in KNOWN_COMPRESSION_ENCODINGS, ( + f"Found unknown compression encoding '{encoding}' " + f"in operation {operation_model.name}" + ) def test_compression(patched_session, monkeypatch): diff --git a/tests/unit/test_awsrequest.py b/tests/unit/test_awsrequest.py index bba00332c1..a29a846116 100644 --- a/tests/unit/test_awsrequest.py +++ b/tests/unit/test_awsrequest.py @@ -27,12 +27,10 @@ AWSRequest, AWSResponse, HeadersDict, - RequestCompressor, create_request_object, prepare_request_dict, ) from botocore.compat import file_type -from botocore.config import Config from botocore.exceptions import UnseekableStreamError from tests import mock, unittest @@ -109,97 +107,6 @@ def tell(self): return self._stream.tell() -def _op_with_compression(): - op = mock.Mock() - op.request_compression = {'encodings': ['gzip']} - op.has_streaming_input = False - return op - - -def _op_unknown_compression(): - op = mock.Mock() - op.request_compression = {'encodings': ['foo']} - op.has_streaming_input = None - return op - - -def _op_without_compression(): - op = mock.Mock() - op.request_compression = None - op.has_streaming_input = False - return op - - -def _streaming_op_with_compression(): - op = _op_with_compression() - op.has_streaming_input = True - streaming_shape = mock.Mock() - streaming_shape.metadata = {} - op.get_streaming_input.return_value = streaming_shape - return op - - -def _streaming_op_with_compression_requires_length(): - op = _streaming_op_with_compression() - streaming_shape = mock.Mock() - streaming_shape.metadata = {'requiresLength': True} - op.get_streaming_input.return_value = streaming_shape - return op - - -OP_WITH_COMPRESSION = _op_with_compression() -OP_UNKNOWN_COMPRESSION = _op_unknown_compression() -STREAMING_OP_WITH_COMPRESSION = _streaming_op_with_compression() -REQUEST_BODY = ( - b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' - b'&MetricData.member.1.MetricName=metric&MetricData.member.1.Unit=Bytes' - b'&MetricData.member.1.Value=128' -) - -DEFAULT_COMPRESSION_CONFIG = Config( - disable_request_compression=False, - request_min_compression_size_bytes=10420, -) -COMPRESSION_CONFIG_128_BYTES = Config( - disable_request_compression=False, - request_min_compression_size_bytes=128, -) -COMPRESSION_CONFIG_0_BYTES = Config( - disable_request_compression=False, - request_min_compression_size_bytes=0, -) - - -def request_dict(): - return { - 'body': REQUEST_BODY, - 'headers': {'foo': 'bar'}, - } - - -def request_dict_with_content_encoding_header(): - return { - 'body': REQUEST_BODY, - 'headers': {'foo': 'bar', 'Content-Encoding': 'identity'}, - } - - -@pytest.fixture(scope='module') -def request_compressor(): - return RequestCompressor() - - -COMPRESSION_HEADERS = {'gzip': b'\x1f\x8b'} - - -def _assert_compression(is_compressed, body, encoding): - if hasattr(body, 'read'): - header = body.read(2) - else: - header = body[:2] - assert is_compressed == (header == COMPRESSION_HEADERS.get(encoding)) - - class TestAWSRequest(unittest.TestCase): def setUp(self): self.tempdir = tempfile.mkdtemp() @@ -891,151 +798,5 @@ def test_iteration(self): self.assertIn(('dead', 'beef'), headers_items) -@pytest.mark.parametrize( - 'config, request_dict, operation_model, is_compressed, encoding', - [ - ( - Config( - disable_request_compression=True, - request_min_compression_size_bytes=1000, - ), - {'body': b'foo'}, - OP_WITH_COMPRESSION, - False, - None, - ), - ( - COMPRESSION_CONFIG_128_BYTES, - request_dict(), - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - Config( - disable_request_compression=False, - request_min_compression_size_bytes=256, - ), - request_dict(), - OP_WITH_COMPRESSION, - False, - None, - ), - ( - Config(request_min_compression_size_bytes=128), - request_dict(), - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - DEFAULT_COMPRESSION_CONFIG, - request_dict(), - STREAMING_OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - DEFAULT_COMPRESSION_CONFIG, - request_dict(), - _streaming_op_with_compression_requires_length(), - False, - None, - ), - ( - DEFAULT_COMPRESSION_CONFIG, - request_dict(), - _op_without_compression(), - False, - None, - ), - ( - COMPRESSION_CONFIG_128_BYTES, - request_dict(), - OP_UNKNOWN_COMPRESSION, - False, - None, - ), - ( - COMPRESSION_CONFIG_128_BYTES, - {'body': REQUEST_BODY.decode()}, - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - {'body': bytearray(REQUEST_BODY)}, - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - {'body': io.BytesIO(REQUEST_BODY)}, - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - {'body': io.StringIO(REQUEST_BODY.decode())}, - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), - OP_UNKNOWN_COMPRESSION, - False, - 'foo', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), - OP_WITH_COMPRESSION, - True, - 'gzip', - ), - ], -) -def test_compress( - request_compressor, - config, - request_dict, - operation_model, - is_compressed, - encoding, -): - request_compressor.compress(config, request_dict, operation_model) - _assert_compression(is_compressed, request_dict['body'], encoding) - assert ( - 'headers' in request_dict - and 'Content-Encoding' in request_dict['headers'] - and encoding in request_dict['headers']['Content-Encoding'] - ) == is_compressed - - -@pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) -def test_compress_bad_types(request_compressor, body): - request_dict = {'body': body} - request_compressor.compress( - COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION - ) - assert request_dict['body'] == body - - -@pytest.mark.parametrize( - 'body', - [io.StringIO('foo'), io.BytesIO(b'foo')], -) -def test_body_streams_position_reset(request_compressor, body): - request_compressor.compress( - COMPRESSION_CONFIG_0_BYTES, {'body': body}, OP_WITH_COMPRESSION - ) - assert body.tell() == 0 - - if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 3a9529665d..c06aa6c672 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1468,7 +1468,7 @@ def inject_params(params, **kwargs): # Ensure the handler passed on the correct param values. body = self.endpoint.make_request.call_args[0][1]['body'] - self.assertEqual(body['Foo'], 'zero') + self.assertIn(b'Foo=zero', body) def test_client_default_for_s3_addressing_style(self): creator = self.create_client_creator() @@ -1748,8 +1748,8 @@ def test_request_compression_client_config_overrides_config_store(self): self.assertEqual( service_client.meta.config.request_min_compression_size_bytes, 0 ) - self.assertEqual( - service_client.meta.config.disable_request_compression, False + self.assertFalse( + service_client.meta.config.disable_request_compression ) def test_bad_request_min_compression_size_bytes(self): diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py new file mode 100644 index 0000000000..08a06c7069 --- /dev/null +++ b/tests/unit/test_compress.py @@ -0,0 +1,250 @@ +# Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). You +# may not use this file except in compliance with the License. A copy of +# the License is located at +# +# http://aws.amazon.com/apache2.0/ +# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. +import io + +import pytest + +from botocore.compress import RequestCompressor +from botocore.config import Config +from tests import mock + + +def _op_with_compression(): + op = mock.Mock() + op.request_compression = {'encodings': ['gzip']} + op.has_streaming_input = False + return op + + +def _op_unknown_compression(): + op = mock.Mock() + op.request_compression = {'encodings': ['foo']} + op.has_streaming_input = None + return op + + +def _op_without_compression(): + op = mock.Mock() + op.request_compression = None + op.has_streaming_input = False + return op + + +def _streaming_op_with_compression(): + op = _op_with_compression() + op.has_streaming_input = True + streaming_shape = mock.Mock() + streaming_shape.metadata = {} + op.get_streaming_input.return_value = streaming_shape + return op + + +def _streaming_op_with_compression_requires_length(): + op = _streaming_op_with_compression() + streaming_shape = mock.Mock() + streaming_shape.metadata = {'requiresLength': True} + op.get_streaming_input.return_value = streaming_shape + return op + + +OP_WITH_COMPRESSION = _op_with_compression() +OP_UNKNOWN_COMPRESSION = _op_unknown_compression() +STREAMING_OP_WITH_COMPRESSION = _streaming_op_with_compression() +REQUEST_BODY = ( + b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' + b'&MetricData.member.1.MetricName=metric&MetricData.member.1.Unit=Bytes' + b'&MetricData.member.1.Value=128' +) + +DEFAULT_COMPRESSION_CONFIG = Config( + disable_request_compression=False, + request_min_compression_size_bytes=10420, +) +COMPRESSION_CONFIG_128_BYTES = Config( + disable_request_compression=False, + request_min_compression_size_bytes=128, +) +COMPRESSION_CONFIG_0_BYTES = Config( + disable_request_compression=False, + request_min_compression_size_bytes=0, +) + + +def request_dict(): + return { + 'body': REQUEST_BODY, + 'headers': {'foo': 'bar'}, + } + + +def request_dict_with_content_encoding_header(): + return { + 'body': REQUEST_BODY, + 'headers': {'foo': 'bar', 'Content-Encoding': 'identity'}, + } + + +COMPRESSION_HEADERS = {'gzip': b'\x1f\x8b'} + + +def _assert_compression(is_compressed, body, encoding): + if hasattr(body, 'read'): + header = body.read(2) + else: + header = body[:2] + assert is_compressed == (header == COMPRESSION_HEADERS.get(encoding)) + + +@pytest.mark.parametrize( + 'config, request_dict, operation_model, is_compressed, encoding', + [ + ( + Config( + disable_request_compression=True, + request_min_compression_size_bytes=1000, + ), + {'body': b'foo'}, + OP_WITH_COMPRESSION, + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict(), + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + Config( + disable_request_compression=False, + request_min_compression_size_bytes=256, + ), + request_dict(), + OP_WITH_COMPRESSION, + False, + None, + ), + ( + Config(request_min_compression_size_bytes=128), + request_dict(), + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + STREAMING_OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + _streaming_op_with_compression_requires_length(), + False, + None, + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict(), + _op_without_compression(), + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict(), + OP_UNKNOWN_COMPRESSION, + False, + None, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': REQUEST_BODY.decode()}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': bytearray(REQUEST_BODY)}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': io.BytesIO(REQUEST_BODY)}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': io.StringIO(REQUEST_BODY.decode())}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict_with_content_encoding_header(), + OP_UNKNOWN_COMPRESSION, + False, + 'foo', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict_with_content_encoding_header(), + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ], +) +def test_compress( + config, + request_dict, + operation_model, + is_compressed, + encoding, +): + RequestCompressor.compress(config, request_dict, operation_model) + _assert_compression(is_compressed, request_dict['body'], encoding) + assert ( + 'headers' in request_dict + and 'Content-Encoding' in request_dict['headers'] + and encoding in request_dict['headers']['Content-Encoding'] + ) == is_compressed + + +@pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) +def test_compress_bad_types(body): + request_dict = {'body': body} + RequestCompressor.compress( + COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION + ) + assert request_dict['body'] == body + + +@pytest.mark.parametrize( + 'body', + [io.StringIO('foo'), io.BytesIO(b'foo')], +) +def test_body_streams_position_reset(body): + RequestCompressor.compress( + COMPRESSION_CONFIG_0_BYTES, {'body': body}, OP_WITH_COMPRESSION + ) + assert body.tell() == 0 diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py index 361389e921..2701c32efe 100644 --- a/tests/unit/test_handlers.py +++ b/tests/unit/test_handlers.py @@ -1089,25 +1089,6 @@ def test_set_operation_specific_signer_s3v4(auth_type, expected_response): assert response == expected_response -@pytest.mark.parametrize( - 'protocol, signature_version, params, expected_body', - [ - ('query', 'v4', {'body': {'foo': 'bar'}}, b'foo=bar'), - ('query', 'v4', {}, None), - ('json', 'v4', {'body': {'foo': 'bar'}}, {'foo': 'bar'}), - ('query', 'v2', {'body': {'foo': 'bar'}}, {'foo': 'bar'}), - ('query', 'v4', {'body': 'foo=bar'}, 'foo=bar'), - ], -) -def test_urlencode_body(protocol, signature_version, params, expected_body): - operation_def = {'name': 'CreateFoo'} - service_def = {'metadata': {'protocol': protocol}, 'shapes': {}} - model = OperationModel(operation_def, ServiceModel(service_def)) - context = {'client_config': Config(signature_version=signature_version)} - handlers.urlencode_body(model, params, context) - assert params.get('body') == expected_body - - class TestConvertStringBodyToFileLikeObject(BaseSessionTest): def assert_converts_to_file_like_object_with_bytes(self, body, body_bytes): params = {'Body': body} diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6b1a33ba78..427abd7d0d 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -94,6 +94,7 @@ set_value_from_jmespath, switch_host_s3_accelerate, switch_to_virtual_host_style, + urlencode_query_body, validate_jmespath_for_set, ) from tests import FreezeTime, RawResponse, create_session, mock, unittest @@ -3476,3 +3477,25 @@ def cached_fn(self, a, b): assert cls2.cached_fn.cache_info().currsize == 2 assert cls2.cached_fn.cache_info().hits == 1 # the call was a cache hit assert cls2.cached_fn.cache_info().misses == 2 + + +@pytest.mark.parametrize( + 'request_dict, protocol, signature_version, expected_body', + [ + ({'body': {'foo': 'bar'}}, 'query', 'v4', b'foo=bar'), + ({'body': b''}, 'query', 'v4', b''), + ({'body': {'foo': 'bar'}}, 'json', 'v4', {'foo': 'bar'}), + ({'body': {'foo': 'bar'}}, 'query', 'v2', {'foo': 'bar'}), + ({'body': 'foo=bar'}, 'query', 'v4', 'foo=bar'), + ], +) +def test_urlencode_query_body( + request_dict, protocol, signature_version, expected_body +): + operation_def = {'name': 'CreateFoo'} + service_def = {'metadata': {'protocol': protocol}, 'shapes': {}} + model = OperationModel(operation_def, ServiceModel(service_def)) + urlencode_query_body( + request_dict, model, Config(signature_version=signature_version) + ) + assert request_dict['body'] == expected_body From 165c5e1f3e101f6934db8a94605b9d2710edeb03 Mon Sep 17 00:00:00 2001 From: davidlm Date: Fri, 7 Jul 2023 10:55:13 -0400 Subject: [PATCH 05/37] cleanup --- botocore/compress.py | 6 ++---- tests/functional/test_compress.py | 2 +- tests/unit/test_client.py | 19 ++++++++++++------- tests/unit/test_compress.py | 16 +++++++++------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 52eaa86b88..4900236f86 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -21,7 +21,7 @@ class RequestCompressor: - """A class that can compress the body of an ``AWSRequest``.""" + """A class that can compress a request body.""" @classmethod def compress(cls, config, request_dict, operation_model): @@ -34,7 +34,7 @@ def compress(cls, config, request_dict, operation_model): body = request_dict['body'] if cls._should_compress_request(config, body, operation_model): encodings = operation_model.request_compression['encodings'] - headers = request_dict.get('headers', {}) + headers = request_dict['headers'] for encoding in encodings: encoder = getattr(cls, f'_{encoding}_compress_body', None) if encoder is not None: @@ -47,8 +47,6 @@ def compress(cls, config, request_dict, operation_model): 'Compressing request with %s encoding', encoding ) request_dict['body'] = encoder(body) - if 'headers' not in request_dict: - request_dict['headers'] = headers else: logger.debug( 'Unsupported compression encoding: %s' % encoding diff --git a/tests/functional/test_compress.py b/tests/functional/test_compress.py index 3ad0465240..f7573599e8 100644 --- a/tests/functional/test_compress.py +++ b/tests/functional/test_compress.py @@ -1,4 +1,4 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"). You # may not use this file except in compliance with the License. A copy of diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index c06aa6c672..e5eff482ad 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1704,9 +1704,7 @@ def test_request_compression_client_config(self): self.assertEqual( service_client.meta.config.request_min_compression_size_bytes, 100 ) - self.assertEqual( - service_client.meta.config.disable_request_compression, True - ) + self.assertTrue(service_client.meta.config.disable_request_compression) def test_request_compression_config_store(self): self.config_store.set_config_variable( @@ -1724,9 +1722,7 @@ def test_request_compression_config_store(self): self.assertEqual( service_client.meta.config.request_min_compression_size_bytes, 100 ) - self.assertEqual( - service_client.meta.config.disable_request_compression, True - ) + self.assertTrue(service_client.meta.config.disable_request_compression) def test_request_compression_client_config_overrides_config_store(self): self.config_store.set_config_variable( @@ -1753,10 +1749,19 @@ def test_request_compression_client_config_overrides_config_store(self): ) def test_bad_request_min_compression_size_bytes(self): + creator = self.create_client_creator() + with self.assertRaises(InvalidConfigError): + creator.create_client( + 'myservice', + 'us-west-2', + credentials=self.credentials, + client_config=botocore.config.Config( + request_min_compression_size_bytes='foo', + ), + ) self.config_store.set_config_variable( 'request_min_compression_size_bytes', 'foo' ) - creator = self.create_client_creator() with self.assertRaises(InvalidConfigError): creator.create_client( 'myservice', diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 08a06c7069..0d7f294aff 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -113,7 +113,7 @@ def _assert_compression(is_compressed, body, encoding): disable_request_compression=True, request_min_compression_size_bytes=1000, ), - {'body': b'foo'}, + {'body': b'foo', 'headers': {}}, OP_WITH_COMPRESSION, False, None, @@ -172,28 +172,28 @@ def _assert_compression(is_compressed, body, encoding): ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': REQUEST_BODY.decode()}, + {'body': REQUEST_BODY.decode(), 'headers': {}}, OP_WITH_COMPRESSION, True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': bytearray(REQUEST_BODY)}, + {'body': bytearray(REQUEST_BODY), 'headers': {}}, OP_WITH_COMPRESSION, True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': io.BytesIO(REQUEST_BODY)}, + {'body': io.BytesIO(REQUEST_BODY), 'headers': {}}, OP_WITH_COMPRESSION, True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': io.StringIO(REQUEST_BODY.decode())}, + {'body': io.StringIO(REQUEST_BODY.decode()), 'headers': {}}, OP_WITH_COMPRESSION, True, 'gzip', @@ -232,7 +232,7 @@ def test_compress( @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) def test_compress_bad_types(body): - request_dict = {'body': body} + request_dict = {'body': body, 'headers': {}} RequestCompressor.compress( COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION ) @@ -245,6 +245,8 @@ def test_compress_bad_types(body): ) def test_body_streams_position_reset(body): RequestCompressor.compress( - COMPRESSION_CONFIG_0_BYTES, {'body': body}, OP_WITH_COMPRESSION + COMPRESSION_CONFIG_0_BYTES, + {'body': body, 'headers': {}}, + OP_WITH_COMPRESSION, ) assert body.tell() == 0 From b97fa8ce0e7ad1f05821aa0ecb5bfff72d1ae559 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 12 Jul 2023 10:27:23 -0400 Subject: [PATCH 06/37] pr feedback --- botocore/args.py | 22 ++++++++--- botocore/compress.py | 79 +++++++++++++++++++------------------ botocore/config.py | 4 +- botocore/configprovider.py | 2 - tests/unit/test_compress.py | 42 ++++++++++++++++---- 5 files changed, 95 insertions(+), 54 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 7164e8975b..86e2efe478 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -59,6 +59,10 @@ # Maximum allowed length of the ``user_agent_appid`` config field. Longer # values result in a warning-level log message. USERAGENT_APPID_MAXLEN = 50 +# Minimum and maximum allowed value of the `request_min_compression_size_bytes` +# config field. Lower or higher values result in an InvalidConfigError raised. +REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN = 0 +REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX = 10485760 class ClientArgsCreator: @@ -575,19 +579,27 @@ def _validate_min_compression_size(self, min_size): if min_size is not None: error_msg_base = ( f'Invalid value "{min_size}" for ' - 'request_min_compression_size_bytes. ' + 'request_min_compression_size_bytes.' ) try: min_size = int(min_size) except ValueError: raise botocore.exceptions.InvalidConfigError( - error_msg=f'{error_msg_base}Value must be an integer.' + error_msg=( + f'{error_msg_base} Value must be an integer.' + f' Received {type(min_size)} instead.' + ) ) - if not 0 <= min_size <= 1048576: + if ( + not REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN + <= min_size + <= REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX + ): raise botocore.exceptions.InvalidConfigError( error_msg=( - f'{error_msg_base}Value must ' - 'be between 0 and 1048576.' + f'{error_msg_base} Value must be between ' + f'{REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN} and ' + f'{REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX}.' ) ) diff --git a/botocore/compress.py b/botocore/compress.py index 4900236f86..8c6f3cf8f1 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -25,33 +25,47 @@ class RequestCompressor: @classmethod def compress(cls, config, request_dict, operation_model): - """Compresses the request body using the specified encodings. - - Check if the request should be compressed based on the contents of its - body and config settings. Set or append the `Content-Encoding` header - with the matched encoding if not present. - """ + """Attempt to compress the request body using the modeled encodings.""" body = request_dict['body'] if cls._should_compress_request(config, body, operation_model): - encodings = operation_model.request_compression['encodings'] - headers = request_dict['headers'] - for encoding in encodings: + for i, encoding in enumerate( + operation_model.request_compression['encodings'] + ): encoder = getattr(cls, f'_{encoding}_compress_body', None) if encoder is not None: - ce_header = headers.get('Content-Encoding') - if ce_header is None: - headers['Content-Encoding'] = encoding - elif encoding not in ce_header.split(','): - headers['Content-Encoding'] = f'{ce_header},{encoding}' logger.debug( - 'Compressing request with %s encoding', encoding + 'Compressing request with %s encoding.', encoding ) request_dict['body'] = encoder(body) + cls._set_compression_header( + request_dict['headers'], encoding + ) + return else: logger.debug( - 'Unsupported compression encoding: %s' % encoding + 'Unsupported compression encoding: %s', encoding ) + @classmethod + def _should_compress_request(cls, config, body, operation_model): + if ( + config.disable_request_compression is not True + and config.signature_version != 'v2' + and operation_model.request_compression is not None + ): + # Request is compressed no matter the content length if it has a streaming input. + # However, if the stream has the `requiresLength` trait it is NOT compressed. + if operation_model.has_streaming_input: + return ( + 'requiresLength' + not in operation_model.get_streaming_input().metadata + ) + return ( + config.request_min_compression_size_bytes + <= cls._get_body_size(body) + ) + return False + @classmethod def _gzip_compress_body(cls, body): if isinstance(body, str): @@ -80,33 +94,22 @@ def _gzip_compress_fileobj(body): compressed_obj.seek(0) return compressed_obj - @classmethod - def _should_compress_request(cls, config, body, operation_model): - if ( - config.disable_request_compression is not True - and config.signature_version != 'v2' - and operation_model.request_compression is not None - ): - # Request is compressed no matter the content length if it has a streaming input. - # However, if the stream has the `requiresLength` trait it is NOT compressed. - if operation_model.has_streaming_input: - return ( - 'requiresLength' - not in operation_model.get_streaming_input().metadata - ) - return ( - config.request_min_compression_size_bytes - <= cls._get_body_size(body) - ) - return False - @staticmethod def _get_body_size(body): size = determine_content_length(body) if size is None: logger.debug( - 'Unable to get length of the request body: %s. Not compressing.' - % body + 'Unable to get length of the request body: %s. ' + 'Skipping compression.', + body, ) return -1 return size + + @staticmethod + def _set_compression_header(headers, encoding): + ce_header = headers.get('Content-Encoding') + if ce_header is None: + headers['Content-Encoding'] = encoding + elif encoding not in ce_header.split(','): + headers['Content-Encoding'] = f'{ce_header},{encoding}' diff --git a/botocore/config.py b/botocore/config.py index de31321d85..1abea5cee7 100644 --- a/botocore/config.py +++ b/botocore/config.py @@ -217,8 +217,8 @@ class Config: Defaults to None. :type disable_request_compression: bool - :param disable_request_compression: Disables the request body compression - if set to True. + :param disable_request_compression: Disables request body compression if + set to True. Defaults to None. """ diff --git a/botocore/configprovider.py b/botocore/configprovider.py index 778507775d..1f9155e320 100644 --- a/botocore/configprovider.py +++ b/botocore/configprovider.py @@ -147,8 +147,6 @@ # whatever the defaults are in _retry.json. 'max_attempts': ('max_attempts', 'AWS_MAX_ATTEMPTS', None, int), 'user_agent_appid': ('sdk_ua_app_id', 'AWS_SDK_UA_APP_ID', None, None), - # This must be a parsable integer between 0 and 1048576, but validation - # is performed during client initialization instead of here. 'request_min_compression_size_bytes': ( 'request_min_compression_size_bytes', 'AWS_REQUEST_MIN_COMPRESSION_SIZE_BYTES', diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 0d7f294aff..7a65c0e9ab 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -10,6 +10,7 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +import gzip import io import pytest @@ -26,6 +27,13 @@ def _op_with_compression(): return op +def _op_with_multiple_compressions(): + op = mock.Mock() + op.request_compression = {'encodings': ['gzip', 'gzip']} + op.has_streaming_input = False + return op + + def _op_unknown_compression(): op = mock.Mock() op.request_compression = {'encodings': ['foo']} @@ -94,15 +102,25 @@ def request_dict_with_content_encoding_header(): } -COMPRESSION_HEADERS = {'gzip': b'\x1f\x8b'} +COMPRESSION_INFO = { + 'gzip': {'headers': b'\x1f\x8b', 'decompress_method': gzip.decompress} +} -def _assert_compression(is_compressed, body, encoding): +def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): if hasattr(body, 'read'): - header = body.read(2) - else: - header = body[:2] - assert is_compressed == (header == COMPRESSION_HEADERS.get(encoding)) + body = body.read() + maybe_compressed_body = maybe_compressed_body.read() + if isinstance(body, str): + body = body.encode('utf-8') + compression_info = COMPRESSION_INFO.get(encoding, {}) + assert is_compressed == ( + maybe_compressed_body[:2] == compression_info.get('headers') + ) + decompress_method = compression_info.get( + 'decompress_method', lambda body: body + ) + assert decompress_method(maybe_compressed_body) == body @pytest.mark.parametrize( @@ -142,6 +160,13 @@ def _assert_compression(is_compressed, body, encoding): True, 'gzip', ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict(), + _op_with_multiple_compressions(), + True, + 'gzip', + ), ( DEFAULT_COMPRESSION_CONFIG, request_dict(), @@ -221,8 +246,11 @@ def test_compress( is_compressed, encoding, ): + original_body = request_dict['body'] RequestCompressor.compress(config, request_dict, operation_model) - _assert_compression(is_compressed, request_dict['body'], encoding) + _assert_compression( + is_compressed, original_body, request_dict['body'], encoding + ) assert ( 'headers' in request_dict and 'Content-Encoding' in request_dict['headers'] From 9326d3fdb370f9aa355efdc5fc92452271402747 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 12 Jul 2023 10:54:02 -0400 Subject: [PATCH 07/37] removed enumerate and set size=-1 instead of returning -1 --- botocore/compress.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 8c6f3cf8f1..ce058c5aa4 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -28,9 +28,7 @@ def compress(cls, config, request_dict, operation_model): """Attempt to compress the request body using the modeled encodings.""" body = request_dict['body'] if cls._should_compress_request(config, body, operation_model): - for i, encoding in enumerate( - operation_model.request_compression['encodings'] - ): + for encoding in operation_model.request_compression['encodings']: encoder = getattr(cls, f'_{encoding}_compress_body', None) if encoder is not None: logger.debug( @@ -103,7 +101,7 @@ def _get_body_size(body): 'Skipping compression.', body, ) - return -1 + size = -1 return size @staticmethod From afee36d49fdc89621b6932b64e71cb63584c5612 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 12 Jul 2023 15:11:46 -0400 Subject: [PATCH 08/37] change to functional implementation --- botocore/client.py | 6 +- botocore/compress.py | 170 +++++++++++++++++------------------- tests/unit/test_compress.py | 8 +- tests/unit/test_utils.py | 8 +- 4 files changed, 93 insertions(+), 99 deletions(-) diff --git a/botocore/client.py b/botocore/client.py index b03c26df97..50f36c33e0 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -16,7 +16,7 @@ from botocore.args import ClientArgsCreator from botocore.auth import AUTH_TYPE_MAPS from botocore.awsrequest import prepare_request_dict -from botocore.compress import RequestCompressor +from botocore.compress import compress_request from botocore.config import Config from botocore.discovery import ( EndpointDiscoveryHandler, @@ -960,9 +960,7 @@ def _make_api_call(self, operation_name, api_params): urlencode_query_body( request_dict, operation_model, self.meta.config ) - RequestCompressor.compress( - self.meta.config, request_dict, operation_model - ) + compress_request(self.meta.config, request_dict, operation_model) apply_request_checksum(request_dict) http, parsed_response = self._make_request( operation_model, request_dict, request_context diff --git a/botocore/compress.py b/botocore/compress.py index ce058c5aa4..48909ee51f 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -20,94 +20,86 @@ logger = logging.getLogger(__name__) -class RequestCompressor: - """A class that can compress a request body.""" - - @classmethod - def compress(cls, config, request_dict, operation_model): - """Attempt to compress the request body using the modeled encodings.""" - body = request_dict['body'] - if cls._should_compress_request(config, body, operation_model): - for encoding in operation_model.request_compression['encodings']: - encoder = getattr(cls, f'_{encoding}_compress_body', None) - if encoder is not None: - logger.debug( - 'Compressing request with %s encoding.', encoding - ) - request_dict['body'] = encoder(body) - cls._set_compression_header( - request_dict['headers'], encoding - ) - return - else: - logger.debug( - 'Unsupported compression encoding: %s', encoding - ) - - @classmethod - def _should_compress_request(cls, config, body, operation_model): - if ( - config.disable_request_compression is not True - and config.signature_version != 'v2' - and operation_model.request_compression is not None - ): - # Request is compressed no matter the content length if it has a streaming input. - # However, if the stream has the `requiresLength` trait it is NOT compressed. - if operation_model.has_streaming_input: - return ( - 'requiresLength' - not in operation_model.get_streaming_input().metadata - ) +def compress_request(config, request_dict, operation_model): + """Attempt to compress the request body using the modeled encodings.""" + body = request_dict['body'] + if _should_compress_request(config, body, operation_model): + for encoding in operation_model.request_compression['encodings']: + encoder = COMPRESSION_MAPPING.get(encoding) + if encoder is not None: + logger.debug('Compressing request with %s encoding.', encoding) + request_dict['body'] = encoder(body) + _set_compression_header(request_dict['headers'], encoding) + return + else: + logger.debug('Unsupported compression encoding: %s', encoding) + + +def _should_compress_request(config, body, operation_model): + if ( + config.disable_request_compression is not True + and config.signature_version != 'v2' + and operation_model.request_compression is not None + ): + # Request is compressed no matter the content length if it has a streaming input. + # However, if the stream has the `requiresLength` trait it is NOT compressed. + if operation_model.has_streaming_input: return ( - config.request_min_compression_size_bytes - <= cls._get_body_size(body) + 'requiresLength' + not in operation_model.get_streaming_input().metadata ) - return False - - @classmethod - def _gzip_compress_body(cls, body): - if isinstance(body, str): - return gzip.compress(body.encode('utf-8')) - elif isinstance(body, (bytes, bytearray)): - return gzip.compress(body) - elif hasattr(body, 'read'): - if hasattr(body, 'seek') and hasattr(body, 'tell'): - current_position = body.tell() - compressed_obj = cls._gzip_compress_fileobj(body) - body.seek(current_position) - return compressed_obj - return cls._gzip_compress_fileobj(body) - - @staticmethod - def _gzip_compress_fileobj(body): - compressed_obj = io.BytesIO() - with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: - while True: - chunk = body.read(8192) - if not chunk: - break - if isinstance(chunk, str): - chunk = chunk.encode('utf-8') - gz.write(chunk) - compressed_obj.seek(0) - return compressed_obj - - @staticmethod - def _get_body_size(body): - size = determine_content_length(body) - if size is None: - logger.debug( - 'Unable to get length of the request body: %s. ' - 'Skipping compression.', - body, - ) - size = -1 - return size - - @staticmethod - def _set_compression_header(headers, encoding): - ce_header = headers.get('Content-Encoding') - if ce_header is None: - headers['Content-Encoding'] = encoding - elif encoding not in ce_header.split(','): - headers['Content-Encoding'] = f'{ce_header},{encoding}' + return config.request_min_compression_size_bytes <= _get_body_size( + body + ) + return False + + +def _gzip_compress_body(body): + if isinstance(body, str): + return gzip.compress(body.encode('utf-8')) + elif isinstance(body, (bytes, bytearray)): + return gzip.compress(body) + elif hasattr(body, 'read'): + if hasattr(body, 'seek') and hasattr(body, 'tell'): + current_position = body.tell() + compressed_obj = _gzip_compress_fileobj(body) + body.seek(current_position) + return compressed_obj + return _gzip_compress_fileobj(body) + + +def _gzip_compress_fileobj(body): + compressed_obj = io.BytesIO() + with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: + while True: + chunk = body.read(8192) + if not chunk: + break + if isinstance(chunk, str): + chunk = chunk.encode('utf-8') + gz.write(chunk) + compressed_obj.seek(0) + return compressed_obj + + +def _get_body_size(body): + size = determine_content_length(body) + if size is None: + logger.debug( + 'Unable to get length of the request body: %s. ' + 'Skipping compression.', + body, + ) + size = -1 + return size + + +def _set_compression_header(headers, encoding): + ce_header = headers.get('Content-Encoding') + if ce_header is None: + headers['Content-Encoding'] = encoding + elif encoding not in ce_header.split(','): + headers['Content-Encoding'] = f'{ce_header},{encoding}' + + +COMPRESSION_MAPPING = {'gzip': _gzip_compress_body} diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 7a65c0e9ab..6984a79592 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -15,7 +15,7 @@ import pytest -from botocore.compress import RequestCompressor +from botocore.compress import compress_request from botocore.config import Config from tests import mock @@ -247,7 +247,7 @@ def test_compress( encoding, ): original_body = request_dict['body'] - RequestCompressor.compress(config, request_dict, operation_model) + compress_request(config, request_dict, operation_model) _assert_compression( is_compressed, original_body, request_dict['body'], encoding ) @@ -261,7 +261,7 @@ def test_compress( @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) def test_compress_bad_types(body): request_dict = {'body': body, 'headers': {}} - RequestCompressor.compress( + compress_request( COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION ) assert request_dict['body'] == body @@ -272,7 +272,7 @@ def test_compress_bad_types(body): [io.StringIO('foo'), io.BytesIO(b'foo')], ) def test_body_streams_position_reset(body): - RequestCompressor.compress( + compress_request( COMPRESSION_CONFIG_0_BYTES, {'body': body, 'headers': {}}, OP_WITH_COMPRESSION, diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 427abd7d0d..14072af212 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -3479,6 +3479,11 @@ def cached_fn(self, a, b): assert cls2.cached_fn.cache_info().misses == 2 +@pytest.fixture +def operation_def(): + return {'name': 'CreateFoo'} + + @pytest.mark.parametrize( 'request_dict, protocol, signature_version, expected_body', [ @@ -3490,9 +3495,8 @@ def cached_fn(self, a, b): ], ) def test_urlencode_query_body( - request_dict, protocol, signature_version, expected_body + operation_def, request_dict, protocol, signature_version, expected_body ): - operation_def = {'name': 'CreateFoo'} service_def = {'metadata': {'protocol': protocol}, 'shapes': {}} model = OperationModel(operation_def, ServiceModel(service_def)) urlencode_query_body( From 68630f6a00e9cb3de096f0b958561506aad12e46 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 12 Jul 2023 15:43:25 -0400 Subject: [PATCH 09/37] simplify assert compression method --- tests/unit/test_compress.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 6984a79592..8a867c6261 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -102,9 +102,7 @@ def request_dict_with_content_encoding_header(): } -COMPRESSION_INFO = { - 'gzip': {'headers': b'\x1f\x8b', 'decompress_method': gzip.decompress} -} +DECOMPRESSION_METHOD_MAP = {'gzip': gzip.decompress} def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): @@ -113,12 +111,8 @@ def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): maybe_compressed_body = maybe_compressed_body.read() if isinstance(body, str): body = body.encode('utf-8') - compression_info = COMPRESSION_INFO.get(encoding, {}) - assert is_compressed == ( - maybe_compressed_body[:2] == compression_info.get('headers') - ) - decompress_method = compression_info.get( - 'decompress_method', lambda body: body + decompress_method = DECOMPRESSION_METHOD_MAP.get( + encoding, lambda body: body ) assert decompress_method(maybe_compressed_body) == body From 10cec6bbdfea5faaa16dd7803a372c70599f338f Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 13 Jul 2023 10:25:37 -0400 Subject: [PATCH 10/37] pr feedback --- botocore/client.py | 6 ++++-- botocore/compress.py | 2 +- tests/unit/test_compress.py | 10 ++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/botocore/client.py b/botocore/client.py index 50f36c33e0..a7f3eacfbe 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -16,7 +16,7 @@ from botocore.args import ClientArgsCreator from botocore.auth import AUTH_TYPE_MAPS from botocore.awsrequest import prepare_request_dict -from botocore.compress import compress_request +from botocore.compress import maybe_compress_request from botocore.config import Config from botocore.discovery import ( EndpointDiscoveryHandler, @@ -960,7 +960,9 @@ def _make_api_call(self, operation_name, api_params): urlencode_query_body( request_dict, operation_model, self.meta.config ) - compress_request(self.meta.config, request_dict, operation_model) + maybe_compress_request( + self.meta.config, request_dict, operation_model + ) apply_request_checksum(request_dict) http, parsed_response = self._make_request( operation_model, request_dict, request_context diff --git a/botocore/compress.py b/botocore/compress.py index 48909ee51f..00d576b70b 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) -def compress_request(config, request_dict, operation_model): +def maybe_compress_request(config, request_dict, operation_model): """Attempt to compress the request body using the modeled encodings.""" body = request_dict['body'] if _should_compress_request(config, body, operation_model): diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 8a867c6261..deeec60844 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -15,7 +15,7 @@ import pytest -from botocore.compress import compress_request +from botocore.compress import maybe_compress_request from botocore.config import Config from tests import mock @@ -29,6 +29,8 @@ def _op_with_compression(): def _op_with_multiple_compressions(): op = mock.Mock() + # TODO: Update second value to a different encoding. Currently only gzip + # is supported. op.request_compression = {'encodings': ['gzip', 'gzip']} op.has_streaming_input = False return op @@ -241,7 +243,7 @@ def test_compress( encoding, ): original_body = request_dict['body'] - compress_request(config, request_dict, operation_model) + maybe_compress_request(config, request_dict, operation_model) _assert_compression( is_compressed, original_body, request_dict['body'], encoding ) @@ -255,7 +257,7 @@ def test_compress( @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) def test_compress_bad_types(body): request_dict = {'body': body, 'headers': {}} - compress_request( + maybe_compress_request( COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION ) assert request_dict['body'] == body @@ -266,7 +268,7 @@ def test_compress_bad_types(body): [io.StringIO('foo'), io.BytesIO(b'foo')], ) def test_body_streams_position_reset(body): - compress_request( + maybe_compress_request( COMPRESSION_CONFIG_0_BYTES, {'body': body, 'headers': {}}, OP_WITH_COMPRESSION, From 6247e2853d219bb8370adf8a13200c46c3cca88d Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 13 Jul 2023 12:52:33 -0400 Subject: [PATCH 11/37] update wording regarding compression for requests with streaming input --- botocore/compress.py | 5 +++-- botocore/config.py | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 00d576b70b..b181b7c9ac 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -41,8 +41,9 @@ def _should_compress_request(config, body, operation_model): and config.signature_version != 'v2' and operation_model.request_compression is not None ): - # Request is compressed no matter the content length if it has a streaming input. - # However, if the stream has the `requiresLength` trait it is NOT compressed. + # Requests with streaming input are compressed regardless of + # `request_min_compression_size_bytes` if they don't contain the + # `requiresLength` trait. if operation_model.has_streaming_input: return ( 'requiresLength' diff --git a/botocore/config.py b/botocore/config.py index 1abea5cee7..a968eacc99 100644 --- a/botocore/config.py +++ b/botocore/config.py @@ -209,10 +209,9 @@ class Config: :type request_min_compression_size_bytes: int :param request_min_compression_bytes: The minimum size in bytes that a - request body should be to trigger compression. A request is compressed - regardless of the size of the body if the operation has streaming input, - but if the operation also contains the `requiresLength` trait, the request - is not compressed. + request body should be to trigger compression. All requests with streaming + input that don't contain the `requiresLength` trait will be compressed + regardless of this setting. Defaults to None. From aa549275ffdea0974f0944691cf122c5abda4d99 Mon Sep 17 00:00:00 2001 From: davidlm Date: Sat, 15 Jul 2023 21:47:27 -0400 Subject: [PATCH 12/37] divert byte encoding --- botocore/utils.py | 4 +--- tests/functional/test_docdb.py | 4 ++-- tests/functional/test_neptune.py | 4 ++-- tests/functional/test_rds.py | 4 ++-- tests/unit/test_utils.py | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/botocore/utils.py b/botocore/utils.py index 9d35c61211..1cad2d068f 100644 --- a/botocore/utils.py +++ b/botocore/utils.py @@ -3440,6 +3440,4 @@ def urlencode_query_body(request_dict, operation_model, config): and isinstance(body, dict) and config.signature_version != 'v2' ): - request_dict['body'] = urlencode( - body, doseq=True, encoding='utf-8' - ).encode('utf-8') + request_dict['body'] = urlencode(body, doseq=True, encoding='utf-8') diff --git a/tests/functional/test_docdb.py b/tests/functional/test_docdb.py index e64bc7f42a..11c02574bf 100644 --- a/tests/functional/test_docdb.py +++ b/tests/functional/test_docdb.py @@ -20,8 +20,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn(b'PreSignedUrl', body) - self.assertNotIn(b'SourceRegion', body) + self.assertIn('PreSignedUrl', body) + self.assertNotIn('SourceRegion', body) def test_create_db_cluster(self): params = { diff --git a/tests/functional/test_neptune.py b/tests/functional/test_neptune.py index 37000d9f13..2ada9421c9 100644 --- a/tests/functional/test_neptune.py +++ b/tests/functional/test_neptune.py @@ -20,8 +20,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn(b'PreSignedUrl', body) - self.assertNotIn(b'SourceRegion', body) + self.assertIn('PreSignedUrl', body) + self.assertNotIn('SourceRegion', body) def test_create_db_cluster(self): params = { diff --git a/tests/functional/test_rds.py b/tests/functional/test_rds.py index d680db2709..cd4cedc219 100644 --- a/tests/functional/test_rds.py +++ b/tests/functional/test_rds.py @@ -21,8 +21,8 @@ def setUp(self): self.http_stubber = ClientHTTPStubber(self.client) def assert_presigned_url_injected_in_request(self, body): - self.assertIn(b'PreSignedUrl', body) - self.assertNotIn(b'SourceRegion', body) + self.assertIn('PreSignedUrl', body) + self.assertNotIn('SourceRegion', body) def test_copy_snapshot(self): params = { diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 14072af212..d8feea9631 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -3487,7 +3487,7 @@ def operation_def(): @pytest.mark.parametrize( 'request_dict, protocol, signature_version, expected_body', [ - ({'body': {'foo': 'bar'}}, 'query', 'v4', b'foo=bar'), + ({'body': {'foo': 'bar'}}, 'query', 'v4', 'foo=bar'), ({'body': b''}, 'query', 'v4', b''), ({'body': {'foo': 'bar'}}, 'json', 'v4', {'foo': 'bar'}), ({'body': {'foo': 'bar'}}, 'query', 'v2', {'foo': 'bar'}), From debd186a8c5fe8978a38d567b401e584ff6f9fa6 Mon Sep 17 00:00:00 2001 From: davidlm Date: Sat, 15 Jul 2023 21:50:05 -0400 Subject: [PATCH 13/37] move min and max min compression size to funciton --- botocore/args.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 86e2efe478..7669b15db4 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -59,10 +59,6 @@ # Maximum allowed length of the ``user_agent_appid`` config field. Longer # values result in a warning-level log message. USERAGENT_APPID_MAXLEN = 50 -# Minimum and maximum allowed value of the `request_min_compression_size_bytes` -# config field. Lower or higher values result in an InvalidConfigError raised. -REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN = 0 -REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX = 10485760 class ClientArgsCreator: @@ -576,6 +572,8 @@ def _compute_request_compression_config(self, config_kwargs): config_kwargs['disable_request_compression'] = disabled def _validate_min_compression_size(self, min_size): + min_allowed_min_size = 0 + max_allowed_min_size = 10485760 if min_size is not None: error_msg_base = ( f'Invalid value "{min_size}" for ' @@ -590,16 +588,11 @@ def _validate_min_compression_size(self, min_size): f' Received {type(min_size)} instead.' ) ) - if ( - not REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN - <= min_size - <= REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX - ): + if not min_allowed_min_size <= min_size <= max_allowed_min_size: raise botocore.exceptions.InvalidConfigError( error_msg=( f'{error_msg_base} Value must be between ' - f'{REQUEST_MIN_COMPRESSION_SIZE_BYTES_MIN} and ' - f'{REQUEST_MIN_COMPRESSION_SIZE_BYTES_MAX}.' + f'{min_allowed_min_size} and {max_allowed_min_size}.' ) ) From 6e8c0a3a8de54eed0cc0f20d60d304ae6f606719 Mon Sep 17 00:00:00 2001 From: davidlm Date: Sat, 15 Jul 2023 22:08:55 -0400 Subject: [PATCH 14/37] fixed test --- tests/unit/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index e5eff482ad..b8beb35330 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1468,7 +1468,7 @@ def inject_params(params, **kwargs): # Ensure the handler passed on the correct param values. body = self.endpoint.make_request.call_args[0][1]['body'] - self.assertIn(b'Foo=zero', body) + self.assertIn('Foo=zero', body) def test_client_default_for_s3_addressing_style(self): creator = self.create_client_creator() From 2f6d52ec1c696ac92c78f46a191e6eac393d4e74 Mon Sep 17 00:00:00 2001 From: davidlm Date: Mon, 17 Jul 2023 09:35:50 -0400 Subject: [PATCH 15/37] fixed max allowed min compression size, added test for it and moved compression config tests to test_args --- botocore/args.py | 2 +- tests/unit/test_args.py | 75 ++++++++++++++++++++++++++++++++++++ tests/unit/test_client.py | 80 --------------------------------------- 3 files changed, 76 insertions(+), 81 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 7669b15db4..ba623b05ba 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -573,7 +573,7 @@ def _compute_request_compression_config(self, config_kwargs): def _validate_min_compression_size(self, min_size): min_allowed_min_size = 0 - max_allowed_min_size = 10485760 + max_allowed_min_size = 1048576 if min_size is not None: error_msg_base = ( f'Invalid value "{min_size}" for ' diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index e82226b000..451dad00f8 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -529,6 +529,81 @@ def test_doesnt_create_ruleset_resolver_if_not_given_data(self): ) m.assert_not_called() + def test_request_compression_client_config(self): + config = self.call_get_client_args( + client_config=Config( + disable_request_compression=True, + request_min_compression_size_bytes=100, + ) + )['client_config'] + self.assertEqual(config.request_min_compression_size_bytes, 100) + self.assertTrue(config.disable_request_compression) + + def test_request_compression_config_store(self): + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 100 + ) + self.config_store.set_config_variable( + 'disable_request_compression', True + ) + config = self.call_get_client_args()['client_config'] + self.assertEqual(config.request_min_compression_size_bytes, 100) + self.assertTrue(config.disable_request_compression) + + def test_request_compression_client_config_overrides_config_store(self): + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 100 + ) + self.config_store.set_config_variable( + 'disable_request_compression', True + ) + config = self.call_get_client_args( + client_config=Config( + disable_request_compression=False, + request_min_compression_size_bytes=0, + ) + )['client_config'] + self.assertEqual(config.request_min_compression_size_bytes, 0) + self.assertFalse(config.disable_request_compression) + + def test_bad_type_request_min_compression_size_bytes(self): + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args( + client_config=Config( + request_min_compression_size_bytes='foo', + ), + ) + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 'foo' + ) + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args() + + def test_bad_value_request_min_compression_size_bytes(self): + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args( + client_config=Config( + request_min_compression_size_bytes=-1, + ), + ) + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 99999999 + ) + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args( + client_config=Config( + request_min_compression_size_bytes=-1, + ), + ) + + def test_bad_value_disable_request_compression(self): + config = self.call_get_client_args( + client_config=Config( + disable_request_compression='foo', + ), + )['client_config'] + self.assertFalse(config.disable_request_compression) + class TestEndpointResolverBuiltins(unittest.TestCase): def setUp(self): diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index b8beb35330..0cd8112d5e 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -27,7 +27,6 @@ from botocore.endpoint import DEFAULT_TIMEOUT from botocore.errorfactory import ClientExceptionsFactory from botocore.exceptions import ( - InvalidConfigError, InvalidMaxRetryAttemptsError, InvalidRetryConfigurationError, InvalidRetryModeError, @@ -1690,85 +1689,6 @@ def test_client_close_context_manager(self): self.endpoint.close.assert_called_once_with() - def test_request_compression_client_config(self): - creator = self.create_client_creator() - service_client = creator.create_client( - 'myservice', - 'us-west-2', - credentials=self.credentials, - client_config=botocore.config.Config( - disable_request_compression=True, - request_min_compression_size_bytes=100, - ), - ) - self.assertEqual( - service_client.meta.config.request_min_compression_size_bytes, 100 - ) - self.assertTrue(service_client.meta.config.disable_request_compression) - - def test_request_compression_config_store(self): - self.config_store.set_config_variable( - 'request_min_compression_size_bytes', 100 - ) - self.config_store.set_config_variable( - 'disable_request_compression', True - ) - creator = self.create_client_creator() - service_client = creator.create_client( - 'myservice', - 'us-west-2', - credentials=self.credentials, - ) - self.assertEqual( - service_client.meta.config.request_min_compression_size_bytes, 100 - ) - self.assertTrue(service_client.meta.config.disable_request_compression) - - def test_request_compression_client_config_overrides_config_store(self): - self.config_store.set_config_variable( - 'request_min_compression_size_bytes', 100 - ) - self.config_store.set_config_variable( - 'disable_request_compression', True - ) - creator = self.create_client_creator() - service_client = creator.create_client( - 'myservice', - 'us-west-2', - credentials=self.credentials, - client_config=botocore.config.Config( - disable_request_compression=False, - request_min_compression_size_bytes=0, - ), - ) - self.assertEqual( - service_client.meta.config.request_min_compression_size_bytes, 0 - ) - self.assertFalse( - service_client.meta.config.disable_request_compression - ) - - def test_bad_request_min_compression_size_bytes(self): - creator = self.create_client_creator() - with self.assertRaises(InvalidConfigError): - creator.create_client( - 'myservice', - 'us-west-2', - credentials=self.credentials, - client_config=botocore.config.Config( - request_min_compression_size_bytes='foo', - ), - ) - self.config_store.set_config_variable( - 'request_min_compression_size_bytes', 'foo' - ) - with self.assertRaises(InvalidConfigError): - creator.create_client( - 'myservice', - 'us-west-2', - credentials=self.credentials, - ) - class TestClientErrors(TestAutoGeneratedClient): def add_error_response(self, error_response): From 56fd999f85b26dda5ee3a7e7f8d0e94ce179a009 Mon Sep 17 00:00:00 2001 From: davidlm Date: Mon, 17 Jul 2023 09:41:26 -0400 Subject: [PATCH 16/37] cleanup tests and test bad min and bad max separately --- tests/unit/test_args.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index 451dad00f8..dd9fd9f421 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -579,7 +579,7 @@ def test_bad_type_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): self.call_get_client_args() - def test_bad_value_request_min_compression_size_bytes(self): + def test_low_min_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): self.call_get_client_args( client_config=Config( @@ -587,14 +587,23 @@ def test_bad_value_request_min_compression_size_bytes(self): ), ) self.config_store.set_config_variable( - 'request_min_compression_size_bytes', 99999999 + 'request_min_compression_size_bytes', -1 ) + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args() + + def test_high_max_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): self.call_get_client_args( client_config=Config( - request_min_compression_size_bytes=-1, + request_min_compression_size_bytes=9999999, ), ) + self.config_store.set_config_variable( + 'request_min_compression_size_bytes', 9999999 + ) + with self.assertRaises(exceptions.InvalidConfigError): + self.call_get_client_args() def test_bad_value_disable_request_compression(self): config = self.call_get_client_args( From f5202b4a9ff9f879630a6690aec5b7de2811f03d Mon Sep 17 00:00:00 2001 From: davidlm Date: Mon, 17 Jul 2023 09:55:50 -0400 Subject: [PATCH 17/37] cleanup --- botocore/args.py | 2 +- botocore/compress.py | 29 ++++++++++++++--------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index ba623b05ba..11543de860 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -567,7 +567,7 @@ def _compute_request_compression_config(self, config_kwargs): 'disable_request_compression' ) else: - # if the user provided a value we must check if its a boolean + # if the user provided a value we must check if it's a boolean disabled = ensure_boolean(disabled) config_kwargs['disable_request_compression'] = disabled diff --git a/botocore/compress.py b/botocore/compress.py index b181b7c9ac..9bea26ec25 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -49,12 +49,23 @@ def _should_compress_request(config, body, operation_model): 'requiresLength' not in operation_model.get_streaming_input().metadata ) - return config.request_min_compression_size_bytes <= _get_body_size( - body - ) + body_size = _get_body_size(body) + return config.request_min_compression_size_bytes <= body_size return False +def _get_body_size(body): + size = determine_content_length(body) + if size is None: + logger.debug( + 'Unable to get length of the request body: %s. ' + 'Skipping compression.', + body, + ) + size = -1 + return size + + def _gzip_compress_body(body): if isinstance(body, str): return gzip.compress(body.encode('utf-8')) @@ -83,18 +94,6 @@ def _gzip_compress_fileobj(body): return compressed_obj -def _get_body_size(body): - size = determine_content_length(body) - if size is None: - logger.debug( - 'Unable to get length of the request body: %s. ' - 'Skipping compression.', - body, - ) - size = -1 - return size - - def _set_compression_header(headers, encoding): ce_header = headers.get('Content-Encoding') if ce_header is None: From 6cecbad6e5325108070aef599c3065f050fcc567 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 18 Jul 2023 17:50:21 -0400 Subject: [PATCH 18/37] pr feedback. clean up tests and move urlencoding dicts into compression --- botocore/args.py | 2 +- botocore/client.py | 4 - botocore/compress.py | 32 ++++++-- botocore/utils.py | 12 --- tests/functional/test_compress.py | 30 +++----- tests/unit/test_args.py | 4 +- tests/unit/test_compress.py | 120 ++++++++++++++++-------------- tests/unit/test_utils.py | 22 ------ 8 files changed, 104 insertions(+), 122 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 11543de860..2f7f7139c1 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -572,7 +572,7 @@ def _compute_request_compression_config(self, config_kwargs): config_kwargs['disable_request_compression'] = disabled def _validate_min_compression_size(self, min_size): - min_allowed_min_size = 0 + min_allowed_min_size = 1 max_allowed_min_size = 1048576 if min_size is not None: error_msg_base = ( diff --git a/botocore/client.py b/botocore/client.py index a7f3eacfbe..9f202029be 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -48,7 +48,6 @@ S3RegionRedirectorv2, ensure_boolean, get_service_module_name, - urlencode_query_body, ) # Keep these imported. There's pre-existing code that uses: @@ -957,9 +956,6 @@ def _make_api_call(self, operation_name, api_params): if event_response is not None: http, parsed_response = event_response else: - urlencode_query_body( - request_dict, operation_model, self.meta.config - ) maybe_compress_request( self.meta.config, request_dict, operation_model ) diff --git a/botocore/compress.py b/botocore/compress.py index 9bea26ec25..52ccdbd2e6 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -15,6 +15,7 @@ import io import logging +from botocore.compat import urlencode from botocore.utils import determine_content_length logger = logging.getLogger(__name__) @@ -22,20 +23,19 @@ def maybe_compress_request(config, request_dict, operation_model): """Attempt to compress the request body using the modeled encodings.""" - body = request_dict['body'] - if _should_compress_request(config, body, operation_model): + if _should_compress_request(config, request_dict, operation_model): for encoding in operation_model.request_compression['encodings']: encoder = COMPRESSION_MAPPING.get(encoding) if encoder is not None: logger.debug('Compressing request with %s encoding.', encoding) - request_dict['body'] = encoder(body) + request_dict['body'] = encoder(request_dict['body']) _set_compression_header(request_dict['headers'], encoding) return else: logger.debug('Unsupported compression encoding: %s', encoding) -def _should_compress_request(config, body, operation_model): +def _should_compress_request(config, request_dict, operation_model): if ( config.disable_request_compression is not True and config.signature_version != 'v2' @@ -49,8 +49,20 @@ def _should_compress_request(config, body, operation_model): 'requiresLength' not in operation_model.get_streaming_input().metadata ) - body_size = _get_body_size(body) - return config.request_min_compression_size_bytes <= body_size + else: + # If the request body is a dictionary, we need to encode it to get the + # length. If compression should occur, the encoded body will be saved + # to the request dictionary so it doesn't need to be encoded again. + body = request_dict['body'] + if isinstance(body, dict): + body = urlencode(body, doseq=True, encoding='utf-8') + should_compress = ( + config.request_min_compression_size_bytes + <= _get_body_size(body) + ) + if should_compress: + request_dict['body'] = body + return should_compress return False @@ -62,7 +74,7 @@ def _get_body_size(body): 'Skipping compression.', body, ) - size = -1 + size = 0 return size @@ -78,6 +90,10 @@ def _gzip_compress_body(body): body.seek(current_position) return compressed_obj return _gzip_compress_fileobj(body) + elif isinstance(body, dict): + return gzip.compress( + urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') + ) def _gzip_compress_fileobj(body): @@ -98,7 +114,7 @@ def _set_compression_header(headers, encoding): ce_header = headers.get('Content-Encoding') if ce_header is None: headers['Content-Encoding'] = encoding - elif encoding not in ce_header.split(','): + else: headers['Content-Encoding'] = f'{ce_header},{encoding}' diff --git a/botocore/utils.py b/botocore/utils.py index 1cad2d068f..266d204629 100644 --- a/botocore/utils.py +++ b/botocore/utils.py @@ -55,7 +55,6 @@ get_tzinfo_options, json, quote, - urlencode, urlparse, urlsplit, urlunsplit, @@ -3430,14 +3429,3 @@ def _serialize_if_needed(self, value, iso=False): 'stepfunctions': 'sfn', 'storagegateway': 'storage-gateway', } - - -def urlencode_query_body(request_dict, operation_model, config): - """URL encode a request's body if it using the query protocol.""" - body = request_dict['body'] - if ( - operation_model.service_model.protocol == 'query' - and isinstance(body, dict) - and config.signature_version != 'v2' - ): - request_dict['body'] = urlencode(body, doseq=True, encoding='utf-8') diff --git a/tests/functional/test_compress.py b/tests/functional/test_compress.py index f7573599e8..866469f2dc 100644 --- a/tests/functional/test_compress.py +++ b/tests/functional/test_compress.py @@ -113,24 +113,16 @@ def test_compression(patched_session, monkeypatch): config=Config(request_min_compression_size_bytes=100), ) with ClientHTTPStubber(client, strict=True) as http_stubber: - http_stubber.add_response( - status=200, - body=( - b"success" - b"Request processed successfully" - ), + http_stubber.add_response(status=200, body=b"") + params_list = [ + {"MockOpParam": f"MockOpParamValue{i}"} for i in range(1, 21) + ] + client.mock_operation(MockOpParamList=params_list) + param_template = ( + "MockOpParamList.member.{i}.MockOpParam=MockOpParamValue{i}" ) - client.mock_operation( - MockOpParamList=[ - {"MockOpParam": f"MockOpParamValue{i}"} for i in range(1, 21) - ] - ) - serialized_body = b"&".join( - [ - f"MockOpParamList.member.{i}.MockOpParam=MockOpParamValue{i}".encode() - for i in range(1, 21) - ] - ) - assert serialized_body in gzip.decompress( - http_stubber.requests[0].body + serialized_body = "&".join( + param_template.format(i=i) for i in range(1, 21) ) + actual_body = gzip.decompress(http_stubber.requests[0].body) + assert serialized_body.encode('utf-8') in actual_body diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index dd9fd9f421..dc636b29e7 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -583,11 +583,11 @@ def test_low_min_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): self.call_get_client_args( client_config=Config( - request_min_compression_size_bytes=-1, + request_min_compression_size_bytes=0, ), ) self.config_store.set_config_variable( - 'request_min_compression_size_bytes', -1 + 'request_min_compression_size_bytes', 0 ) with self.assertRaises(exceptions.InvalidConfigError): self.call_get_client_args() diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index deeec60844..5e36ade62d 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -15,61 +15,42 @@ import pytest -from botocore.compress import maybe_compress_request +from botocore.compat import urlencode +from botocore.compress import COMPRESSION_MAPPING, maybe_compress_request from botocore.config import Config from tests import mock -def _op_with_compression(): - op = mock.Mock() - op.request_compression = {'encodings': ['gzip']} - op.has_streaming_input = False - return op - - -def _op_with_multiple_compressions(): - op = mock.Mock() - # TODO: Update second value to a different encoding. Currently only gzip - # is supported. - op.request_compression = {'encodings': ['gzip', 'gzip']} - op.has_streaming_input = False - return op - - -def _op_unknown_compression(): - op = mock.Mock() - op.request_compression = {'encodings': ['foo']} - op.has_streaming_input = None - return op - - -def _op_without_compression(): +def _make_op( + request_compression=None, + has_streaming_input=False, + streaming_metadata=None, +): op = mock.Mock() - op.request_compression = None - op.has_streaming_input = False - return op - - -def _streaming_op_with_compression(): - op = _op_with_compression() - op.has_streaming_input = True - streaming_shape = mock.Mock() - streaming_shape.metadata = {} - op.get_streaming_input.return_value = streaming_shape + op.request_compression = request_compression + op.has_streaming_input = has_streaming_input + if streaming_metadata is not None: + streaming_shape = mock.Mock() + streaming_shape.metadata = streaming_metadata + op.get_streaming_input.return_value = streaming_shape return op -def _streaming_op_with_compression_requires_length(): - op = _streaming_op_with_compression() - streaming_shape = mock.Mock() - streaming_shape.metadata = {'requiresLength': True} - op.get_streaming_input.return_value = streaming_shape - return op - - -OP_WITH_COMPRESSION = _op_with_compression() -OP_UNKNOWN_COMPRESSION = _op_unknown_compression() -STREAMING_OP_WITH_COMPRESSION = _streaming_op_with_compression() +OP_WITH_COMPRESSION = _make_op(request_compression={'encodings': ['gzip']}) +OP_UNKNOWN_COMPRESSION = _make_op(request_compression={'encodings': ['foo']}) +OP_MULTIPLE_COMPRESSIONS = _make_op( + request_compression={'encodings': ['gzip', 'foo']} +) +STREAMING_OP_WITH_COMPRESSION = _make_op( + request_compression={'encodings': ['gzip']}, + has_streaming_input=True, + streaming_metadata={}, +) +STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH = _make_op( + request_compression={'encodings': ['gzip']}, + has_streaming_input=True, + streaming_metadata={'requiresLength': True}, +) REQUEST_BODY = ( b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' b'&MetricData.member.1.MetricName=metric&MetricData.member.1.Unit=Bytes' @@ -107,18 +88,28 @@ def request_dict_with_content_encoding_header(): DECOMPRESSION_METHOD_MAP = {'gzip': gzip.decompress} -def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): +def _assert_compression(body, maybe_compressed_body, encoding): if hasattr(body, 'read'): body = body.read() maybe_compressed_body = maybe_compressed_body.read() if isinstance(body, str): body = body.encode('utf-8') + if isinstance(body, dict) and encoding is not None: + body = urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') decompress_method = DECOMPRESSION_METHOD_MAP.get( encoding, lambda body: body ) assert decompress_method(maybe_compressed_body) == body +def _bad_compression(body): + raise ValueError('Reached unintended compression algorithm "foo"') + + +MOCK_COMPRESSION = {'foo': _bad_compression} +MOCK_COMPRESSION.update(COMPRESSION_MAPPING) + + @pytest.mark.parametrize( 'config, request_dict, operation_model, is_compressed, encoding', [ @@ -159,7 +150,7 @@ def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): ( COMPRESSION_CONFIG_128_BYTES, request_dict(), - _op_with_multiple_compressions(), + OP_MULTIPLE_COMPRESSIONS, True, 'gzip', ), @@ -173,14 +164,14 @@ def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): ( DEFAULT_COMPRESSION_CONFIG, request_dict(), - _streaming_op_with_compression_requires_length(), + STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH, False, None, ), ( DEFAULT_COMPRESSION_CONFIG, request_dict(), - _op_without_compression(), + _make_op(), False, None, ), @@ -233,6 +224,20 @@ def _assert_compression(is_compressed, body, maybe_compressed_body, encoding): True, 'gzip', ), + ( + COMPRESSION_CONFIG_0_BYTES, + {'body': {'foo': 'bar'}, 'headers': {}}, + OP_WITH_COMPRESSION, + True, + 'gzip', + ), + ( + COMPRESSION_CONFIG_128_BYTES, + {'body': {'foo': 'bar'}, 'headers': {}}, + OP_WITH_COMPRESSION, + False, + None, + ), ], ) def test_compress( @@ -244,9 +249,7 @@ def test_compress( ): original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) - _assert_compression( - is_compressed, original_body, request_dict['body'], encoding - ) + _assert_compression(original_body, request_dict['body'], encoding) assert ( 'headers' in request_dict and 'Content-Encoding' in request_dict['headers'] @@ -274,3 +277,12 @@ def test_body_streams_position_reset(body): OP_WITH_COMPRESSION, ) assert body.tell() == 0 + + +def test_only_compress_once(): + with mock.patch('botocore.compress.COMPRESSION_MAPPING', MOCK_COMPRESSION): + request_dict = {'body': REQUEST_BODY, 'headers': {}} + maybe_compress_request( + COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION + ) + _assert_compression(REQUEST_BODY, request_dict['body'], 'gzip') diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index d8feea9631..3efbeef532 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -94,7 +94,6 @@ set_value_from_jmespath, switch_host_s3_accelerate, switch_to_virtual_host_style, - urlencode_query_body, validate_jmespath_for_set, ) from tests import FreezeTime, RawResponse, create_session, mock, unittest @@ -3482,24 +3481,3 @@ def cached_fn(self, a, b): @pytest.fixture def operation_def(): return {'name': 'CreateFoo'} - - -@pytest.mark.parametrize( - 'request_dict, protocol, signature_version, expected_body', - [ - ({'body': {'foo': 'bar'}}, 'query', 'v4', 'foo=bar'), - ({'body': b''}, 'query', 'v4', b''), - ({'body': {'foo': 'bar'}}, 'json', 'v4', {'foo': 'bar'}), - ({'body': {'foo': 'bar'}}, 'query', 'v2', {'foo': 'bar'}), - ({'body': 'foo=bar'}, 'query', 'v4', 'foo=bar'), - ], -) -def test_urlencode_query_body( - operation_def, request_dict, protocol, signature_version, expected_body -): - service_def = {'metadata': {'protocol': protocol}, 'shapes': {}} - model = OperationModel(operation_def, ServiceModel(service_def)) - urlencode_query_body( - request_dict, model, Config(signature_version=signature_version) - ) - assert request_dict['body'] == expected_body From 053f0537587dd76cffc79c378bc058932ec2639f Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 18 Jul 2023 21:40:11 -0400 Subject: [PATCH 19/37] cleaned up tests --- tests/unit/test_args.py | 4 ++-- tests/unit/test_compress.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index dc636b29e7..4919938d2b 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -560,10 +560,10 @@ def test_request_compression_client_config_overrides_config_store(self): config = self.call_get_client_args( client_config=Config( disable_request_compression=False, - request_min_compression_size_bytes=0, + request_min_compression_size_bytes=1, ) )['client_config'] - self.assertEqual(config.request_min_compression_size_bytes, 0) + self.assertEqual(config.request_min_compression_size_bytes, 1) self.assertFalse(config.disable_request_compression) def test_bad_type_request_min_compression_size_bytes(self): diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 5e36ade62d..f5e03abced 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -65,9 +65,9 @@ def _make_op( disable_request_compression=False, request_min_compression_size_bytes=128, ) -COMPRESSION_CONFIG_0_BYTES = Config( +COMPRESSION_CONFIG_1_BYTE = Config( disable_request_compression=False, - request_min_compression_size_bytes=0, + request_min_compression_size_bytes=1, ) @@ -225,7 +225,7 @@ def _bad_compression(body): 'gzip', ), ( - COMPRESSION_CONFIG_0_BYTES, + COMPRESSION_CONFIG_1_BYTE, {'body': {'foo': 'bar'}, 'headers': {}}, OP_WITH_COMPRESSION, True, @@ -261,7 +261,7 @@ def test_compress( def test_compress_bad_types(body): request_dict = {'body': body, 'headers': {}} maybe_compress_request( - COMPRESSION_CONFIG_0_BYTES, request_dict, OP_WITH_COMPRESSION + COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION ) assert request_dict['body'] == body @@ -272,7 +272,7 @@ def test_compress_bad_types(body): ) def test_body_streams_position_reset(body): maybe_compress_request( - COMPRESSION_CONFIG_0_BYTES, + COMPRESSION_CONFIG_1_BYTE, {'body': body, 'headers': {}}, OP_WITH_COMPRESSION, ) From df00a70e7600e7338c44160f651426a0f7472d97 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 18 Jul 2023 22:02:40 -0400 Subject: [PATCH 20/37] added TypeError to min compression validation and a bunch of formatting cleanup --- botocore/args.py | 6 ++--- botocore/compress.py | 6 ++--- tests/unit/test_args.py | 53 +++++++++++++++---------------------- tests/unit/test_client.py | 2 +- tests/unit/test_compress.py | 4 +-- tests/unit/test_utils.py | 5 ---- 6 files changed, 29 insertions(+), 47 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 2f7f7139c1..6717ce220b 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -581,11 +581,11 @@ def _validate_min_compression_size(self, min_size): ) try: min_size = int(min_size) - except ValueError: + except (ValueError, TypeError): raise botocore.exceptions.InvalidConfigError( error_msg=( - f'{error_msg_base} Value must be an integer.' - f' Received {type(min_size)} instead.' + f'{error_msg_base} Value must be an integer. ' + f'Received {type(min_size)} instead.' ) ) if not min_allowed_min_size <= min_size <= max_allowed_min_size: diff --git a/botocore/compress.py b/botocore/compress.py index 52ccdbd2e6..53c95f6e54 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -56,10 +56,8 @@ def _should_compress_request(config, request_dict, operation_model): body = request_dict['body'] if isinstance(body, dict): body = urlencode(body, doseq=True, encoding='utf-8') - should_compress = ( - config.request_min_compression_size_bytes - <= _get_body_size(body) - ) + config_min = config.request_min_compression_size_bytes + should_compress = config_min <= _get_body_size(body) if should_compress: request_dict['body'] = body return should_compress diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index 4919938d2b..7efa764cea 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -530,12 +530,12 @@ def test_doesnt_create_ruleset_resolver_if_not_given_data(self): m.assert_not_called() def test_request_compression_client_config(self): - config = self.call_get_client_args( - client_config=Config( - disable_request_compression=True, - request_min_compression_size_bytes=100, - ) - )['client_config'] + input_config = Config( + disable_request_compression=True, + request_min_compression_size_bytes=100, + ) + client_args = self.call_get_client_args(client_config=input_config) + config = client_args['client_config'] self.assertEqual(config.request_min_compression_size_bytes, 100) self.assertTrue(config.disable_request_compression) @@ -557,22 +557,19 @@ def test_request_compression_client_config_overrides_config_store(self): self.config_store.set_config_variable( 'disable_request_compression', True ) - config = self.call_get_client_args( - client_config=Config( - disable_request_compression=False, - request_min_compression_size_bytes=1, - ) - )['client_config'] + input_config = Config( + disable_request_compression=False, + request_min_compression_size_bytes=1, + ) + client_args = self.call_get_client_args(client_config=input_config) + config = client_args['client_config'] self.assertEqual(config.request_min_compression_size_bytes, 1) self.assertFalse(config.disable_request_compression) def test_bad_type_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): - self.call_get_client_args( - client_config=Config( - request_min_compression_size_bytes='foo', - ), - ) + config = Config(request_min_compression_size_bytes='foo') + self.call_get_client_args(client_config=config) self.config_store.set_config_variable( 'request_min_compression_size_bytes', 'foo' ) @@ -581,11 +578,8 @@ def test_bad_type_request_min_compression_size_bytes(self): def test_low_min_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): - self.call_get_client_args( - client_config=Config( - request_min_compression_size_bytes=0, - ), - ) + config = Config(request_min_compression_size_bytes=0) + self.call_get_client_args(client_config=config) self.config_store.set_config_variable( 'request_min_compression_size_bytes', 0 ) @@ -594,11 +588,8 @@ def test_low_min_request_min_compression_size_bytes(self): def test_high_max_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): - self.call_get_client_args( - client_config=Config( - request_min_compression_size_bytes=9999999, - ), - ) + config = Config(request_min_compression_size_bytes=9999999) + self.call_get_client_args(client_config=config) self.config_store.set_config_variable( 'request_min_compression_size_bytes', 9999999 ) @@ -606,11 +597,9 @@ def test_high_max_request_min_compression_size_bytes(self): self.call_get_client_args() def test_bad_value_disable_request_compression(self): - config = self.call_get_client_args( - client_config=Config( - disable_request_compression='foo', - ), - )['client_config'] + input_config = Config(disable_request_compression='foo') + client_args = self.call_get_client_args(client_config=input_config) + config = client_args['client_config'] self.assertFalse(config.disable_request_compression) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 0cd8112d5e..c868615b8d 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -1467,7 +1467,7 @@ def inject_params(params, **kwargs): # Ensure the handler passed on the correct param values. body = self.endpoint.make_request.call_args[0][1]['body'] - self.assertIn('Foo=zero', body) + self.assertEqual(body['Foo'], 'zero') def test_client_default_for_s3_addressing_style(self): creator = self.create_client_creator() diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index f5e03abced..3901fce215 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -240,7 +240,7 @@ def _bad_compression(body): ), ], ) -def test_compress( +def test_maybe_compress( config, request_dict, operation_model, @@ -258,7 +258,7 @@ def test_compress( @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) -def test_compress_bad_types(body): +def test_maybe_compress_bad_types(body): request_dict = {'body': body, 'headers': {}} maybe_compress_request( COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 3efbeef532..6b1a33ba78 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -3476,8 +3476,3 @@ def cached_fn(self, a, b): assert cls2.cached_fn.cache_info().currsize == 2 assert cls2.cached_fn.cache_info().hits == 1 # the call was a cache hit assert cls2.cached_fn.cache_info().misses == 2 - - -@pytest.fixture -def operation_def(): - return {'name': 'CreateFoo'} From 30d7c126b07345e853de16fd499463e1355afa36 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 19 Jul 2023 16:53:13 -0400 Subject: [PATCH 21/37] extract dict type normalization into separate function --- botocore/compress.py | 47 +++++++++++++++++++------------------ tests/unit/test_compress.py | 2 +- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 53c95f6e54..c0f4571b4c 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -41,29 +41,34 @@ def _should_compress_request(config, request_dict, operation_model): and config.signature_version != 'v2' and operation_model.request_compression is not None ): - # Requests with streaming input are compressed regardless of - # `request_min_compression_size_bytes` if they don't contain the - # `requiresLength` trait. - if operation_model.has_streaming_input: - return ( - 'requiresLength' - not in operation_model.get_streaming_input().metadata + if not _is_compressible_type(request_dict): + body_type = type(request_dict['body']) + logger.debug( + 'Body type %s does not support compression.', body_type ) - else: - # If the request body is a dictionary, we need to encode it to get the - # length. If compression should occur, the encoded body will be saved - # to the request dictionary so it doesn't need to be encoded again. - body = request_dict['body'] - if isinstance(body, dict): - body = urlencode(body, doseq=True, encoding='utf-8') - config_min = config.request_min_compression_size_bytes - should_compress = config_min <= _get_body_size(body) - if should_compress: - request_dict['body'] = body - return should_compress + return False + + if operation_model.has_streaming_input: + streaming_input = operation_model.get_streaming_input() + streaming_metadata = streaming_input.metadata + return 'requiresLength' not in streaming_metadata + + body_size = _get_body_size(request_dict['body']) + min_size = config.request_min_compression_size_bytes + return min_size <= body_size return False +def _is_compressible_type(request_dict): + body = request_dict['body'] + # Coerce dict to a format compatible with compression. + if isinstance(body, dict): + body = urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') + request_dict['body'] = body + is_supported_type = isinstance(body, (str, bytes, bytearray)) + return is_supported_type or hasattr(body, 'read') + + def _get_body_size(body): size = determine_content_length(body) if size is None: @@ -88,10 +93,6 @@ def _gzip_compress_body(body): body.seek(current_position) return compressed_obj return _gzip_compress_fileobj(body) - elif isinstance(body, dict): - return gzip.compress( - urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') - ) def _gzip_compress_fileobj(body): diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 3901fce215..b50311c327 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -94,7 +94,7 @@ def _assert_compression(body, maybe_compressed_body, encoding): maybe_compressed_body = maybe_compressed_body.read() if isinstance(body, str): body = body.encode('utf-8') - if isinstance(body, dict) and encoding is not None: + if isinstance(body, dict): body = urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') decompress_method = DECOMPRESSION_METHOD_MAP.get( encoding, lambda body: body From b9334df6567efb2cf2364611c9e360a5e5edf27e Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 11:05:14 -0400 Subject: [PATCH 22/37] refactor unit tests --- botocore/compress.py | 1 + tests/unit/test_compress.py | 269 ++++++++++++++++++++++-------------- 2 files changed, 163 insertions(+), 107 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index c0f4571b4c..1ca9f99a7c 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -56,6 +56,7 @@ def _should_compress_request(config, request_dict, operation_model): body_size = _get_body_size(request_dict['body']) min_size = config.request_min_compression_size_bytes return min_size <= body_size + return False diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index b50311c327..40f725efbb 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -36,6 +36,7 @@ def _make_op( return op +OP_NO_COMPRESSION = _make_op() OP_WITH_COMPRESSION = _make_op(request_compression={'encodings': ['gzip']}) OP_UNKNOWN_COMPRESSION = _make_op(request_compression={'encodings': ['foo']}) OP_MULTIPLE_COMPRESSIONS = _make_op( @@ -71,35 +72,49 @@ def _make_op( ) -def request_dict(): +def _request_dict(body=None, headers=None): + if body is None: + body = b'' + if headers is None: + headers = {} + return { - 'body': REQUEST_BODY, - 'headers': {'foo': 'bar'}, + 'body': body, + 'headers': headers, } -def request_dict_with_content_encoding_header(): - return { - 'body': REQUEST_BODY, - 'headers': {'foo': 'bar', 'Content-Encoding': 'identity'}, - } +def default_request_dict(): + return _request_dict(REQUEST_BODY) -DECOMPRESSION_METHOD_MAP = {'gzip': gzip.decompress} +def request_dict_string(): + return _request_dict(REQUEST_BODY.decode('utf-8')) + +def request_dict_bytearray(): + return _request_dict(bytearray(REQUEST_BODY)) -def _assert_compression(body, maybe_compressed_body, encoding): - if hasattr(body, 'read'): - body = body.read() - maybe_compressed_body = maybe_compressed_body.read() - if isinstance(body, str): - body = body.encode('utf-8') - if isinstance(body, dict): - body = urlencode(body, doseq=True, encoding='utf-8').encode('utf-8') - decompress_method = DECOMPRESSION_METHOD_MAP.get( - encoding, lambda body: body + +def request_dict_with_content_encoding_header(): + return _request_dict( + REQUEST_BODY, {'foo': b'bar', 'Content-Encoding': 'identity'} ) - assert decompress_method(maybe_compressed_body) == body + + +def request_dict_string_io(): + return _request_dict(io.StringIO(REQUEST_BODY.decode('utf-8'))) + + +def request_dict_bytes_io(): + return _request_dict(io.BytesIO(REQUEST_BODY)) + + +def request_dict_dict(): + return _request_dict({'foo': 'bar'}) + + +DECOMPRESSION_METHOD_MAP = {'gzip': gzip.decompress} def _bad_compression(body): @@ -110,151 +125,191 @@ def _bad_compression(body): MOCK_COMPRESSION.update(COMPRESSION_MAPPING) +def _assert_compression_body(original_body, compressed_body, encoding): + if hasattr(original_body, 'read'): + original_body = original_body.read() + compressed_body = compressed_body.read() + if isinstance(original_body, str): + original_body = original_body.encode('utf-8') + decompress = DECOMPRESSION_METHOD_MAP[encoding] + assert original_body == decompress(compressed_body) + + +def _assert_compression_header(request_dict, encoding): + assert ( + 'headers' in request_dict + and 'Content-Encoding' in request_dict['headers'] + and encoding in request_dict['headers']['Content-Encoding'] + ) + + @pytest.mark.parametrize( - 'config, request_dict, operation_model, is_compressed, encoding', + 'config, request_dict, operation_model, encoding', [ - ( - Config( - disable_request_compression=True, - request_min_compression_size_bytes=1000, - ), - {'body': b'foo', 'headers': {}}, - OP_WITH_COMPRESSION, - False, - None, - ), ( COMPRESSION_CONFIG_128_BYTES, - request_dict(), + default_request_dict(), OP_WITH_COMPRESSION, - True, 'gzip', ), ( - Config( - disable_request_compression=False, - request_min_compression_size_bytes=256, - ), - request_dict(), - OP_WITH_COMPRESSION, - False, - None, - ), - ( - Config(request_min_compression_size_bytes=128), - request_dict(), + COMPRESSION_CONFIG_128_BYTES, + default_request_dict(), OP_WITH_COMPRESSION, - True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - request_dict(), + default_request_dict(), OP_MULTIPLE_COMPRESSIONS, - True, 'gzip', ), ( DEFAULT_COMPRESSION_CONFIG, - request_dict(), + default_request_dict(), STREAMING_OP_WITH_COMPRESSION, - True, 'gzip', ), - ( - DEFAULT_COMPRESSION_CONFIG, - request_dict(), - STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH, - False, - None, - ), - ( - DEFAULT_COMPRESSION_CONFIG, - request_dict(), - _make_op(), - False, - None, - ), ( COMPRESSION_CONFIG_128_BYTES, - request_dict(), - OP_UNKNOWN_COMPRESSION, - False, - None, + request_dict_bytearray(), + OP_WITH_COMPRESSION, + 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': REQUEST_BODY.decode(), 'headers': {}}, + request_dict_with_content_encoding_header(), OP_WITH_COMPRESSION, - True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': bytearray(REQUEST_BODY), 'headers': {}}, + request_dict_string(), OP_WITH_COMPRESSION, - True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': io.BytesIO(REQUEST_BODY), 'headers': {}}, + request_dict_bytes_io(), OP_WITH_COMPRESSION, - True, 'gzip', ), ( COMPRESSION_CONFIG_128_BYTES, - {'body': io.StringIO(REQUEST_BODY.decode()), 'headers': {}}, + request_dict_string_io(), OP_WITH_COMPRESSION, - True, 'gzip', ), + ], +) +def test_maybe_compress( + config, + request_dict, + operation_model, + encoding, +): + original_body = request_dict['body'] + maybe_compress_request(config, request_dict, operation_model) + _assert_compression_body(original_body, request_dict['body'], encoding) + _assert_compression_header(request_dict, encoding) + + +def test_maybe_compress_dict(): + request_dict = request_dict_dict() + original_body = request_dict['body'] + encoded_body = urlencode( + original_body, doseq=True, encoding='utf-8' + ).encode('utf-8') + encoding = 'gzip' + maybe_compress_request( + COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION + ) + _assert_compression_body(encoded_body, request_dict['body'], encoding) + _assert_compression_header(request_dict, encoding) + + +@pytest.mark.parametrize( + 'config, request_dict, operation_model', + [ + ( + Config( + disable_request_compression=True, + request_min_compression_size_bytes=1000, + ), + default_request_dict(), + OP_WITH_COMPRESSION, + ), + ( + Config( + disable_request_compression=False, + request_min_compression_size_bytes=256, + ), + default_request_dict(), + OP_WITH_COMPRESSION, + ), + ( + DEFAULT_COMPRESSION_CONFIG, + default_request_dict(), + STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH, + ), + ( + DEFAULT_COMPRESSION_CONFIG, + default_request_dict(), + OP_NO_COMPRESSION, + ), ( COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), + default_request_dict(), OP_UNKNOWN_COMPRESSION, - False, - 'foo', ), ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), + DEFAULT_COMPRESSION_CONFIG, + request_dict_string(), OP_WITH_COMPRESSION, - True, - 'gzip', ), ( - COMPRESSION_CONFIG_1_BYTE, - {'body': {'foo': 'bar'}, 'headers': {}}, + DEFAULT_COMPRESSION_CONFIG, + request_dict_bytearray(), OP_WITH_COMPRESSION, - True, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - {'body': {'foo': 'bar'}, 'headers': {}}, + DEFAULT_COMPRESSION_CONFIG, + request_dict_bytes_io(), OP_WITH_COMPRESSION, - False, - None, + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict_string_io(), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + request_dict_with_content_encoding_header(), + OP_UNKNOWN_COMPRESSION, ), ], ) -def test_maybe_compress( +def test_no_compression( config, request_dict, operation_model, - is_compressed, - encoding, ): + ce_header = request_dict['headers'].get('Content-Encoding') original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) - _assert_compression(original_body, request_dict['body'], encoding) - assert ( - 'headers' in request_dict - and 'Content-Encoding' in request_dict['headers'] - and encoding in request_dict['headers']['Content-Encoding'] - ) == is_compressed + assert request_dict['body'] is original_body + assert ce_header == request_dict['headers'].get('Content-Encoding') + + +def test_dict_no_compression(): + request_dict = request_dict_dict() + original_body = request_dict['body'] + maybe_compress_request( + COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION + ) + assert request_dict['body'] == urlencode( + original_body, doseq=True, encoding='utf-8' + ).encode('utf-8') @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) @@ -267,16 +322,16 @@ def test_maybe_compress_bad_types(body): @pytest.mark.parametrize( - 'body', - [io.StringIO('foo'), io.BytesIO(b'foo')], + 'request_dict', + [request_dict_string_io(), request_dict_bytes_io()], ) -def test_body_streams_position_reset(body): +def test_body_streams_position_reset(request_dict): maybe_compress_request( - COMPRESSION_CONFIG_1_BYTE, - {'body': body, 'headers': {}}, + COMPRESSION_CONFIG_128_BYTES, + request_dict, OP_WITH_COMPRESSION, ) - assert body.tell() == 0 + assert request_dict['body'].tell() == 0 def test_only_compress_once(): @@ -285,4 +340,4 @@ def test_only_compress_once(): maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION ) - _assert_compression(REQUEST_BODY, request_dict['body'], 'gzip') + _assert_compression_body(REQUEST_BODY, request_dict['body'], 'gzip') From e93321f2811877364c48d41962dc584da7cff179 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 12:11:48 -0400 Subject: [PATCH 23/37] adjust compression assertion method and move dict compression into parametrized test --- tests/unit/test_compress.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 40f725efbb..d2ece0019e 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -131,6 +131,10 @@ def _assert_compression_body(original_body, compressed_body, encoding): compressed_body = compressed_body.read() if isinstance(original_body, str): original_body = original_body.encode('utf-8') + if isinstance(original_body, dict): + original_body = urlencode( + original_body, doseq=True, encoding='utf-8' + ).encode('utf-8') decompress = DECOMPRESSION_METHOD_MAP[encoding] assert original_body == decompress(compressed_body) @@ -200,6 +204,12 @@ def _assert_compression_header(request_dict, encoding): OP_WITH_COMPRESSION, 'gzip', ), + ( + COMPRESSION_CONFIG_1_BYTE, + request_dict_dict(), + OP_WITH_COMPRESSION, + 'gzip', + ), ], ) def test_maybe_compress( @@ -214,20 +224,6 @@ def test_maybe_compress( _assert_compression_header(request_dict, encoding) -def test_maybe_compress_dict(): - request_dict = request_dict_dict() - original_body = request_dict['body'] - encoded_body = urlencode( - original_body, doseq=True, encoding='utf-8' - ).encode('utf-8') - encoding = 'gzip' - maybe_compress_request( - COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION - ) - _assert_compression_body(encoded_body, request_dict['body'], encoding) - _assert_compression_header(request_dict, encoding) - - @pytest.mark.parametrize( 'config, request_dict, operation_model', [ From ebcd98b87bcb97c85a2f72768ed184b27df4d412 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 14:42:58 -0400 Subject: [PATCH 24/37] formatting cleanup --- tests/unit/test_compress.py | 60 +++++++++++++------------------------ 1 file changed, 20 insertions(+), 40 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index d2ece0019e..073a0eecc2 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -37,20 +37,18 @@ def _make_op( OP_NO_COMPRESSION = _make_op() -OP_WITH_COMPRESSION = _make_op(request_compression={'encodings': ['gzip']}) -OP_UNKNOWN_COMPRESSION = _make_op(request_compression={'encodings': ['foo']}) -OP_MULTIPLE_COMPRESSIONS = _make_op( - request_compression={'encodings': ['gzip', 'foo']} -) +OP_WITH_COMPRESSION = _make_op({'encodings': ['gzip']}) +OP_UNKNOWN_COMPRESSION = _make_op({'encodings': ['foo']}) +OP_MULTIPLE_COMPRESSIONS = _make_op({'encodings': ['gzip', 'foo']}) STREAMING_OP_WITH_COMPRESSION = _make_op( - request_compression={'encodings': ['gzip']}, - has_streaming_input=True, - streaming_metadata={}, + {'encodings': ['gzip']}, + True, + {}, ) STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH = _make_op( - request_compression={'encodings': ['gzip']}, - has_streaming_input=True, - streaming_metadata={'requiresLength': True}, + {'encodings': ['gzip']}, + True, + {'requiresLength': True}, ) REQUEST_BODY = ( b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' @@ -129,20 +127,17 @@ def _assert_compression_body(original_body, compressed_body, encoding): if hasattr(original_body, 'read'): original_body = original_body.read() compressed_body = compressed_body.read() + if isinstance(original_body, dict): + original_body = urlencode(original_body, doseq=True, encoding='utf-8') if isinstance(original_body, str): original_body = original_body.encode('utf-8') - if isinstance(original_body, dict): - original_body = urlencode( - original_body, doseq=True, encoding='utf-8' - ).encode('utf-8') decompress = DECOMPRESSION_METHOD_MAP[encoding] assert original_body == decompress(compressed_body) def _assert_compression_header(request_dict, encoding): assert ( - 'headers' in request_dict - and 'Content-Encoding' in request_dict['headers'] + 'Content-Encoding' in request_dict['headers'] and encoding in request_dict['headers']['Content-Encoding'] ) @@ -156,12 +151,6 @@ def _assert_compression_header(request_dict, encoding): OP_WITH_COMPRESSION, 'gzip', ), - ( - COMPRESSION_CONFIG_128_BYTES, - default_request_dict(), - OP_WITH_COMPRESSION, - 'gzip', - ), ( COMPRESSION_CONFIG_128_BYTES, default_request_dict(), @@ -212,12 +201,7 @@ def _assert_compression_header(request_dict, encoding): ), ], ) -def test_maybe_compress( - config, - request_dict, - operation_model, - encoding, -): +def test_compression(config, request_dict, operation_model, encoding): original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) _assert_compression_body(original_body, request_dict['body'], encoding) @@ -285,11 +269,7 @@ def test_maybe_compress( ), ], ) -def test_no_compression( - config, - request_dict, - operation_model, -): +def test_no_compression(config, request_dict, operation_model): ce_header = request_dict['headers'].get('Content-Encoding') original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) @@ -303,18 +283,18 @@ def test_dict_no_compression(): maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION ) - assert request_dict['body'] == urlencode( - original_body, doseq=True, encoding='utf-8' - ).encode('utf-8') + body = request_dict['body'] + encoded_body = urlencode(original_body, doseq=True, encoding='utf-8') + assert body == encoded_body.encode('utf-8') @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) def test_maybe_compress_bad_types(body): - request_dict = {'body': body, 'headers': {}} + request_dict = _request_dict(body) maybe_compress_request( COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION ) - assert request_dict['body'] == body + assert request_dict['body'] is body @pytest.mark.parametrize( @@ -332,7 +312,7 @@ def test_body_streams_position_reset(request_dict): def test_only_compress_once(): with mock.patch('botocore.compress.COMPRESSION_MAPPING', MOCK_COMPRESSION): - request_dict = {'body': REQUEST_BODY, 'headers': {}} + request_dict = default_request_dict() maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION ) From ccd81fafd3e4998ee8cfc1674e0af5b4fd1d16d1 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 16:31:57 -0400 Subject: [PATCH 25/37] actually convert request_compression_min_size_bytes to int and some more formatting fixes --- botocore/args.py | 25 +++++++++++++------------ tests/unit/test_args.py | 6 ++++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/botocore/args.py b/botocore/args.py index 6717ce220b..83e6b1e27e 100644 --- a/botocore/args.py +++ b/botocore/args.py @@ -557,11 +557,12 @@ def _compute_request_compression_config(self, config_kwargs): min_size = self._config_store.get_config_variable( 'request_min_compression_size_bytes' ) - config_kwargs['request_min_compression_size_bytes'] = min_size # conversion func is skipped so input validation must be done here # regardless if the value is coming from the config store or the # config object - self._validate_min_compression_size(min_size) + min_size = self._validate_min_compression_size(min_size) + config_kwargs['request_min_compression_size_bytes'] = min_size + if disabled is None: disabled = self._config_store.get_config_variable( 'disable_request_compression' @@ -582,19 +583,19 @@ def _validate_min_compression_size(self, min_size): try: min_size = int(min_size) except (ValueError, TypeError): - raise botocore.exceptions.InvalidConfigError( - error_msg=( - f'{error_msg_base} Value must be an integer. ' - f'Received {type(min_size)} instead.' - ) + msg = ( + f'{error_msg_base} Value must be an integer. ' + f'Received {type(min_size)} instead.' ) + raise botocore.exceptions.InvalidConfigError(error_msg=msg) if not min_allowed_min_size <= min_size <= max_allowed_min_size: - raise botocore.exceptions.InvalidConfigError( - error_msg=( - f'{error_msg_base} Value must be between ' - f'{min_allowed_min_size} and {max_allowed_min_size}.' - ) + msg = ( + f'{error_msg_base} Value must be between ' + f'{min_allowed_min_size} and {max_allowed_min_size}.' ) + raise botocore.exceptions.InvalidConfigError(error_msg=msg) + + return min_size def _ensure_boolean(self, val): if isinstance(val, bool): diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index 7efa764cea..dfdcd9d514 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -566,6 +566,12 @@ def test_request_compression_client_config_overrides_config_store(self): self.assertEqual(config.request_min_compression_size_bytes, 1) self.assertFalse(config.disable_request_compression) + def test_coercible_value_request_min_compression_size_bytes(self): + config = Config(request_min_compression_size_bytes='100') + client_args = self.call_get_client_args(client_config=config) + config = client_args['client_config'] + self.assertEqual(config.request_min_compression_size_bytes, 100) + def test_bad_type_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): config = Config(request_min_compression_size_bytes='foo') From 82e6bedacf4555066ce91cef07f6e5a274620ad0 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 16:47:09 -0400 Subject: [PATCH 26/37] add test case for coercible boolean and small fix to no compression test --- tests/unit/test_args.py | 6 ++++++ tests/unit/test_compress.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index dfdcd9d514..8f1c992422 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -572,6 +572,12 @@ def test_coercible_value_request_min_compression_size_bytes(self): config = client_args['client_config'] self.assertEqual(config.request_min_compression_size_bytes, 100) + def test_coercible_value_disable_request_compression(self): + config = Config(disable_request_compression='true') + client_args = self.call_get_client_args(client_config=config) + config = client_args['client_config'] + self.assertTrue(config.disable_request_compression) + def test_bad_type_request_min_compression_size_bytes(self): with self.assertRaises(exceptions.InvalidConfigError): config = Config(request_min_compression_size_bytes='foo') diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 073a0eecc2..d9389de3d0 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -274,7 +274,7 @@ def test_no_compression(config, request_dict, operation_model): original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) assert request_dict['body'] is original_body - assert ce_header == request_dict['headers'].get('Content-Encoding') + assert ce_header is request_dict['headers'].get('Content-Encoding') def test_dict_no_compression(): From 19586f4c46e076060a8adad6610b192e447e2c5b Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 17:04:22 -0400 Subject: [PATCH 27/37] fixed test --- tests/unit/test_compress.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index d9389de3d0..a0a1119443 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -291,6 +291,7 @@ def test_dict_no_compression(): @pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) def test_maybe_compress_bad_types(body): request_dict = _request_dict(body) + body = request_dict['body'] maybe_compress_request( COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION ) From acd343ddbb1174d881e67c95740e1fdce40a729d Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 17:09:00 -0400 Subject: [PATCH 28/37] actually fixed test --- tests/unit/test_compress.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index a0a1119443..02520f5300 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -288,10 +288,9 @@ def test_dict_no_compression(): assert body == encoded_body.encode('utf-8') -@pytest.mark.parametrize('body', [1, object(), None, True, 1.0]) +@pytest.mark.parametrize('body', [1, object(), True, 1.0]) def test_maybe_compress_bad_types(body): request_dict = _request_dict(body) - body = request_dict['body'] maybe_compress_request( COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION ) From 9b6b03a39b245c5af0c159f72368c9e512177e71 Mon Sep 17 00:00:00 2001 From: davidlm Date: Tue, 25 Jul 2023 18:09:13 -0400 Subject: [PATCH 29/37] assert_compression method --- tests/unit/test_compress.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 02520f5300..5f816e0ee4 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -135,13 +135,20 @@ def _assert_compression_body(original_body, compressed_body, encoding): assert original_body == decompress(compressed_body) -def _assert_compression_header(request_dict, encoding): +def _assert_compression_header(headers, encoding): assert ( - 'Content-Encoding' in request_dict['headers'] - and encoding in request_dict['headers']['Content-Encoding'] + 'Content-Encoding' in headers + and encoding in headers['Content-Encoding'] ) +def assert_compression(original_body, request_dict, encoding): + compressed_body = request_dict['body'] + headers = request_dict['headers'] + _assert_compression_body(original_body, compressed_body, encoding) + _assert_compression_header(headers, encoding) + + @pytest.mark.parametrize( 'config, request_dict, operation_model, encoding', [ @@ -204,8 +211,7 @@ def _assert_compression_header(request_dict, encoding): def test_compression(config, request_dict, operation_model, encoding): original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) - _assert_compression_body(original_body, request_dict['body'], encoding) - _assert_compression_header(request_dict, encoding) + assert_compression(original_body, request_dict, encoding) @pytest.mark.parametrize( @@ -313,7 +319,8 @@ def test_body_streams_position_reset(request_dict): def test_only_compress_once(): with mock.patch('botocore.compress.COMPRESSION_MAPPING', MOCK_COMPRESSION): request_dict = default_request_dict() + body = request_dict['body'] maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION ) - _assert_compression_body(REQUEST_BODY, request_dict['body'], 'gzip') + assert_compression(body, request_dict, 'gzip') From a5071e154706d42ce48c34d8af1d96d716f06282 Mon Sep 17 00:00:00 2001 From: davidlm Date: Wed, 26 Jul 2023 10:54:14 -0400 Subject: [PATCH 30/37] add test cases for non-seekable streams --- tests/functional/test_compress.py | 2 +- tests/unit/test_compress.py | 60 +++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_compress.py b/tests/functional/test_compress.py index 866469f2dc..e069606b93 100644 --- a/tests/functional/test_compress.py +++ b/tests/functional/test_compress.py @@ -99,7 +99,7 @@ def test_no_unknown_compression_encodings(operation_model): for encoding in operation_model.request_compression["encodings"]: assert encoding in KNOWN_COMPRESSION_ENCODINGS, ( f"Found unknown compression encoding '{encoding}' " - f"in operation {operation_model.name}" + f"in operation {operation_model.name}." ) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 5f816e0ee4..0181cd532c 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -12,6 +12,7 @@ # language governing permissions and limitations under the License. import gzip import io +from copy import deepcopy import pytest @@ -56,6 +57,8 @@ def _make_op( b'&MetricData.member.1.Value=128' ) +REQUEST_BODY_STRING = REQUEST_BODY.decode('utf-8') + DEFAULT_COMPRESSION_CONFIG = Config( disable_request_compression=False, request_min_compression_size_bytes=10420, @@ -70,6 +73,14 @@ def _make_op( ) +class NonSeekableStream: + def __init__(self, buffer): + self._buffer = buffer + + def read(self, size=None): + return self._buffer.read(size) + + def _request_dict(body=None, headers=None): if body is None: body = b'' @@ -87,7 +98,7 @@ def default_request_dict(): def request_dict_string(): - return _request_dict(REQUEST_BODY.decode('utf-8')) + return _request_dict(REQUEST_BODY_STRING) def request_dict_bytearray(): @@ -101,13 +112,21 @@ def request_dict_with_content_encoding_header(): def request_dict_string_io(): - return _request_dict(io.StringIO(REQUEST_BODY.decode('utf-8'))) + return _request_dict(io.StringIO(REQUEST_BODY_STRING)) def request_dict_bytes_io(): return _request_dict(io.BytesIO(REQUEST_BODY)) +def request_dict_non_seekable_text_stream(): + return _request_dict(NonSeekableStream(io.StringIO(REQUEST_BODY_STRING))) + + +def request_dict_non_seekable_bytes_stream(): + return _request_dict(NonSeekableStream(io.BytesIO(REQUEST_BODY))) + + def request_dict_dict(): return _request_dict({'foo': 'bar'}) @@ -214,6 +233,33 @@ def test_compression(config, request_dict, operation_model, encoding): assert_compression(original_body, request_dict, encoding) +@pytest.mark.parametrize( + 'config, request_dict, operation_model, encoding', + [ + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict_non_seekable_bytes_stream(), + STREAMING_OP_WITH_COMPRESSION, + 'gzip', + ), + ( + DEFAULT_COMPRESSION_CONFIG, + request_dict_non_seekable_text_stream(), + STREAMING_OP_WITH_COMPRESSION, + 'gzip', + ), + ], +) +def test_compression_non_seekable_streams( + config, request_dict, operation_model, encoding +): + # since the body can't be reset, we must make a copy + # of the original body to test against + original_body = deepcopy(request_dict['body']) + maybe_compress_request(config, request_dict, operation_model) + assert_compression(original_body, request_dict, encoding) + + @pytest.mark.parametrize( 'config, request_dict, operation_model', [ @@ -273,6 +319,16 @@ def test_compression(config, request_dict, operation_model, encoding): request_dict_with_content_encoding_header(), OP_UNKNOWN_COMPRESSION, ), + ( + COMPRESSION_CONFIG_1_BYTE, + request_dict_non_seekable_bytes_stream(), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + request_dict_non_seekable_text_stream(), + OP_WITH_COMPRESSION, + ), ], ) def test_no_compression(config, request_dict, operation_model): From 8ed62a876571c7d4ed3fdfbfb13d2a9c581487e0 Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 27 Jul 2023 10:40:37 -0400 Subject: [PATCH 31/37] pr feedback --- botocore/compress.py | 5 +- tests/functional/test_compress.py | 5 +- tests/unit/test_compress.py | 76 +++++++++++++++++++++---------- 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 1ca9f99a7c..d78b3053ed 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -43,9 +43,8 @@ def _should_compress_request(config, request_dict, operation_model): ): if not _is_compressible_type(request_dict): body_type = type(request_dict['body']) - logger.debug( - 'Body type %s does not support compression.', body_type - ) + log_msg = 'Body type %s does not support compression.' + logger.debug(log_msg, body_type) return False if operation_model.has_streaming_input: diff --git a/tests/functional/test_compress.py b/tests/functional/test_compress.py index e069606b93..53ed482b21 100644 --- a/tests/functional/test_compress.py +++ b/tests/functional/test_compress.py @@ -15,11 +15,10 @@ import pytest +from botocore.compress import COMPRESSION_MAPPING from botocore.config import Config from tests import ALL_SERVICES, ClientHTTPStubber, patch_load_service_model -KNOWN_COMPRESSION_ENCODINGS = ("gzip",) - FAKE_MODEL = { "version": "2.0", "documentation": "", @@ -97,7 +96,7 @@ def _all_compression_operations(): @pytest.mark.parametrize("operation_model", _all_compression_operations()) def test_no_unknown_compression_encodings(operation_model): for encoding in operation_model.request_compression["encodings"]: - assert encoding in KNOWN_COMPRESSION_ENCODINGS, ( + assert encoding in COMPRESSION_MAPPING.keys(), ( f"Found unknown compression encoding '{encoding}' " f"in operation {operation_model.name}." ) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 0181cd532c..3cf601b19b 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -10,6 +10,11 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +""" +NOTE: All classes and functions in this module are considered private and are +subject to abrupt breaking changes. Please do not use them directly. + +""" import gzip import io from copy import deepcopy @@ -59,9 +64,10 @@ def _make_op( REQUEST_BODY_STRING = REQUEST_BODY.decode('utf-8') -DEFAULT_COMPRESSION_CONFIG = Config( + +COMPRESSION_CONFIG_10240_BYTES = Config( disable_request_compression=False, - request_min_compression_size_bytes=10420, + request_min_compression_size_bytes=10240, ) COMPRESSION_CONFIG_128_BYTES = Config( disable_request_compression=False, @@ -81,9 +87,7 @@ def read(self, size=None): return self._buffer.read(size) -def _request_dict(body=None, headers=None): - if body is None: - body = b'' +def _request_dict(body=b'', headers=None): if headers is None: headers = {} @@ -184,7 +188,7 @@ def assert_compression(original_body, request_dict, encoding): 'gzip', ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, default_request_dict(), STREAMING_OP_WITH_COMPRESSION, 'gzip', @@ -237,13 +241,13 @@ def test_compression(config, request_dict, operation_model, encoding): 'config, request_dict, operation_model, encoding', [ ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_non_seekable_bytes_stream(), STREAMING_OP_WITH_COMPRESSION, 'gzip', ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_non_seekable_text_stream(), STREAMING_OP_WITH_COMPRESSION, 'gzip', @@ -280,12 +284,12 @@ def test_compression_non_seekable_streams( OP_WITH_COMPRESSION, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, default_request_dict(), STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, default_request_dict(), OP_NO_COMPRESSION, ), @@ -295,22 +299,22 @@ def test_compression_non_seekable_streams( OP_UNKNOWN_COMPRESSION, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_string(), OP_WITH_COMPRESSION, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_bytearray(), OP_WITH_COMPRESSION, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_bytes_io(), OP_WITH_COMPRESSION, ), ( - DEFAULT_COMPRESSION_CONFIG, + COMPRESSION_CONFIG_10240_BYTES, request_dict_string_io(), OP_WITH_COMPRESSION, ), @@ -329,6 +333,41 @@ def test_compression_non_seekable_streams( request_dict_non_seekable_text_stream(), OP_WITH_COMPRESSION, ), + ( + COMPRESSION_CONFIG_1_BYTE, + request_dict_non_seekable_text_stream(), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(1), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(1.0), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(object()), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(True), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(None), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_1_BYTE, + _request_dict(set()), + OP_WITH_COMPRESSION, + ), ], ) def test_no_compression(config, request_dict, operation_model): @@ -350,15 +389,6 @@ def test_dict_no_compression(): assert body == encoded_body.encode('utf-8') -@pytest.mark.parametrize('body', [1, object(), True, 1.0]) -def test_maybe_compress_bad_types(body): - request_dict = _request_dict(body) - maybe_compress_request( - COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION - ) - assert request_dict['body'] is body - - @pytest.mark.parametrize( 'request_dict', [request_dict_string_io(), request_dict_bytes_io()], From 004a175da8b49cd6d39d4879e3625e9cd6a12e56 Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 27 Jul 2023 10:47:17 -0400 Subject: [PATCH 32/37] put private note in wrong file. Also removed `classes` since there arent any --- botocore/compress.py | 5 +++++ tests/unit/test_compress.py | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index d78b3053ed..13c74636a2 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -10,6 +10,11 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +""" +NOTE: All functions in this module are considered private and are +subject to abrupt breaking changes. Please do not use them directly. + +""" import gzip import io diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 3cf601b19b..5a2f5507c5 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -10,11 +10,6 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. -""" -NOTE: All classes and functions in this module are considered private and are -subject to abrupt breaking changes. Please do not use them directly. - -""" import gzip import io from copy import deepcopy From bd6249a40a32f6647091fbe9d8f70b6136bd5af6 Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 27 Jul 2023 10:59:24 -0400 Subject: [PATCH 33/37] remove duplicate test case --- tests/unit/test_compress.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 5a2f5507c5..a7044ff7df 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -328,11 +328,6 @@ def test_compression_non_seekable_streams( request_dict_non_seekable_text_stream(), OP_WITH_COMPRESSION, ), - ( - COMPRESSION_CONFIG_1_BYTE, - request_dict_non_seekable_text_stream(), - OP_WITH_COMPRESSION, - ), ( COMPRESSION_CONFIG_1_BYTE, _request_dict(1), From 4011831a89c6c0e7a47bec6ca494afd6cb623758 Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Thu, 27 Jul 2023 12:54:10 -0600 Subject: [PATCH 34/37] Refactor unit tests --- botocore/compress.py | 9 +- tests/unit/test_compress.py | 317 ++++++++++++++---------------------- 2 files changed, 124 insertions(+), 202 deletions(-) diff --git a/botocore/compress.py b/botocore/compress.py index 13c74636a2..1f8577e84b 100644 --- a/botocore/compress.py +++ b/botocore/compress.py @@ -16,9 +16,10 @@ """ -import gzip import io import logging +from gzip import GzipFile +from gzip import compress as gzip_compress from botocore.compat import urlencode from botocore.utils import determine_content_length @@ -88,9 +89,9 @@ def _get_body_size(body): def _gzip_compress_body(body): if isinstance(body, str): - return gzip.compress(body.encode('utf-8')) + return gzip_compress(body.encode('utf-8')) elif isinstance(body, (bytes, bytearray)): - return gzip.compress(body) + return gzip_compress(body) elif hasattr(body, 'read'): if hasattr(body, 'seek') and hasattr(body, 'tell'): current_position = body.tell() @@ -102,7 +103,7 @@ def _gzip_compress_body(body): def _gzip_compress_fileobj(body): compressed_obj = io.BytesIO() - with gzip.GzipFile(fileobj=compressed_obj, mode='wb') as gz: + with GzipFile(fileobj=compressed_obj, mode='wb') as gz: while True: chunk = body.read(8192) if not chunk: diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index a7044ff7df..480421c5ac 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -12,11 +12,11 @@ # language governing permissions and limitations under the License. import gzip import io -from copy import deepcopy +import sys import pytest -from botocore.compat import urlencode +import botocore from botocore.compress import COMPRESSION_MAPPING, maybe_compress_request from botocore.config import Config from tests import mock @@ -51,19 +51,22 @@ def _make_op( True, {'requiresLength': True}, ) + + REQUEST_BODY = ( b'Action=PutMetricData&Version=2010-08-01&Namespace=Namespace' b'&MetricData.member.1.MetricName=metric&MetricData.member.1.Unit=Bytes' b'&MetricData.member.1.Value=128' ) - -REQUEST_BODY_STRING = REQUEST_BODY.decode('utf-8') +REQUEST_BODY_COMPRESSED = ( + b'\x1f\x8b\x08\x00\x01\x00\x00\x00\x02\xffsL.\xc9\xcc\xcf\xb3\r(-\xf1M-)' + b'\xcaLvI,IT\x0bK-*\x06\x89\x1a\x19\x18\x1a\xe8\x1aX\xe8\x1a\x18\xaa\xf9%' + b'\xe6\xa6\x16\x17$&\xa7\xda\xc2Yj\x08\x1dz\xb9\xa9\xb9I\xa9Ez\x86z\x101\x90' + b'\x1a\xdb\\0\x13\xab\xaa\xd0\xbc\xcc\x12[\xa7\xca\x92\xd4b\xac\xd2a\x899\xa5' + b'\xa9\xb6\x86F\x16\x00\x1e\xdd\t\xfd\x9e\x00\x00\x00' +) -COMPRESSION_CONFIG_10240_BYTES = Config( - disable_request_compression=False, - request_min_compression_size_bytes=10240, -) COMPRESSION_CONFIG_128_BYTES = Config( disable_request_compression=False, request_min_compression_size_bytes=128, @@ -82,7 +85,7 @@ def read(self, size=None): return self._buffer.read(size) -def _request_dict(body=b'', headers=None): +def _request_dict(body=REQUEST_BODY, headers=None): if headers is None: headers = {} @@ -92,45 +95,24 @@ def _request_dict(body=b'', headers=None): } -def default_request_dict(): - return _request_dict(REQUEST_BODY) - - -def request_dict_string(): - return _request_dict(REQUEST_BODY_STRING) - - -def request_dict_bytearray(): - return _request_dict(bytearray(REQUEST_BODY)) - - -def request_dict_with_content_encoding_header(): - return _request_dict( - REQUEST_BODY, {'foo': b'bar', 'Content-Encoding': 'identity'} - ) - - -def request_dict_string_io(): - return _request_dict(io.StringIO(REQUEST_BODY_STRING)) - - -def request_dict_bytes_io(): - return _request_dict(io.BytesIO(REQUEST_BODY)) - - def request_dict_non_seekable_text_stream(): - return _request_dict(NonSeekableStream(io.StringIO(REQUEST_BODY_STRING))) + stream = NonSeekableStream(io.StringIO(REQUEST_BODY.decode('utf-8'))) + return _request_dict(stream) def request_dict_non_seekable_bytes_stream(): return _request_dict(NonSeekableStream(io.BytesIO(REQUEST_BODY))) -def request_dict_dict(): - return _request_dict({'foo': 'bar'}) +class StaticGzipFile(gzip.GzipFile): + def __init__(self, *args, **kwargs): + kwargs['mtime'] = 1 + super().__init__(*args, **kwargs) -DECOMPRESSION_METHOD_MAP = {'gzip': gzip.decompress} +def static_compress(*args, **kwargs): + kwargs['mtime'] = 1 + return gzip.compress(*args, **kwargs) def _bad_compression(body): @@ -141,122 +123,78 @@ def _bad_compression(body): MOCK_COMPRESSION.update(COMPRESSION_MAPPING) -def _assert_compression_body(original_body, compressed_body, encoding): - if hasattr(original_body, 'read'): - original_body = original_body.read() - compressed_body = compressed_body.read() - if isinstance(original_body, dict): - original_body = urlencode(original_body, doseq=True, encoding='utf-8') - if isinstance(original_body, str): - original_body = original_body.encode('utf-8') - decompress = DECOMPRESSION_METHOD_MAP[encoding] - assert original_body == decompress(compressed_body) +def _assert_compression_body(compressed_body, expected_body): + data = compressed_body + if hasattr(compressed_body, 'read'): + data = compressed_body.read() + assert data == expected_body -def _assert_compression_header(headers, encoding): - assert ( - 'Content-Encoding' in headers - and encoding in headers['Content-Encoding'] - ) +def _assert_compression_header(headers, encoding='gzip'): + assert 'Content-Encoding' in headers + assert encoding in headers['Content-Encoding'] -def assert_compression(original_body, request_dict, encoding): - compressed_body = request_dict['body'] - headers = request_dict['headers'] - _assert_compression_body(original_body, compressed_body, encoding) - _assert_compression_header(headers, encoding) +def assert_request_compressed(request_dict, expected_body): + _assert_compression_body(request_dict['body'], expected_body) + _assert_compression_header(request_dict['headers']) @pytest.mark.parametrize( - 'config, request_dict, operation_model, encoding', + 'request_dict, operation_model', [ ( - COMPRESSION_CONFIG_128_BYTES, - default_request_dict(), + _request_dict(), OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - default_request_dict(), + _request_dict(), OP_MULTIPLE_COMPRESSIONS, - 'gzip', ), ( - COMPRESSION_CONFIG_10240_BYTES, - default_request_dict(), + _request_dict(), STREAMING_OP_WITH_COMPRESSION, - 'gzip', - ), - ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_bytearray(), - OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), + _request_dict(bytearray(REQUEST_BODY)), OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_string(), + _request_dict(headers={'Content-Encoding': 'identity'}), OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_bytes_io(), + _request_dict(REQUEST_BODY.decode('utf-8')), OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_128_BYTES, - request_dict_string_io(), + _request_dict(io.BytesIO(REQUEST_BODY)), OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_1_BYTE, - request_dict_dict(), + _request_dict(io.StringIO(REQUEST_BODY.decode('utf-8'))), OP_WITH_COMPRESSION, - 'gzip', ), - ], -) -def test_compression(config, request_dict, operation_model, encoding): - original_body = request_dict['body'] - maybe_compress_request(config, request_dict, operation_model) - assert_compression(original_body, request_dict, encoding) - - -@pytest.mark.parametrize( - 'config, request_dict, operation_model, encoding', - [ ( - COMPRESSION_CONFIG_10240_BYTES, request_dict_non_seekable_bytes_stream(), STREAMING_OP_WITH_COMPRESSION, - 'gzip', ), ( - COMPRESSION_CONFIG_10240_BYTES, request_dict_non_seekable_text_stream(), STREAMING_OP_WITH_COMPRESSION, - 'gzip', ), ], ) -def test_compression_non_seekable_streams( - config, request_dict, operation_model, encoding -): - # since the body can't be reset, we must make a copy - # of the original body to test against - original_body = deepcopy(request_dict['body']) - maybe_compress_request(config, request_dict, operation_model) - assert_compression(original_body, request_dict, encoding) +@mock.patch.object(botocore.compress, 'GzipFile', StaticGzipFile) +@mock.patch.object(botocore.compress, 'gzip_compress', static_compress) +@pytest.mark.skipif( + sys.version_info < (3, 8), reason="requires python3.8 or higher" +) +def test_compression(request_dict, operation_model): + maybe_compress_request( + COMPRESSION_CONFIG_128_BYTES, request_dict, operation_model + ) + assert_request_compressed(request_dict, REQUEST_BODY_COMPRESSED) @pytest.mark.parametrize( @@ -265,9 +203,9 @@ def test_compression_non_seekable_streams( ( Config( disable_request_compression=True, - request_min_compression_size_bytes=1000, + request_min_compression_size_bytes=1, ), - default_request_dict(), + _request_dict(), OP_WITH_COMPRESSION, ), ( @@ -275,128 +213,111 @@ def test_compression_non_seekable_streams( disable_request_compression=False, request_min_compression_size_bytes=256, ), - default_request_dict(), + _request_dict(), OP_WITH_COMPRESSION, ), ( - COMPRESSION_CONFIG_10240_BYTES, - default_request_dict(), + Config( + disable_request_compression=False, + request_min_compression_size_bytes=1, + signature_version='v2', + ), + _request_dict(), + OP_WITH_COMPRESSION, + ), + ( + COMPRESSION_CONFIG_128_BYTES, + _request_dict(), STREAMING_OP_WITH_COMPRESSION_REQUIRES_LENGTH, ), ( - COMPRESSION_CONFIG_10240_BYTES, - default_request_dict(), + COMPRESSION_CONFIG_128_BYTES, + _request_dict(), OP_NO_COMPRESSION, ), ( COMPRESSION_CONFIG_128_BYTES, - default_request_dict(), + _request_dict(), OP_UNKNOWN_COMPRESSION, ), - ( - COMPRESSION_CONFIG_10240_BYTES, - request_dict_string(), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_10240_BYTES, - request_dict_bytearray(), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_10240_BYTES, - request_dict_bytes_io(), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_10240_BYTES, - request_dict_string_io(), - OP_WITH_COMPRESSION, - ), ( COMPRESSION_CONFIG_128_BYTES, - request_dict_with_content_encoding_header(), + _request_dict(headers={'Content-Encoding': 'identity'}), OP_UNKNOWN_COMPRESSION, ), ( - COMPRESSION_CONFIG_1_BYTE, + COMPRESSION_CONFIG_128_BYTES, request_dict_non_seekable_bytes_stream(), OP_WITH_COMPRESSION, ), - ( - COMPRESSION_CONFIG_1_BYTE, - request_dict_non_seekable_text_stream(), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(1), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(1.0), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(object()), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(True), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(None), - OP_WITH_COMPRESSION, - ), - ( - COMPRESSION_CONFIG_1_BYTE, - _request_dict(set()), - OP_WITH_COMPRESSION, - ), ], ) def test_no_compression(config, request_dict, operation_model): ce_header = request_dict['headers'].get('Content-Encoding') original_body = request_dict['body'] maybe_compress_request(config, request_dict, operation_model) - assert request_dict['body'] is original_body - assert ce_header is request_dict['headers'].get('Content-Encoding') + assert request_dict['body'] == original_body + assert ce_header == request_dict['headers'].get('Content-Encoding') -def test_dict_no_compression(): - request_dict = request_dict_dict() - original_body = request_dict['body'] +@pytest.mark.parametrize( + 'operation_model, expected_body', + [ + ( + OP_WITH_COMPRESSION, + ( + b'\x1f\x8b\x08\x00\x01\x00\x00\x00\x02\xffK\xcb' + b'\xcf\xb7MJ,\x02\x00v\x8e5\x1c\x07\x00\x00\x00' + ), + ), + (OP_NO_COMPRESSION, {'foo': 'bar'}), + ], +) +@mock.patch.object(botocore.compress, 'gzip_compress', static_compress) +@pytest.mark.skipif( + sys.version_info < (3, 8), reason="requires python3.8 or higher" +) +def test_dict_compression(operation_model, expected_body): + request_dict = _request_dict({'foo': 'bar'}) maybe_compress_request( - COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION + COMPRESSION_CONFIG_1_BYTE, request_dict, operation_model ) body = request_dict['body'] - encoded_body = urlencode(original_body, doseq=True, encoding='utf-8') - assert body == encoded_body.encode('utf-8') + assert body == expected_body -@pytest.mark.parametrize( - 'request_dict', - [request_dict_string_io(), request_dict_bytes_io()], -) -def test_body_streams_position_reset(request_dict): +@pytest.mark.parametrize('body', [1, object(), True, 1.0]) +def test_maybe_compress_bad_types(body): + request_dict = _request_dict(body) + maybe_compress_request( + COMPRESSION_CONFIG_1_BYTE, request_dict, OP_WITH_COMPRESSION + ) + assert request_dict['body'] == body + + +@mock.patch.object(botocore.compress, 'GzipFile', StaticGzipFile) +def test_body_streams_position_reset(): + request_dict = _request_dict(io.BytesIO(REQUEST_BODY)) maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION, ) assert request_dict['body'].tell() == 0 + assert 'Content-Encoding' in request_dict['headers'] + assert request_dict['body'].read() == REQUEST_BODY_COMPRESSED +@mock.patch.object(botocore.compress, 'gzip_compress', static_compress) +@mock.patch.object(botocore.compress, 'COMPRESSION_MAPPING', MOCK_COMPRESSION) +@pytest.mark.skipif( + sys.version_info < (3, 8), reason="requires python3.8 or higher" +) def test_only_compress_once(): - with mock.patch('botocore.compress.COMPRESSION_MAPPING', MOCK_COMPRESSION): - request_dict = default_request_dict() - body = request_dict['body'] - maybe_compress_request( - COMPRESSION_CONFIG_128_BYTES, request_dict, OP_WITH_COMPRESSION - ) - assert_compression(body, request_dict, 'gzip') + request_dict = _request_dict() + maybe_compress_request( + COMPRESSION_CONFIG_128_BYTES, + request_dict, + OP_WITH_COMPRESSION, + ) + assert_request_compressed(request_dict, REQUEST_BODY_COMPRESSED) From d16faf6d63eacbe79c62de2d839a91b569ebdb2b Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Thu, 27 Jul 2023 14:03:00 -0600 Subject: [PATCH 35/37] Fix incorret operation model --- tests/unit/test_compress.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 480421c5ac..4c879b135a 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -318,6 +318,6 @@ def test_only_compress_once(): maybe_compress_request( COMPRESSION_CONFIG_128_BYTES, request_dict, - OP_WITH_COMPRESSION, + OP_MULTIPLE_COMPRESSIONS, ) assert_request_compressed(request_dict, REQUEST_BODY_COMPRESSED) From 3be8638c12bf1dc8e16a281b198f00dc43cc6881 Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 27 Jul 2023 16:30:00 -0400 Subject: [PATCH 36/37] use compression assertion function in stream test and only use single quotes --- tests/unit/test_compress.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_compress.py b/tests/unit/test_compress.py index 4c879b135a..c149d7c6bc 100644 --- a/tests/unit/test_compress.py +++ b/tests/unit/test_compress.py @@ -188,7 +188,7 @@ def assert_request_compressed(request_dict, expected_body): @mock.patch.object(botocore.compress, 'GzipFile', StaticGzipFile) @mock.patch.object(botocore.compress, 'gzip_compress', static_compress) @pytest.mark.skipif( - sys.version_info < (3, 8), reason="requires python3.8 or higher" + sys.version_info < (3, 8), reason='requires python3.8 or higher' ) def test_compression(request_dict, operation_model): maybe_compress_request( @@ -275,7 +275,7 @@ def test_no_compression(config, request_dict, operation_model): ) @mock.patch.object(botocore.compress, 'gzip_compress', static_compress) @pytest.mark.skipif( - sys.version_info < (3, 8), reason="requires python3.8 or higher" + sys.version_info < (3, 8), reason='requires python3.8 or higher' ) def test_dict_compression(operation_model, expected_body): request_dict = _request_dict({'foo': 'bar'}) @@ -304,14 +304,13 @@ def test_body_streams_position_reset(): OP_WITH_COMPRESSION, ) assert request_dict['body'].tell() == 0 - assert 'Content-Encoding' in request_dict['headers'] - assert request_dict['body'].read() == REQUEST_BODY_COMPRESSED + assert_request_compressed(request_dict, REQUEST_BODY_COMPRESSED) @mock.patch.object(botocore.compress, 'gzip_compress', static_compress) @mock.patch.object(botocore.compress, 'COMPRESSION_MAPPING', MOCK_COMPRESSION) @pytest.mark.skipif( - sys.version_info < (3, 8), reason="requires python3.8 or higher" + sys.version_info < (3, 8), reason='requires python3.8 or higher' ) def test_only_compress_once(): request_dict = _request_dict() From 494c7e3a0d262ce472f0fb346a4c2892f8dccb32 Mon Sep 17 00:00:00 2001 From: davidlm Date: Thu, 27 Jul 2023 18:50:26 -0400 Subject: [PATCH 37/37] small fix to functional test and changelog --- .changes/next-release/enhancement-compression-36791.json | 5 +++++ tests/functional/test_compress.py | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 .changes/next-release/enhancement-compression-36791.json diff --git a/.changes/next-release/enhancement-compression-36791.json b/.changes/next-release/enhancement-compression-36791.json new file mode 100644 index 0000000000..ea56ede0e1 --- /dev/null +++ b/.changes/next-release/enhancement-compression-36791.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "compression", + "description": "Adds support for the ``requestcompression`` operation trait." +} diff --git a/tests/functional/test_compress.py b/tests/functional/test_compress.py index 53ed482b21..63f1510b13 100644 --- a/tests/functional/test_compress.py +++ b/tests/functional/test_compress.py @@ -120,8 +120,10 @@ def test_compression(patched_session, monkeypatch): param_template = ( "MockOpParamList.member.{i}.MockOpParam=MockOpParamValue{i}" ) - serialized_body = "&".join( + serialized_params = "&".join( param_template.format(i=i) for i in range(1, 21) ) + additional_params = "Action=MockOperation&Version=2020-02-02" + serialized_body = f"{additional_params}&{serialized_params}" actual_body = gzip.decompress(http_stubber.requests[0].body) - assert serialized_body.encode('utf-8') in actual_body + assert serialized_body.encode('utf-8') == actual_body