Skip to content

Commit

Permalink
Fix a bad bounds check for downloads.
Browse files Browse the repository at this point in the history
Previously, if we created a download with `auto_transfer=True` with the full
download smaller than the default `chunksize`, and then called
`StreamInChunks`, we'd end up downloading the full content twice. This was due
to a bad bounds check in setting a range header -- we'd end up with a header
like `Range: bytes=100-99`, and the server would ignore the start value,
giving us all the bytes again.

This adds a fix and a test.
  • Loading branch information
craigcitro committed Aug 22, 2017
1 parent 154baaa commit 9e706f9
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
2 changes: 1 addition & 1 deletion apitools/base/py/transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def __NormalizeStartEnd(self, start, end=None):
def __SetRangeHeader(self, request, start, end=None):
if start < 0:
request.headers['range'] = 'bytes=%d' % start
elif end is None:
elif end is None or end < start:
request.headers['range'] = 'bytes=%d-' % start
else:
request.headers['range'] = 'bytes=%d-%d' % (start, end)
Expand Down
40 changes: 40 additions & 0 deletions apitools/base/py/transfer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,46 @@ def testComputeEndByteSmallTotal(self):
download._Download__ComputeEndByte(start),
msg='Failed on start={0}'.format(start))

def testDownloadThenStream(self):
bytes_http = object()
http = object()
download_stream = six.StringIO()
download = transfer.Download.FromStream(download_stream,
total_size=26)
download.bytes_http = bytes_http
base_url = 'https://part.one/'
with mock.patch.object(http_wrapper, 'MakeRequest',
autospec=True) as make_request:
make_request.return_value = http_wrapper.Response(
info={
'content-range': 'bytes 0-25/26',
'status': http_client.OK,
},
content=string.ascii_lowercase,
request_url=base_url,
)
request = http_wrapper.Request(url='https://part.one/')
download.InitializeDownload(request, http=http)
self.assertEqual(1, make_request.call_count)
received_request = make_request.call_args[0][1]
self.assertEqual(base_url, received_request.url)
self.assertRangeAndContentRangeCompatible(
received_request, make_request.return_value)

with mock.patch.object(http_wrapper, 'MakeRequest',
autospec=True) as make_request:
make_request.return_value = http_wrapper.Response(
info={
'status': http_client.REQUESTED_RANGE_NOT_SATISFIABLE,
},
content='error',
request_url=base_url,
)
download.StreamInChunks()
self.assertEqual(1, make_request.call_count)
received_request = make_request.call_args[0][1]
self.assertEqual('bytes=26-', received_request.headers['range'])

def testGetRange(self):
for (start_byte, end_byte) in [(0, 25), (5, 15), (0, 0), (25, 25)]:
bytes_http = object()
Expand Down

0 comments on commit 9e706f9

Please sign in to comment.