diff --git a/swiftclient/shell.py b/swiftclient/shell.py index e1a5a8d1..4e3ed0af 100755 --- a/swiftclient/shell.py +++ b/swiftclient/shell.py @@ -1239,17 +1239,17 @@ def st_tempurl(parser, args, thread_manager): parsed = urlparse(path) - try: - seconds = int(seconds) - except ValueError: - thread_manager.error('Seconds must be an integer') - return if method.upper() not in ['GET', 'PUT', 'HEAD', 'POST', 'DELETE']: thread_manager.print_msg('WARNING: Non default HTTP method %s for ' 'tempurl specified, possibly an error' % method.upper()) - path = generate_temp_url(parsed.path, seconds, key, method, - absolute=options['absolute_expiry']) + try: + path = generate_temp_url(parsed.path, seconds, key, method, + absolute=options['absolute_expiry']) + except ValueError as err: + thread_manager.error(err) + return + if parsed.scheme and parsed.netloc: url = "%s://%s%s" % (parsed.scheme, parsed.netloc, path) else: diff --git a/swiftclient/utils.py b/swiftclient/utils.py index d3942839..2e727ad5 100644 --- a/swiftclient/utils.py +++ b/swiftclient/utils.py @@ -68,14 +68,20 @@ def generate_temp_url(path, seconds, key, method, absolute=False): :param path: The full path to the Swift object. Example: /v1/AUTH_account/c/o. - :param seconds: The amount of time in seconds the temporary URL will - be valid for. + :param seconds: If absolute is False then this specifies the amount of time + in seconds for which the temporary URL will be valid. If absolute is + True then this specifies an absolute time at which the temporary URL + will expire. :param key: The secret temporary URL key set on the Swift cluster. To set a key, run 'swift post -m "Temp-URL-Key: <substitute tempurl key here>"' :param method: A HTTP method, typically either GET or PUT, to allow for this temporary URL. - :raises: ValueError if seconds is not a positive integer + :param absolute: if True then the seconds parameter is interpreted as an + absolute Unix time, otherwise seconds is interpreted as a relative time + offset from current time. + :raises: ValueError if seconds is not a positive integer or path is not to + an object. :raises: TypeError if seconds is not an integer :return: the path portion of a temporary URL """ @@ -94,6 +100,10 @@ def generate_temp_url(path, seconds, key, method, absolute=False): else: path_for_body = path + parts = path_for_body.split('/', 4) + if len(parts) != 5 or parts[0] or not all(parts[1:]): + raise ValueError('path must be full path to an object e.g. /v1/a/c/o') + standard_methods = ['GET', 'PUT', 'HEAD', 'POST', 'DELETE'] if method.upper() not in standard_methods: logger = logging.getLogger("swiftclient") diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py index 40881c24..3f077dc6 100644 --- a/tests/unit/test_shell.py +++ b/tests/unit/test_shell.py @@ -1402,7 +1402,7 @@ class TestShell(unittest.TestCase): "secret_key"] swiftclient.shell.main(argv) temp_url.assert_called_with( - '/v1/AUTH_account/c/o', 60, 'secret_key', 'GET', absolute=False) + '/v1/AUTH_account/c/o', "60", 'secret_key', 'GET', absolute=False) @mock.patch('swiftclient.shell.generate_temp_url', return_value='') def test_absolute_expiry_temp_url(self, temp_url): @@ -1410,7 +1410,7 @@ class TestShell(unittest.TestCase): "secret_key", "--absolute"] swiftclient.shell.main(argv) temp_url.assert_called_with( - '/v1/AUTH_account/c/o', 60, 'secret_key', 'GET', absolute=True) + '/v1/AUTH_account/c/o', "60", 'secret_key', 'GET', absolute=True) def test_temp_url_output(self): argv = ["", "tempurl", "GET", "60", "/v1/a/c/o", @@ -1428,6 +1428,18 @@ class TestShell(unittest.TestCase): expected = "http://saio:8080%s" % expected self.assertEqual(expected, output.out) + def test_temp_url_error_output(self): + expected = 'path must be full path to an object e.g. /v1/a/c/o\n' + for bad_path in ('/v1/a/c', 'v1/a/c/o', '/v1/a/c/', '/v1/a//o', + 'http://saio/v1/a/c', 'http://v1/a/c/o'): + argv = ["", "tempurl", "GET", "60", bad_path, + "secret_key", "--absolute"] + with CaptureOutput(suppress_systemexit=True) as output: + swiftclient.shell.main(argv) + self.assertEqual(expected, output.err, + 'Expected %r but got %r for path %r' % + (expected, output.err, bad_path)) + @mock.patch('swiftclient.service.Connection') def test_capabilities(self, connection): argv = ["", "capabilities"] diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index f0de79ce..e820aff1 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -180,6 +180,32 @@ class TestTempURL(unittest.TestCase): self.assertEqual(exc_manager.exception.args[0], 'seconds must be a positive integer') + def test_generate_temp_url_bad_path(self): + with self.assertRaises(ValueError) as exc_manager: + u.generate_temp_url('/v1/a/c', 60, self.key, self.method) + self.assertEqual(exc_manager.exception.args[0], + 'path must be full path to an object e.g. /v1/a/c/o') + + with self.assertRaises(ValueError) as exc_manager: + u.generate_temp_url('v1/a/c/o', 60, self.key, self.method) + self.assertEqual(exc_manager.exception.args[0], + 'path must be full path to an object e.g. /v1/a/c/o') + + with self.assertRaises(ValueError) as exc_manager: + u.generate_temp_url('blah/v1/a/c/o', 60, self.key, self.method) + self.assertEqual(exc_manager.exception.args[0], + 'path must be full path to an object e.g. /v1/a/c/o') + + with self.assertRaises(ValueError) as exc_manager: + u.generate_temp_url('/v1//c/o', 60, self.key, self.method) + self.assertEqual(exc_manager.exception.args[0], + 'path must be full path to an object e.g. /v1/a/c/o') + + with self.assertRaises(ValueError) as exc_manager: + u.generate_temp_url('/v1/a/c/', 60, self.key, self.method) + self.assertEqual(exc_manager.exception.args[0], + 'path must be full path to an object e.g. /v1/a/c/o') + class TestTempURLUnicodePathAndKey(TestTempURL): url = u'/v1/\u00e4/c/\u00f3'