Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan343 committed Oct 22, 2024
1 parent 17ef6f7 commit 0dd1f6d
Show file tree
Hide file tree
Showing 5 changed files with 256 additions and 32 deletions.
14 changes: 0 additions & 14 deletions botocore/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
ParamValidationError,
UnsupportedTLSVersionWarning,
)
from botocore.httpchecksum import DEFAULT_CHECKSUM_ALGORITHM
from botocore.regions import EndpointResolverBuiltins
from botocore.signers import (
add_generate_db_auth_token,
Expand Down Expand Up @@ -1270,15 +1269,6 @@ def _update_status_code(response, **kwargs):
http_response.status_code = parsed_status_code


def set_default_multipart_checksum_algorithm(params, **kwargs):
"""Sets the ``ChecksumAlgorithm`` parameter to the SDKs default checksum.
The ``CreateMultipartUpload`` operation isn't modeled with the ``httpchecksum``
trait and requires us to set a default checksum algorithm when none is provided.
"""
params.setdefault('ChecksumAlgorithm', DEFAULT_CHECKSUM_ALGORITHM)


# This is a list of (event_name, handler).
# When a Session is created, everything in this list will be
# automatically registered with that Session.
Expand Down Expand Up @@ -1333,10 +1323,6 @@ def set_default_multipart_checksum_algorithm(params, **kwargs):
'before-parameter-build.s3.CreateMultipartUpload',
validate_ascii_metadata,
),
(
'before-parameter-build.s3.CreateMultipartUpload',
set_default_multipart_checksum_algorithm,
),
('before-parameter-build.s3-control', remove_accid_host_prefix_from_model),
('docs.*.s3.CopyObject.complete-section', document_copy_source_form),
('docs.*.s3.UploadPartCopy.complete-section', document_copy_source_form),
Expand Down
6 changes: 3 additions & 3 deletions botocore/httpchecksum.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from binascii import crc32
from hashlib import sha1, sha256

from botocore.compat import HAS_CRT
from botocore.compat import HAS_CRT, urlparse
from botocore.exceptions import (
AwsChunkedWrapperError,
FlexibleChecksumError,
Expand Down Expand Up @@ -293,14 +293,14 @@ def resolve_request_checksum_algorithm(
elif request_checksum_required or (
algorithm_member and request_checksum_calculation == "when_supported"
):
algorithm_name = "crc32"
algorithm_name = DEFAULT_CHECKSUM_ALGORITHM.lower()
else:
return

location_type = "header"
if operation_model.has_streaming_input:
# Operations with streaming input must support trailers.
if request["url"].startswith("https:"):
if urlparse(request["url"]).scheme == "https":
# We only support unsigned trailer checksums currently. As this
# disables payload signing we'll only use trailers over TLS.
location_type = "trailer"
Expand Down
86 changes: 86 additions & 0 deletions botocore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 base64
import binascii
import datetime
import email.message
Expand Down Expand Up @@ -49,8 +50,10 @@
HAS_CRT,
IPV4_RE,
IPV6_ADDRZ_RE,
MD5_AVAILABLE,
UNSAFE_URL_CHARS,
OrderedDict,
get_md5,
get_tzinfo_options,
json,
quote,
Expand Down Expand Up @@ -3220,6 +3223,38 @@ def get_encoding_from_headers(headers, default='ISO-8859-1'):
return default


def calculate_md5(body, **kwargs):
"""This function has been deprecated, but is kept for backwards compatibility."""
if isinstance(body, (bytes, bytearray)):
binary_md5 = _calculate_md5_from_bytes(body)
else:
binary_md5 = _calculate_md5_from_file(body)
return base64.b64encode(binary_md5).decode('ascii')


def _calculate_md5_from_bytes(body_bytes):
"""This function has been deprecated, but is kept for backwards compatibility."""
md5 = get_md5(body_bytes)
return md5.digest()


def _calculate_md5_from_file(fileobj):
"""This function has been deprecated, but is kept for backwards compatibility."""
start_position = fileobj.tell()
md5 = get_md5()
for chunk in iter(lambda: fileobj.read(1024 * 1024), b''):
md5.update(chunk)
fileobj.seek(start_position)
return md5.digest()


def _is_s3express_request(params):
endpoint_properties = params.get('context', {}).get(
'endpoint_properties', {}
)
return endpoint_properties.get('backend') == 'S3Express'


def _has_checksum_header(params):
headers = params['headers']

Expand All @@ -3232,6 +3267,57 @@ def _has_checksum_header(params):
return False


def conditionally_calculate_checksum(params, **kwargs):
"""This function has been deprecated, but is kept for backwards compatibility."""
if not _has_checksum_header(params):
conditionally_calculate_md5(params, **kwargs)
conditionally_enable_crc32(params, **kwargs)


def conditionally_enable_crc32(params, **kwargs):
"""This function has been deprecated, but is kept for backwards compatibility."""
checksum_context = params.get('context', {}).get('checksum', {})
checksum_algorithm = checksum_context.get('request_algorithm')
if (
_is_s3express_request(params)
and params['body'] is not None
and checksum_algorithm in (None, "conditional-md5")
):
params['context']['checksum'] = {
'request_algorithm': {
'algorithm': 'crc32',
'in': 'header',
'name': 'x-amz-checksum-crc32',
}
}


def conditionally_calculate_md5(params, **kwargs):
"""
This function has been deprecated, but is kept for backwards compatibility.
Only add a Content-MD5 if the system supports it.
"""
body = params['body']
checksum_context = params.get('context', {}).get('checksum', {})
checksum_algorithm = checksum_context.get('request_algorithm')
if checksum_algorithm and checksum_algorithm != 'conditional-md5':
# Skip for requests that will have a flexible checksum applied
return

if _has_checksum_header(params):
# Don't add a new header if one is already available.
return

if _is_s3express_request(params):
# S3Express doesn't support MD5
return

if MD5_AVAILABLE and body is not None:
md5_digest = calculate_md5(body, **kwargs)
params['headers']['Content-MD5'] = md5_digest


class FileWebIdentityTokenLoader:
def __init__(self, web_identity_token_path, _open=open):
self._web_identity_token_path = web_identity_token_path
Expand Down
13 changes: 8 additions & 5 deletions tests/functional/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
UnsupportedS3AccesspointConfigurationError,
UnsupportedS3ConfigurationError,
)
from botocore.httpchecksum import _CHECKSUM_CLS
from botocore.httpchecksum import HAS_CRT, Crc32Checksum, CrtCrc32Checksum
from tests import (
BaseSessionTest,
ClientHTTPStubber,
Expand Down Expand Up @@ -1533,6 +1533,8 @@ def test_trailing_checksum_set(self):
sent_headers["x-amz-content-sha256"],
b"STREAMING-UNSIGNED-PAYLOAD-TRAILER",
)
body = self.http_stubber.requests[0].body.read()
self.assertIn(b"x-amz-checksum-crc32", body)

def test_trailing_checksum_set_empty_body(self):
with self.http_stubber:
Expand All @@ -1548,12 +1550,15 @@ def test_trailing_checksum_set_empty_body(self):
sent_headers["x-amz-content-sha256"],
b"STREAMING-UNSIGNED-PAYLOAD-TRAILER",
)
body = self.http_stubber.requests[0].body.read()
self.assertIn(b"x-amz-checksum-crc32", body)

