From bb252130ac2f332110172b8e2094dc629c8a896b Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Mon, 18 May 2015 10:14:09 -0700 Subject: Always decode command-line arguments as UTF-8 There was always an implicit assumption that they were UTF-8 before, and by converting them to unicode we close another hole allowing raw bytes to appear in user-facing messages. Closes-Bug: #1431866 Change-Id: If2e41d9a592c3ad02818e9c6f0959fd4b73cd0e0 --- swiftclient/shell.py | 15 ++++++---- tests/unit/test_shell.py | 75 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/swiftclient/shell.py b/swiftclient/shell.py index 4438e9d..0663e4f 100755 --- a/swiftclient/shell.py +++ b/swiftclient/shell.py @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from __future__ import print_function +from __future__ import print_function, unicode_literals import logging import signal @@ -23,6 +23,7 @@ import socket from optparse import OptionParser, OptionGroup, SUPPRESS_HELP from os import environ, walk, _exit as os_exit from os.path import isfile, isdir, join +from six import text_type from sys import argv as sys_argv, exit, stderr from time import gmtime, strftime @@ -106,7 +107,7 @@ def st_delete(parser, args, output_manager): if '/' in container: output_manager.error( 'WARNING: / in container name; you ' - 'might have meant %r instead of %r.' % ( + "might have meant '%s' instead of '%s'." % ( container.replace('/', ' ', 1), container) ) return @@ -249,7 +250,7 @@ def st_download(parser, args, output_manager): if '/' in container: output_manager.error( 'WARNING: / in container name; you ' - 'might have meant %r instead of %r.' % ( + "might have meant '%s' instead of '%s'." % ( container.replace('/', ' ', 1), container) ) return @@ -500,7 +501,7 @@ def st_stat(parser, args, output_manager): if '/' in container: output_manager.error( 'WARNING: / in container name; you might have ' - 'meant %r instead of %r.' % + "meant '%s' instead of '%s'." % (container.replace('/', ' ', 1), container)) return args = args[1:] @@ -604,7 +605,7 @@ def st_post(parser, args, output_manager): if '/' in container: output_manager.error( 'WARNING: / in container name; you might have ' - 'meant %r instead of %r.' % + "meant '%s' instead of '%s'." % (args[0].replace('/', ' ', 1), args[0])) return args = args[1:] @@ -844,7 +845,7 @@ def st_upload(parser, args, output_manager): msg = ': %s' % error output_manager.warning( 'Warning: failed to create container ' - '%r%s', container, msg + "'%s'%s", container, msg ) else: output_manager.error("%s" % error) @@ -1037,6 +1038,8 @@ def main(arguments=None): else: argv = sys_argv + argv = [a if isinstance(a, text_type) else a.decode('utf-8') for a in argv] + version = client_version parser = OptionParser(version='python-swiftclient %s' % version, usage=''' diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py index 117b66b..9f97990 100644 --- a/tests/unit/test_shell.py +++ b/tests/unit/test_shell.py @@ -12,6 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import unicode_literals from genericpath import getmtime import hashlib @@ -650,6 +651,38 @@ class TestShell(unittest.TestCase): connection.return_value.delete_object.assert_called_with( 'container', 'object', query_string=None, response_dict={}) + def test_delete_verbose_output_utf8(self): + container = 't\u00e9st_c' + base_argv = ['', '--verbose', 'delete'] + + # simulate container having an object with utf-8 code points in name, + # just returning the object delete result + res = {'success': True, 'response_dict': {}, 'attempts': 2, + 'container': container, 'action': 'delete_object', + 'object': 'obj_t\u00east_o'} + + with mock.patch('swiftclient.shell.SwiftService.delete') as mock_func: + with CaptureOutput() as out: + mock_func.return_value = [res] + swiftclient.shell.main(base_argv + [container.encode('utf-8')]) + + mock_func.assert_called_once_with(container=container) + self.assertTrue(out.out.find( + 'obj_t\u00east_o [after 2 attempts]') >= 0, out) + + # simulate empty container + res = {'success': True, 'response_dict': {}, 'attempts': 2, + 'container': container, 'action': 'delete_container'} + + with mock.patch('swiftclient.shell.SwiftService.delete') as mock_func: + with CaptureOutput() as out: + mock_func.return_value = [res] + swiftclient.shell.main(base_argv + [container.encode('utf-8')]) + + mock_func.assert_called_once_with(container=container) + self.assertTrue(out.out.find( + 't\u00e9st_c [after 2 attempts]') >= 0, out) + @mock.patch('swiftclient.service.Connection') def test_delete_object(self, connection): argv = ["", "delete", "container", "object"] @@ -661,8 +694,8 @@ class TestShell(unittest.TestCase): def test_delete_verbose_output(self): del_obj_res = {'success': True, 'response_dict': {}, 'attempts': 2, - 'container': 'test_c', 'action': 'delete_object', - 'object': 'test_o'} + 'container': 't\xe9st_c', 'action': 'delete_object', + 'object': 't\xe9st_o'} del_seg_res = del_obj_res.copy() del_seg_res.update({'action': 'delete_segment'}) @@ -670,7 +703,7 @@ class TestShell(unittest.TestCase): del_con_res = del_obj_res.copy() del_con_res.update({'action': 'delete_container', 'object': None}) - test_exc = Exception('test_exc') + test_exc = Exception('t\xe9st_exc') error_res = del_obj_res.copy() error_res.update({'success': False, 'error': test_exc, 'object': None}) @@ -680,39 +713,39 @@ class TestShell(unittest.TestCase): with mock.patch('swiftclient.shell.SwiftService.delete', mock_delete): with CaptureOutput() as out: mock_delete.return_value = [del_obj_res] - swiftclient.shell.main(base_argv + ['test_c', 'test_o']) + swiftclient.shell.main(base_argv + ['t\xe9st_c', 't\xe9st_o']) - mock_delete.assert_called_once_with(container='test_c', - objects=['test_o']) + mock_delete.assert_called_once_with(container='t\xe9st_c', + objects=['t\xe9st_o']) self.assertTrue(out.out.find( - 'test_o [after 2 attempts]') >= 0) + 't\xe9st_o [after 2 attempts]') >= 0) with CaptureOutput() as out: mock_delete.return_value = [del_seg_res] - swiftclient.shell.main(base_argv + ['test_c', 'test_o']) + swiftclient.shell.main(base_argv + ['t\xe9st_c', 't\xe9st_o']) - mock_delete.assert_called_with(container='test_c', - objects=['test_o']) + mock_delete.assert_called_with(container='t\xe9st_c', + objects=['t\xe9st_o']) self.assertTrue(out.out.find( - 'test_c/test_o [after 2 attempts]') >= 0) + 't\xe9st_c/t\xe9st_o [after 2 attempts]') >= 0) with CaptureOutput() as out: mock_delete.return_value = [del_con_res] - swiftclient.shell.main(base_argv + ['test_c']) + swiftclient.shell.main(base_argv + ['t\xe9st_c']) - mock_delete.assert_called_with(container='test_c') + mock_delete.assert_called_with(container='t\xe9st_c') self.assertTrue(out.out.find( - 'test_c [after 2 attempts]') >= 0) + 't\xe9st_c [after 2 attempts]') >= 0) with CaptureOutput() as out: mock_delete.return_value = [error_res] self.assertRaises(SystemExit, swiftclient.shell.main, - base_argv + ['test_c']) + base_argv + ['t\xe9st_c']) - mock_delete.assert_called_with(container='test_c') + mock_delete.assert_called_with(container='t\xe9st_c') self.assertTrue(out.err.find( - 'Error Deleting: test_c: test_exc') >= 0) + 'Error Deleting: t\xe9st_c: t\xe9st_exc') >= 0) @mock.patch('swiftclient.service.Connection') def test_post_account(self, connection): @@ -1636,7 +1669,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): self.assertRequests([('PUT', self.cont_path), ('PUT', self.obj_path)]) self.assertEqual(self.obj[1:], out.strip()) - expected_err = 'Warning: failed to create container %r: 403 Fake' \ + expected_err = "Warning: failed to create container '%s': 403 Fake" \ % self.cont self.assertEqual(expected_err, out.err.strip()) @@ -1658,7 +1691,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): self.assertRequests([('PUT', self.cont_path), ('PUT', self.obj_path)]) self.assertEqual(self.obj[1:], out.strip()) - expected_err = 'Warning: failed to create container %r: 403 Fake' \ + expected_err = "Warning: failed to create container '%s': 403 Fake" \ % self.cont self.assertEqual(expected_err, out.err.strip()) @@ -1693,7 +1726,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): self.assert_request(('PUT', segment_path_1)) self.assert_request(('PUT', self.obj_path)) self.assertTrue(self.obj[1:] in out.out) - expected_err = 'Warning: failed to create container %r: 403 Fake' \ + expected_err = "Warning: failed to create container '%s': 403 Fake" \ % self.cont self.assertEqual(expected_err, out.err.strip()) @@ -1782,7 +1815,7 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest): self.assertRequests([('GET', self.obj_path)]) path = '%s%s' % (self.cont, self.obj) - expected_err = 'Error downloading object %r' % path + expected_err = "Error downloading object '%s'" % path self.assertTrue(out.err.startswith(expected_err)) self.assertEqual('', out) -- cgit v1.2.1