openstack/python-swiftclient
Code Issues Proposed changes

Make tempurl command check for valid object path

Browse Source 
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
This commit is contained in:
Alistair Coles 2016-09-08 14:15:40 +01:00
parent 89ff29e788
commit 4c955751d3
4 changed files with 60 additions and 12 deletions

14
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:

16
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")

16
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"]

26
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'