Skip to content

Commit 90a897b

Browse files
Merge branch 'release-0.13.0'
* release-0.13.0: Bumping version to 0.13.0 Update change type to feature change condition Validate multipart downloads using object ETags Revert "Validate multipart downloads using object ETags" Validate multipart downloads using object ETags
2 parents e1415f8 + e94b39b commit 90a897b

9 files changed

Lines changed: 162 additions & 16 deletions

File tree

.changes/0.13.0.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[
2+
{
3+
"category": "``GetObjectTask``",
4+
"description": "Validate ETag of stored object during multipart downloads",
5+
"type": "feature"
6+
}
7+
]

CHANGELOG.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
CHANGELOG
33
=========
44

5+
0.13.0
6+
======
7+
8+
* feature:``GetObjectTask``: Validate ETag of stored object during multipart downloads
9+
10+
511
0.12.0
612
======
713

s3transfer/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def __call__(self, bytes_amount):
145145
from s3transfer.exceptions import RetriesExceededError, S3UploadFailedError
146146

147147
__author__ = 'Amazon Web Services'
148-
__version__ = '0.12.0'
148+
__version__ = '0.13.0'
149149

150150

151151
class NullHandler(logging.Handler):

s3transfer/download.py

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
import logging
1515
import threading
1616

17+
from botocore.exceptions import ClientError
18+
1719
from s3transfer.compat import seekable
18-
from s3transfer.exceptions import RetriesExceededError
20+
from s3transfer.exceptions import RetriesExceededError, S3DownloadFailedError
1921
from s3transfer.futures import IN_MEMORY_DOWNLOAD_TAG
2022
from s3transfer.tasks import SubmissionTask, Task
2123
from s3transfer.utils import (
@@ -346,17 +348,23 @@ def _submit(
346348
:param bandwidth_limiter: The bandwidth limiter to use when
347349
downloading streams
348350
"""
349-
if transfer_future.meta.size is None:
350-
# If a size was not provided figure out the size for the
351-
# user.
351+
if (
352+
transfer_future.meta.size is None
353+
or transfer_future.meta.etag is None
354+
):
352355
response = client.head_object(
353356
Bucket=transfer_future.meta.call_args.bucket,
354357
Key=transfer_future.meta.call_args.key,
355358
**transfer_future.meta.call_args.extra_args,
356359
)
360+
# If a size was not provided figure out the size for the
361+
# user.
357362
transfer_future.meta.provide_transfer_size(
358363
response['ContentLength']
359364
)
365+
# Provide an etag to ensure a stored object is not modified
366+
# during a multipart download.
367+
transfer_future.meta.provide_object_etag(response.get('ETag'))
360368

361369
download_output_manager = self._get_download_output_manager_cls(
362370
transfer_future, osutil
@@ -479,9 +487,12 @@ def _submit_ranged_download_request(
479487
part_size, i, num_parts
480488
)
481489

482-
# Inject the Range parameter to the parameters to be passed in
483-
# as extra args
484-
extra_args = {'Range': range_parameter}
490+
# Inject extra parameters to be passed in as extra args
491+
extra_args = {
492+
'Range': range_parameter,
493+
}
494+
if transfer_future.meta.etag is not None:
495+
extra_args['IfMatch'] = transfer_future.meta.etag
485496
extra_args.update(call_args.extra_args)
486497
finalize_download_invoker.increment()
487498
# Submit the ranged downloads
@@ -593,6 +604,15 @@ def _main(
593604
else:
594605
return
595606
return
607+
except ClientError as e:
608+
error_code = e.response.get('Error', {}).get('Code')
609+
if error_code == "PreconditionFailed":
610+
raise S3DownloadFailedError(
611+
f'Contents of stored object "{key}" in bucket '
612+
f'"{bucket}" did not match expected ETag.'
613+
)
614+
else:
615+
raise
596616
except S3_RETRYABLE_DOWNLOAD_ERRORS as e:
597617
logger.debug(
598618
"Retrying exception caught (%s), "

s3transfer/exceptions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ class S3UploadFailedError(Exception):
2323
pass
2424

2525

26+
class S3DownloadFailedError(Exception):
27+
pass
28+
29+
2630
class InvalidSubscriberMethodError(Exception):
2731
pass
2832

s3transfer/futures.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ def __init__(self, call_args=None, transfer_id=None):
134134
self._transfer_id = transfer_id
135135
self._size = None
136136
self._user_context = {}
137+
self._etag = None
137138

138139
@property
139140
def call_args(self):
@@ -155,6 +156,11 @@ def user_context(self):
155156
"""A dictionary that requesters can store data in"""
156157
return self._user_context
157158

159+
@property
160+
def etag(self):
161+
"""The etag of the stored object for validating multipart downloads"""
162+
return self._etag
163+
158164
def provide_transfer_size(self, size):
159165
"""A method to provide the size of a transfer request
160166
@@ -164,6 +170,15 @@ def provide_transfer_size(self, size):
164170
"""
165171
self._size = size
166172

173+
def provide_object_etag(self, etag):
174+
"""A method to provide the etag of a transfer request
175+
176+
By providing this value, the TransferManager will validate
177+
multipart downloads by supplying an IfMatch parameter with
178+
the etag as the value to GetObject requests.
179+
"""
180+
self._etag = etag
181+
167182

168183
class TransferCoordinator:
169184
"""A helper class for managing TransferFuture"""

tests/__init__.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ def on_queued(self, future, **kwargs):
155155
future.meta.provide_transfer_size(self.file_size)
156156

157157

158+
class ETagProvider:
159+
def __init__(self, etag):
160+
self.etag = etag
161+
162+
def on_queued(self, future, **kwargs):
163+
future.meta.provide_object_etag(self.etag)
164+
165+
158166
class FileCreator:
159167
def __init__(self):
160168
self.rootdir = tempfile.mkdtemp()

tests/functional/test_download.py

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@
2121
from botocore.exceptions import ClientError
2222

2323
from s3transfer.compat import SOCKET_ERROR
24-
from s3transfer.exceptions import RetriesExceededError
24+
from s3transfer.exceptions import RetriesExceededError, S3DownloadFailedError
2525
from s3transfer.manager import TransferConfig, TransferManager
2626
from tests import (
2727
BaseGeneralInterfaceTest,
28+
ETagProvider,
2829
FileSizeProvider,
2930
NonSeekableWriter,
3031
RecordingOSUtils,
@@ -48,6 +49,7 @@ def setUp(self):
4849
# Initialize some default arguments
4950
self.bucket = 'mybucket'
5051
self.key = 'mykey'
52+
self.etag = 'myetag'
5153
self.extra_args = {}
5254
self.subscribers = []
5355

@@ -84,7 +86,10 @@ def create_stubbed_responses(self):
8486
return [
8587
{
8688
'method': 'head_object',
87-
'service_response': {'ContentLength': len(self.content)},
89+
'service_response': {
90+
'ContentLength': len(self.content),
91+
'ETag': self.etag,
92+
},
8893
},
8994
{
9095
'method': 'get_object',
@@ -290,11 +295,14 @@ def test_retry_rewinds_callbacks(self):
290295
]
291296
self.assertEqual(-3, progress_byte_amts[1])
292297

293-
def test_can_provide_file_size(self):
298+
def test_can_provide_file_size_and_etag(self):
294299
self.add_successful_get_object_responses()
295300

296301
call_kwargs = self.create_call_kwargs()
297-
call_kwargs['subscribers'] = [FileSizeProvider(len(self.content))]
302+
call_kwargs['subscribers'] = [
303+
FileSizeProvider(len(self.content)),
304+
ETagProvider(self.etag),
305+
]
298306

299307
future = self.manager.download(**call_kwargs)
300308
future.result()
@@ -469,7 +477,10 @@ def create_stubbed_responses(self):
469477
return [
470478
{
471479
'method': 'head_object',
472-
'service_response': {'ContentLength': len(self.content)},
480+
'service_response': {
481+
'ContentLength': len(self.content),
482+
'ETag': self.etag,
483+
},
473484
},
474485
{
475486
'method': 'get_object',
@@ -502,7 +513,7 @@ def test_download(self):
502513
expected_ranges = ['bytes=0-3', 'bytes=4-7', 'bytes=8-']
503514
self.add_head_object_response(expected_params)
504515
self.add_successful_get_object_responses(
505-
expected_params, expected_ranges
516+
{**expected_params, 'IfMatch': self.etag}, expected_ranges
506517
)
507518

508519
future = self.manager.download(
@@ -523,6 +534,76 @@ def test_download_with_checksum_enabled(self):
523534
}
524535
expected_ranges = ['bytes=0-3', 'bytes=4-7', 'bytes=8-']
525536
self.add_head_object_response(expected_params)
537+
self.add_successful_get_object_responses(
538+
{**expected_params, 'IfMatch': self.etag}, expected_ranges
539+
)
540+
541+
future = self.manager.download(
542+
self.bucket, self.key, self.filename, self.extra_args
543+
)
544+
future.result()
545+
546+
# Ensure that the contents are correct
547+
with open(self.filename, 'rb') as f:
548+
self.assertEqual(self.content, f.read())
549+
550+
def test_download_raises_if_etag_validation_fails(self):
551+
expected_params = {
552+
'Bucket': self.bucket,
553+
'Key': self.key,
554+
}
555+
expected_ranges = ['bytes=0-3', 'bytes=4-7']
556+
self.add_head_object_response(expected_params)
557+
558+
# Add successful GetObject responses for the first 2 requests.
559+
for i, stubbed_response in enumerate(
560+
self.create_stubbed_responses()[1:3]
561+
):
562+
stubbed_response['expected_params'] = copy.deepcopy(
563+
{**expected_params, 'IfMatch': self.etag}
564+
)
565+
stubbed_response['expected_params']['Range'] = expected_ranges[i]
566+
self.stubber.add_response(**stubbed_response)
567+
568+
# Simulate ETag validation failure by adding a
569+
# client error for the last GetObject request.
570+
self.stubber.add_client_error(
571+
method='get_object',
572+
service_error_code='PreconditionFailed',
573+
service_message=(
574+
'At least one of the pre-conditions you specified did not hold'
575+
),
576+
http_status_code=412,
577+
)
578+
579+
future = self.manager.download(
580+
self.bucket, self.key, self.filename, self.extra_args
581+
)
582+
with self.assertRaises(S3DownloadFailedError) as e:
583+
future.result()
584+
self.assertIn('did not match expected ETag', str(e.exception))
585+
586+
# Ensure no data is written to disk.
587+
self.assertFalse(os.path.exists(self.filename))
588+
589+
def test_download_without_etag(self):
590+
expected_params = {
591+
'Bucket': self.bucket,
592+
'Key': self.key,
593+
}
594+
expected_ranges = ['bytes=0-3', 'bytes=4-7', 'bytes=8-']
595+
596+
# Stub HeadObject response with no ETag
597+
head_object_response = {
598+
'method': 'head_object',
599+
'service_response': {
600+
'ContentLength': len(self.content),
601+
},
602+
'expected_params': expected_params,
603+
}
604+
self.stubber.add_response(**head_object_response)
605+
606+
# This asserts that IfMatch isn't in the GetObject requests.
526607
self.add_successful_get_object_responses(
527608
expected_params, expected_ranges
528609
)

tests/unit/test_download.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ def setUp(self):
395395

396396
self.bucket = 'mybucket'
397397
self.key = 'mykey'
398-
self.extra_args = {}
398+
self.etag = 'myetag'
399+
self.extra_args = {'IfMatch': self.etag}
399400
self.subscribers = []
400401

401402
# Create a stream to read from
@@ -452,7 +453,11 @@ def assert_tag_for_get_object(self, tag_value):
452453

453454
def add_head_object_response(self):
454455
self.stubber.add_response(
455-
'head_object', {'ContentLength': len(self.content)}
456+
'head_object',
457+
{
458+
'ContentLength': len(self.content),
459+
'ETag': self.etag,
460+
},
456461
)
457462

458463
def add_get_responses(self):

0 commit comments

Comments
 (0)