def test_trailing_checksum_set_empty_file(self):
with self.http_stubber:
with temporary_file("rb") as f:
assert f.read() == b""
self.client.put_object(Bucket="foo", Key="bar", Body=f)
body = self.http_stubber.requests[0].body.read()
sent_headers = self.get_sent_headers()
self.assertEqual(sent_headers["Content-Encoding"], b"aws-chunked")
self.assertEqual(sent_headers["Transfer-Encoding"], b"chunked")
Expand All @@ -1565,6 +1570,7 @@ def test_trailing_checksum_set_empty_file(self):
sent_headers["x-amz-content-sha256"],
b"STREAMING-UNSIGNED-PAYLOAD-TRAILER",
)
self.assertIn(b"x-amz-checksum-crc32", body)

def test_content_sha256_not_set_if_config_value_is_true(self):
# By default, put_object() provides a trailing checksum and includes the
Expand All @@ -1584,7 +1590,6 @@ def test_content_sha256_not_set_if_config_value_is_true(self):
self.client.put_object(Bucket="foo", Key="bar", Body="baz")
sent_headers = self.get_sent_headers()
sha_header = sent_headers.get("x-amz-content-sha256")
self.assertIsNotNone(sha_header)
self.assertEqual(sha_header, b"STREAMING-UNSIGNED-PAYLOAD-TRAILER")

def test_content_sha256_not_set_if_config_value_is_false(self):
Expand All @@ -1605,7 +1610,6 @@ def test_content_sha256_not_set_if_config_value_is_false(self):
self.client.put_object(Bucket="foo", Key="bar", Body="baz")
sent_headers = self.get_sent_headers()
sha_header = sent_headers.get("x-amz-content-sha256")
self.assertIsNotNone(sha_header)
self.assertEqual(sha_header, b"STREAMING-UNSIGNED-PAYLOAD-TRAILER")

def test_content_sha256_not_set_if_config_value_not_set_put_object(self):
Expand All @@ -1625,7 +1629,6 @@ def test_content_sha256_not_set_if_config_value_not_set_put_object(self):
self.client.put_object(Bucket="foo", Key="bar", Body="baz")
sent_headers = self.get_sent_headers()
sha_header = sent_headers.get("x-amz-content-sha256")
self.assertIsNotNone(sha_header)
self.assertEqual(sha_header, b"STREAMING-UNSIGNED-PAYLOAD-TRAILER")

def test_content_sha256_set_if_config_value_not_set_list_objects(self):
Expand Down Expand Up @@ -3735,7 +3738,7 @@ def _verify_presigned_url_addressing(

class TestS3XMLPayloadEscape(BaseS3OperationTest):
def assert_correct_crc32_checksum(self, request):
checksum = _CHECKSUM_CLS.get("crc32")()
checksum = CrtCrc32Checksum() if HAS_CRT else Crc32Checksum()
crc32_checksum = checksum.handle(request.body).encode()
self.assertEqual(
crc32_checksum, request.headers["x-amz-checksum-crc32"]
Expand Down
Loading

0 comments on commit 0dd1f6d

Please sign in to comment.