From 3d56b65c898d7997819f2627a2fb722bd0c33b69 Mon Sep 17 00:00:00 2001
From: Pete Zaitcev <zaitcev@kotori.zaitcev.us>
Date: Thu, 14 Aug 2014 13:57:53 -0600
Subject: [PATCH] Fix crash when downloading a pseudo-directory
If a user creates an object with name ending with a slash, then
downloading such container ends in a traceback like this:
..............
test5g.file [auth 1.516s, headers 1.560s, total 244.565s, 22.089 MB/s]
Traceback (most recent call last):
File "/usr/lib/python2.6/site-packages/swiftclient/multithreading.py", lin
result = self.func(item, *self.args, **self.kwargs)
File "/usr/lib/python2.6/site-packages/swiftclient/shell.py", line 403, in
fp = open(path, 'wb')
IOError: [Errno 21] Is a directory: 'first-pseudo-folder/'
The proposed fix is not to save this object. Note that the contents of
the object are available with --output option, as before. Only the
crash is fixed.
Even though we do not use the contents, we download the object and
check its Etag, in case. We also create a corresponding directory,
in case the pseudo-directory contains no objects.
The format of printout is changed, so user realizes easier when
pseudo-directory convention is in effect. Note that this is not a
compatibility issue because previously there was crash in such case.
Change-Id: I3352f7a4eaf9970961af0cc84c4706fc1eab281d
---
swiftclient/shell.py | 34 ++++++++++++++++++++++------------
tests/unit/test_shell.py | 11 ++++++++---
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/swiftclient/shell.py b/swiftclient/shell.py
index ef153c71..f967009c 100755
--- a/swiftclient/shell.py
+++ b/swiftclient/shell.py
@@ -24,7 +24,7 @@ from errno import EEXIST, ENOENT
from hashlib import md5
from optparse import OptionParser, 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
@@ -381,7 +381,9 @@ def st_download(parser, args, thread_manager):
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)
@@ -397,24 +399,28 @@ def st_download(parser, args, thread_manager):
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',
@@ -424,8 +430,7 @@ def st_download(parser, args, thread_manager):
'%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:
@@ -434,10 +439,15 @@ def st_download(parser, args, thread_manager):
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)
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index 83113f0c..f4a569cf 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -183,6 +183,7 @@ class TestShell(unittest.TestCase):
# 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
@@ -191,9 +192,13 @@ class TestShell(unittest.TestCase):
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: