From 1df55dd952fe52c1c1fc2583326d017275b01ddc Mon Sep 17 00:00:00 2001 From: Ravi Shekhar Jethani Date: Tue, 18 Oct 2016 13:43:06 +0530 Subject: [PATCH] Validate input args before trying image download Currently client is contacting glance service even if the caller has niether specified any redirection nor '--file' option. This unnecessary request although isn't causing any critical issues but can be avoided by simply doing input validation first. TrivialFix Change-Id: I841bebeda38814235079429eca0b1e5fd2f04dae --- glanceclient/tests/unit/v2/test_shell_v2.py | 14 ++++++++++++++ glanceclient/v2/shell.py | 15 ++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/glanceclient/tests/unit/v2/test_shell_v2.py b/glanceclient/tests/unit/v2/test_shell_v2.py index fed1f720..4ac8c17a 100644 --- a/glanceclient/tests/unit/v2/test_shell_v2.py +++ b/glanceclient/tests/unit/v2/test_shell_v2.py @@ -590,6 +590,20 @@ class ShellV2Test(testtools.TestCase): test_shell.do_image_download(self.gc, args) mocked_data.assert_called_once_with('IMG-01') + @mock.patch.object(utils, 'exit') + @mock.patch('sys.stdout', autospec=True) + def test_image_download_no_file_arg(self, mocked_stdout, + mocked_utils_exit): + # Indicate that no file name was given as command line argument + args = self._make_args({'id': '1234', 'file': None, 'progress': False}) + # Indicate that no file is specified for output redirection + mocked_stdout.isatty = lambda: True + test_shell.do_image_download(self.gc, args) + mocked_utils_exit.assert_called_once_with( + 'No redirection or local file specified for downloaded image' + ' data. Please specify a local file with --file to save' + ' downloaded image or redirect output to another source.') + def test_do_image_delete(self): args = argparse.Namespace(id=['image1', 'image2']) with mock.patch.object(self.gc.images, 'delete') as mocked_delete: diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py index 5354db42..9ce6f96f 100644 --- a/glanceclient/v2/shell.py +++ b/glanceclient/v2/shell.py @@ -278,6 +278,12 @@ def do_explain(gc, args): help=_('Show download progress bar.')) def do_image_download(gc, args): """Download a specific image.""" + if sys.stdout.isatty() and (args.file is None): + msg = ('No redirection or local file specified for downloaded image ' + 'data. Please specify a local file with --file to save ' + 'downloaded image or redirect output to another source.') + utils.exit(msg) + try: body = gc.images.data(args.id) except (exc.HTTPForbidden, exc.HTTPException) as e: @@ -290,13 +296,8 @@ def do_image_download(gc, args): if args.progress: body = progressbar.VerboseIteratorWrapper(body, len(body)) - if not (sys.stdout.isatty() and args.file is None): - utils.save_image(body, args.file) - else: - msg = ('No redirection or local file specified for downloaded image ' - 'data. Please specify a local file with --file to save ' - 'downloaded image or redirect output to another source.') - utils.exit(msg) + + utils.save_image(body, args.file) @utils.arg('--file', metavar='',