Skip to content

Commit

Permalink
Merge "Fix crash when downloading a pseudo-directory"
Browse files Browse the repository at this point in the history
  • Loading branch information
Jenkins authored and openstack-gerrit committed Aug 17, 2014
2 parents 23474b7 + 3d56b65 commit eedb0d4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
34 changes: 22 additions & 12 deletions swiftclient/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from hashlib import md5
from optparse import OptionParser, OptionGroup, SUPPRESS_HELP
from os import environ, listdir, makedirs, utime, _exit as os_exit
from os.path import dirname, getmtime, getsize, isdir, join, \
from os.path import basename, dirname, getmtime, getsize, isdir, join, \
sep as os_path_sep
from random import shuffle
from sys import argv as sys_argv, exit, stderr, stdout
Expand Down Expand Up @@ -381,7 +381,9 @@ def _download_object(queue_arg, conn):
content_length = None
etag = headers.get('etag')
md5sum = None
make_dir = not options.no_download and out_file != "-"
pseudodir = False
no_file = options.no_download
make_dir = not no_file and out_file != "-"
if content_type.split(';', 1)[0] == 'text/directory':
if make_dir and not isdir(path):
mkdirs(path)
Expand All @@ -397,24 +399,28 @@ def _download_object(queue_arg, conn):
dirpath = dirname(path)
if make_dir and dirpath and not isdir(dirpath):
mkdirs(dirpath)
if not options.no_download:
if not no_file:
if out_file == "-":
fp = stdout
elif out_file:
fp = open(out_file, 'wb')
else:
fp = open(path, 'wb')
if basename(path):
fp = open(path, 'wb')
else:
pseudodir = True
no_file = True
read_length = 0
if 'x-object-manifest' not in headers and \
'x-static-large-object' not in headers:
md5sum = md5()
for chunk in body:
if not options.no_download:
if not no_file:
fp.write(chunk)
read_length += len(chunk)
if md5sum:
md5sum.update(chunk)
if not options.no_download:
if not no_file:
fp.close()
if md5sum and md5sum.hexdigest() != etag:
thread_manager.error('%s: md5sum != etag, %s != %s',
Expand All @@ -424,8 +430,7 @@ def _download_object(queue_arg, conn):
'%s: read_length != content_length, %d != %d',
path, read_length, content_length)
if 'x-object-meta-mtime' in headers and not options.out_file \
and not options.no_download:

and not no_file:
mtime = float(headers['x-object-meta-mtime'])
utime(path, (mtime, mtime))
if options.verbose:
Expand All @@ -434,10 +439,15 @@ def _download_object(queue_arg, conn):
headers_receipt = headers_receipt - start_time
total_time = finish_time - start_time
download_time = total_time - auth_time
time_str = ('auth %.3fs, headers %.3fs, total %.3fs, '
'%.3f MB/s' % (
auth_time, headers_receipt, total_time,
float(read_length) / download_time / 1000000))
if pseudodir:
time_str = (
'auth %.3fs, headers %.3fs, total %.3fs, pseudo' % (
auth_time, headers_receipt, total_time))
else:
time_str = (
'auth %.3fs, headers %.3fs, total %.3fs, %.3f MB/s' % (
auth_time, headers_receipt, total_time,
float(read_length) / download_time / 1000000))
if conn.attempts > 1:
thread_manager.print_msg('%s [%s after %d attempts]', path,
time_str, conn.attempts)
Expand Down
11 changes: 8 additions & 3 deletions tests/unit/test_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def test_download(self, connection):
# Test downloading whole container
connection.return_value.get_container.side_effect = [
[None, [{'name': 'object'}]],
[None, [{'name': 'pseudo/'}]],
[None, []],
]
connection.return_value.auth_end_time = 0
Expand All @@ -191,9 +192,13 @@ def test_download(self, connection):
with mock.patch(BUILTIN_OPEN) as mock_open:
argv = ["", "download", "container"]
swiftclient.shell.main(argv)
connection.return_value.get_object.assert_called_with(
'container', 'object', headers={}, resp_chunk_size=65536)
mock_open.assert_called_with('object', 'wb')
calls = [mock.call('container', 'object',
headers={}, resp_chunk_size=65536),
mock.call('container', 'pseudo/',
headers={}, resp_chunk_size=65536)]
connection.return_value.get_object.assert_has_calls(
calls, any_order=True)
mock_open.assert_called_once_with('object', 'wb')

# Test downloading single object
with mock.patch(BUILTIN_OPEN) as mock_open:
Expand Down

0 comments on commit eedb0d4

Please sign in to comment.