From 4c955751d340a8f71a2eebdb3c58d90b36874a66 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistair.coles@hpe.com>
Date: Thu, 8 Sep 2016 14:15:40 +0100
Subject: [PATCH] Make tempurl command check for valid object path
If the supplied path is not of the form /v1/a/c/o then
swift tempurl subcommand will now return an error message.
Also removes redundant check for seconds parameter being an int
from shell.py because the same check is made when calling
utils.generate_temp_url.
Drive-by fix for missing param definition for generate_temp_url.
Change-Id: I41f4389948b01fadaa5fc4939ea12e0ed2167345
Related-Change: I0fb2ce125fe12d660e4deb778265016bdd5ff31b
---
swiftclient/shell.py | 14 +++++++-------
swiftclient/utils.py | 16 +++++++++++++---
tests/unit/test_shell.py | 16 ++++++++++++++--
tests/unit/test_utils.py | 26 ++++++++++++++++++++++++++
4 files changed, 60 insertions(+), 12 deletions(-)
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'