[API] Validate display name/description length
In case of snapshot create/update, if display name or description is above max limit (fields created in db with limit 255), manila internally throws DB exception. But the error reported to user is not meaningful. Fix by validating name/description length should not cross max limit. Closes-bug: #2023964 Change-Id: I6b1a274da3692700650f84736877c0ae98d46c81
This commit is contained in:
parent
afd0d723bb
commit
c88aac595b
@ -267,6 +267,16 @@ def check_share_network_is_active(share_network):
|
|||||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||||
|
|
||||||
|
|
||||||
|
def check_display_field_length(field, field_name):
|
||||||
|
if field:
|
||||||
|
length = len(field)
|
||||||
|
if length > constants.DB_DISPLAY_FIELDS_MAX_LENGTH:
|
||||||
|
raise exception.InvalidInput(
|
||||||
|
reason=("%(field_name)s can only be %(len)d characters long."
|
||||||
|
% {'field_name': field_name,
|
||||||
|
'len': constants.DB_DISPLAY_FIELDS_MAX_LENGTH}))
|
||||||
|
|
||||||
|
|
||||||
def parse_is_public(is_public):
|
def parse_is_public(is_public):
|
||||||
"""Parse is_public into something usable.
|
"""Parse is_public into something usable.
|
||||||
|
|
||||||
|
@ -186,6 +186,11 @@ class ShareSnapshotMixin(object):
|
|||||||
for key in valid_update_keys
|
for key in valid_update_keys
|
||||||
if key in snapshot_data}
|
if key in snapshot_data}
|
||||||
|
|
||||||
|
common.check_display_field_length(
|
||||||
|
update_dict.get('display_name'), 'display_name')
|
||||||
|
common.check_display_field_length(
|
||||||
|
update_dict.get('display_description'), 'display_description')
|
||||||
|
|
||||||
try:
|
try:
|
||||||
snapshot = self.share_api.get_snapshot(context, id)
|
snapshot = self.share_api.get_snapshot(context, id)
|
||||||
except exception.NotFound:
|
except exception.NotFound:
|
||||||
@ -230,12 +235,16 @@ class ShareSnapshotMixin(object):
|
|||||||
# NOTE(rushiagr): v2 API allows name instead of display_name
|
# NOTE(rushiagr): v2 API allows name instead of display_name
|
||||||
if 'name' in snapshot:
|
if 'name' in snapshot:
|
||||||
snapshot['display_name'] = snapshot.get('name')
|
snapshot['display_name'] = snapshot.get('name')
|
||||||
|
common.check_display_field_length(
|
||||||
|
snapshot['display_name'], 'name')
|
||||||
del snapshot['name']
|
del snapshot['name']
|
||||||
|
|
||||||
# NOTE(rushiagr): v2 API allows description instead of
|
# NOTE(rushiagr): v2 API allows description instead of
|
||||||
# display_description
|
# display_description
|
||||||
if 'description' in snapshot:
|
if 'description' in snapshot:
|
||||||
snapshot['display_description'] = snapshot.get('description')
|
snapshot['display_description'] = snapshot.get('description')
|
||||||
|
common.check_display_field_length(
|
||||||
|
snapshot['display_description'], 'description')
|
||||||
del snapshot['description']
|
del snapshot['description']
|
||||||
|
|
||||||
kwargs = {}
|
kwargs = {}
|
||||||
|
@ -218,6 +218,11 @@ class ShareMixin(object):
|
|||||||
for key in valid_update_keys
|
for key in valid_update_keys
|
||||||
if key in share_data}
|
if key in share_data}
|
||||||
|
|
||||||
|
common.check_display_field_length(
|
||||||
|
update_dict.get('display_name'), 'display_name')
|
||||||
|
common.check_display_field_length(
|
||||||
|
update_dict.get('display_description'), 'display_description')
|
||||||
|
|
||||||
try:
|
try:
|
||||||
share = self.share_api.get(context, id)
|
share = self.share_api.get(context, id)
|
||||||
except exception.NotFound:
|
except exception.NotFound:
|
||||||
@ -258,12 +263,15 @@ class ShareMixin(object):
|
|||||||
# NOTE(rushiagr): Manila API allows 'name' instead of 'display_name'.
|
# NOTE(rushiagr): Manila API allows 'name' instead of 'display_name'.
|
||||||
if share.get('name'):
|
if share.get('name'):
|
||||||
share['display_name'] = share.get('name')
|
share['display_name'] = share.get('name')
|
||||||
|
common.check_display_field_length(share['display_name'], 'name')
|
||||||
del share['name']
|
del share['name']
|
||||||
|
|
||||||
# NOTE(rushiagr): Manila API allows 'description' instead of
|
# NOTE(rushiagr): Manila API allows 'description' instead of
|
||||||
# 'display_description'.
|
# 'display_description'.
|
||||||
if share.get('description'):
|
if share.get('description'):
|
||||||
share['display_description'] = share.get('description')
|
share['display_description'] = share.get('description')
|
||||||
|
common.check_display_field_length(
|
||||||
|
share['display_description'], 'description')
|
||||||
del share['description']
|
del share['description']
|
||||||
|
|
||||||
size = share['size']
|
size = share['size']
|
||||||
|
@ -16,6 +16,9 @@
|
|||||||
# The maximum value a signed INT type may have
|
# The maximum value a signed INT type may have
|
||||||
DB_MAX_INT = 0x7FFFFFFF
|
DB_MAX_INT = 0x7FFFFFFF
|
||||||
|
|
||||||
|
# The maximum length a display field may have
|
||||||
|
DB_DISPLAY_FIELDS_MAX_LENGTH = 255
|
||||||
|
|
||||||
# SHARE AND GENERAL STATUSES
|
# SHARE AND GENERAL STATUSES
|
||||||
STATUS_CREATING = 'creating'
|
STATUS_CREATING = 'creating'
|
||||||
STATUS_CREATING_FROM_SNAPSHOT = 'creating_from_snapshot'
|
STATUS_CREATING_FROM_SNAPSHOT = 'creating_from_snapshot'
|
||||||
|
@ -23,6 +23,7 @@ from manila.api.v1 import share_snapshots
|
|||||||
from manila.common import constants
|
from manila.common import constants
|
||||||
from manila import context
|
from manila import context
|
||||||
from manila import db
|
from manila import db
|
||||||
|
from manila import exception
|
||||||
from manila.share import api as share_api
|
from manila.share import api as share_api
|
||||||
from manila import test
|
from manila import test
|
||||||
from manila.tests.api.contrib import stubs
|
from manila.tests.api.contrib import stubs
|
||||||
@ -89,6 +90,32 @@ class ShareSnapshotAPITest(test.TestCase):
|
|||||||
|
|
||||||
self.assertEqual(expected, res_dict)
|
self.assertEqual(expected, res_dict)
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'name': 'name1', 'description': 'x' * 256},
|
||||||
|
{'name': 'x' * 256, 'description': 'description1'},
|
||||||
|
)
|
||||||
|
@ddt.unpack
|
||||||
|
def test_snapshot_create_invalid_input(self, name, description):
|
||||||
|
self.mock_object(share_api.API, 'create_snapshot')
|
||||||
|
self.mock_object(
|
||||||
|
share_api.API,
|
||||||
|
'get',
|
||||||
|
mock.Mock(return_value={'snapshot_support': True,
|
||||||
|
'is_soft_deleted': False}))
|
||||||
|
body = {
|
||||||
|
'snapshot': {
|
||||||
|
'share_id': 200,
|
||||||
|
'force': False,
|
||||||
|
'name': name,
|
||||||
|
'description': description,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
req = fakes.HTTPRequest.blank('/fake/snapshots')
|
||||||
|
|
||||||
|
self.assertRaises(
|
||||||
|
exception.InvalidInput,
|
||||||
|
self.controller.create, req, body)
|
||||||
|
|
||||||
@ddt.data(0, False)
|
@ddt.data(0, False)
|
||||||
def test_snapshot_create_no_support(self, snapshot_support):
|
def test_snapshot_create_no_support(self, snapshot_support):
|
||||||
self.mock_object(share_api.API, 'create_snapshot')
|
self.mock_object(share_api.API, 'create_snapshot')
|
||||||
|
@ -458,6 +458,30 @@ class ShareAPITest(test.TestCase):
|
|||||||
self.mock_policy_check.assert_called_once_with(
|
self.mock_policy_check.assert_called_once_with(
|
||||||
req.environ['manila.context'], self.resource_name, 'create')
|
req.environ['manila.context'], self.resource_name, 'create')
|
||||||
|
|
||||||
|
@ddt.data(
|
||||||
|
{'name': 'name1', 'description': 'x' * 256},
|
||||||
|
{'name': 'x' * 256, 'description': 'description1'},
|
||||||
|
)
|
||||||
|
@ddt.unpack
|
||||||
|
def test_share_create_invalid_input(self, name, description):
|
||||||
|
self.mock_object(share_api.API, 'create')
|
||||||
|
shr = {
|
||||||
|
"size": 100,
|
||||||
|
"name": name,
|
||||||
|
"description": description,
|
||||||
|
"share_proto": "fakeproto",
|
||||||
|
"availability_zone": "zone1:host1",
|
||||||
|
}
|
||||||
|
body = {"share": shr}
|
||||||
|
req = fakes.HTTPRequest.blank('/v1/fake/shares')
|
||||||
|
|
||||||
|
self.assertRaises(exception.InvalidInput,
|
||||||
|
self.controller.create,
|
||||||
|
req,
|
||||||
|
body)
|
||||||
|
self.mock_policy_check.assert_called_once_with(
|
||||||
|
req.environ['manila.context'], self.resource_name, 'create')
|
||||||
|
|
||||||
@ddt.data("1.0", "2.0")
|
@ddt.data("1.0", "2.0")
|
||||||
def test_share_create_from_snapshot_not_supported(self, microversion):
|
def test_share_create_from_snapshot_not_supported(self, microversion):
|
||||||
# This create operation should work, because the 1.0 API doesn't check
|
# This create operation should work, because the 1.0 API doesn't check
|
||||||
|
@ -0,0 +1,8 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
During share/snapshot create/update API calls, if display name or
|
||||||
|
description is above max db limit i.e. 255, Manila throws error. But in
|
||||||
|
this case, end user message is not meaningful. Fixed it by adding valid
|
||||||
|
error message. For more details, please check
|
||||||
|
`Launchpad Bug #2023964 <https://bugs.launchpad.net/manila/+bug/2023964>`_
|
Loading…
Reference in New Issue
Block a user