From 8a3d11cacd3043266da7a6c11f46611a30fdeee0 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 19 May 2016 17:16:56 +0200 Subject: [PATCH] Fail on disabled_reasons with more than 255 chars Current code removes leading and trailing spaces when checking disabled reason for a service, but then stores it as non trimmed, so we could be trying to store in the DB a string longer than 255 characters. To preserve existing storing behavior this patch will store leading and trailing spaces as we are doing now, but will take them into consideration when counting maximum length. It will also raise an error if the reason is only comprised of spaces, just like we are doing now. Change-Id: Iee97d8786644ad02a83c902c7780055e2c075198 Closes-Bug: #1583652 --- cinder/api/contrib/services.py | 4 ++-- cinder/tests/unit/api/contrib/test_services.py | 6 +++++- cinder/utils.py | 8 +++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py index fa531a08f8f..cddb6b8fae6 100644 --- a/cinder/api/contrib/services.py +++ b/cinder/api/contrib/services.py @@ -98,8 +98,8 @@ class ServiceController(wsgi.Controller): if not reason: return False try: - utils.check_string_length(reason.strip(), 'Disabled reason', - min_length=1, max_length=255) + utils.check_string_length(reason, 'Disabled reason', min_length=1, + max_length=255, allow_all_spaces=False) except exception.InvalidInput: return False diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index 23a5066a5f0..9c330060a15 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -653,10 +653,14 @@ class ServicesTest(test.TestCase): req, "disable-log-reason", body) def test_invalid_reason_field(self): - reason = ' ' + # Check that empty strings are not allowed + reason = ' ' * 10 self.assertFalse(self.controller._is_valid_as_reason(reason)) reason = 'a' * 256 self.assertFalse(self.controller._is_valid_as_reason(reason)) + # Check that spaces at the end are also counted + reason = 'a' * 255 + ' ' + self.assertFalse(self.controller._is_valid_as_reason(reason)) reason = 'it\'s a valid reason.' self.assertTrue(self.controller._is_valid_as_reason(reason)) reason = None diff --git a/cinder/utils.py b/cinder/utils.py index 15b23fd31ce..dc461409d24 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -625,7 +625,8 @@ def get_blkdev_major_minor(path, lookup_for_file=True): raise exception.Error(msg) -def check_string_length(value, name, min_length=0, max_length=None): +def check_string_length(value, name, min_length=0, max_length=None, + allow_all_spaces=True): """Check the length of specified string. :param value: the value of the string @@ -640,6 +641,11 @@ def check_string_length(value, name, min_length=0, max_length=None): except(ValueError, TypeError) as exc: raise exception.InvalidInput(reason=exc) + if not allow_all_spaces and str.isspace(value): + msg = _('%(name)s cannot be all spaces.') + raise exception.InvalidInput(reason=msg) + + _visible_admin_metadata_keys = ['readonly', 'attached_mode']