From fc119315f1931f4893fb4e7423b4c806772f77a5 Mon Sep 17 00:00:00 2001 From: Deepti Ramakrishna Date: Thu, 22 Oct 2015 16:40:02 -0700 Subject: [PATCH] Fix volume filtering for quoted display name Cinder v2 API allows creating a volume with a quoted (single or double quotes or even unbalanced number of quotes) display name. But when we try to get info for such volume, we end up getting an error message saying that no volume with such a name or ID exists. This error is due to the inadvertent stripping of quotes from the filter in the api layer. The api call eventually comes to check_volume_filters() in cinder/volume/api.py. The invocation of ast.literal_eval() inside this method strips the quotes for certain quoted strings leading to this incorrect filtering. ast.literal_eval() is used to convert string representations into python objects which are then used to frame the SQL queries in the db layer. For example, the string "[1,2]" for a filter (not the display name filter) gets converted to a list object and results in an "IN" operation being emitted in the SQL query as opposed to an exact match. When display_name does not contain any quotes or contains an unbalanced number of quotes, then ast.literal_eval() throws (just like the Python interpreter would throw for an unquoted string literal or one with unbalanced number of quotes). We handle this by ignoring the exception and using the raw input value as the filter and moving on. For string containing balanced number of quotes, such as, '"foo"', ast.literal_eval() succeeds and returns the input with the surrounding quotes stripped (just like how the python interpreter strips quotes from a string literal to initialize a string var's value in memory). To always use the raw user input string as the filter value, we can either not pass string inputs to ast.literal_eval() or encode the string using encode("string-escape") so that we get the original string back after passing through ast.literal_eval(). We choose the former as the latter buys us nothing. Change-Id: I48e0aea801ccb011cb974eea3d685bb9f35c61b2 Closes-Bug: #1503485 --- cinder/tests/unit/api/v2/test_volumes.py | 34 ++++++++++++++++--- cinder/volume/api.py | 7 ++++ ...-quoted-display-name-7f5e8ac888a73001.yaml | 5 +++ 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 6dafffeab..265286948 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -1362,20 +1362,46 @@ class VolumeApiTest(test.TestCase): body = {'volume': 'string'} self._create_volume_bad_request(body=body) - @mock.patch('cinder.volume.api.API.get_all') - def test_get_volumes_filter_with_string(self, get_all): + def _test_get_volumes_by_name(self, get_all, display_name): req = mock.MagicMock() context = mock.Mock() req.environ = {'cinder.context': context} - req.params = {'display_name': 'Volume-573108026'} + req.params = {'display_name': display_name} self.controller._view_builder.detail_list = mock.Mock() self.controller._get_volumes(req, True) get_all.assert_called_once_with( context, None, CONF.osapi_max_limit, sort_keys=['created_at'], sort_dirs=['desc'], - filters={'display_name': 'Volume-573108026'}, + filters={'display_name': display_name}, viewable_admin_meta=True, offset=0) + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_string(self, get_all): + """Test to get a volume with an alpha-numeric display name.""" + self._test_get_volumes_by_name(get_all, 'Volume-573108026') + + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_double_quoted_string(self, get_all): + """Test to get a volume with a double-quoted display name.""" + self._test_get_volumes_by_name(get_all, '"Volume-573108026"') + + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_single_quoted_string(self, get_all): + """Test to get a volume with a single-quoted display name.""" + self._test_get_volumes_by_name(get_all, "'Volume-573108026'") + + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_quote_in_between_string(self, get_all): + """Test to get a volume with a quote in between the display name.""" + self._test_get_volumes_by_name(get_all, 'Volu"me-573108026') + + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_mixed_quoted_string(self, get_all): + """Test to get a volume with a mix of single and double quotes. """ + # The display name starts with a single quote and ends with a + # double quote + self._test_get_volumes_by_name(get_all, '\'Volume-573108026"') + @mock.patch('cinder.volume.api.API.get_all') def test_get_volumes_filter_with_true(self, get_all): req = mock.MagicMock() diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 3d3f9429c..e25dc2862 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -1667,6 +1667,13 @@ class API(base.Base): filters[k] = True else: filters[k] = bool(v) + elif k == 'display_name': + # Use the raw value of display name as is for the filter + # without passing it through ast.literal_eval(). If the + # display name is a properly quoted string (e.g. '"foo"') + # then literal_eval() strips the quotes (i.e. 'foo'), so + # the filter becomes different from the user input. + continue else: filters[k] = ast.literal_eval(v) except (ValueError, SyntaxError): diff --git a/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml b/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml new file mode 100644 index 000000000..8192b265c --- /dev/null +++ b/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Filtering volumes by their display name now + correctly handles display names with single and + double quotes. -- 2.45.2