openstack/ironic
Code Issues Proposed changes

Allow special characters in patch field keys

Browse Source 
Allow special characters in patch field keys, as long as they
are correctly encoded per JSON pointer and patch specs RFC 6901
and RFC 6902.

Closes-Bug: #1604148
Change-Id: I7eeb52b51a0e8ba96103e0863819653021c79271
This commit is contained in:
cid 2024-10-30 11:40:46 +01:00 committed by Afonne-CID
parent 41124271a1
commit 5fccd55c9f
6 changed files with 123 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/contributor/webapi-version-history.rst
View File

@ -2,6 +2,10 @@
REST API Version History
========================
1.98 (Flamingo)
Add support for patching object attributes with keys containing ~ or /.
1.97 (Flamingo)
-----------------------

16
ironic/api/controllers/v1/utils.py
View File

@ -399,6 +399,17 @@ def patched_validate_with_schema(patched_dict, schema, validator=None):
validator('patch', patched_dict)
def allow_special_characters_in_patch_fields():
"""Check if special characters are allowed in patch field keys."""
return (api.request.version.minor
>= versions.MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES)
def decode_json_pointer(path):
"""Decode JSON pointer as per RFC 6901 to support '~' and '/'."""
return path.replace('~1', '/').replace('~0', '~')
def patch_validate_allowed_fields(patch, allowed_fields):
"""Validate that a patch list only modifies allowed fields.
@ -410,7 +421,10 @@ def patch_validate_allowed_fields(patch, allowed_fields):
"""
fields = set()
for p in patch:
path = p['path'].split('/')[1]
if allow_special_characters_in_patch_fields():
path = decode_json_pointer(p['path'].split('/')[1])
else:
path = p['path'].split('/')[1]
if path not in allowed_fields:
msg = _("Cannot patch %s. Only the following can be updated: %s")
raise exception.Invalid(

4
ironic/api/controllers/v1/versions.py
View File

@ -135,6 +135,7 @@ BASE_VERSION = 1
# v1.95: Add node support for disable_power_off
# v1.96: Migrate inspection rules from Inspector
# v1.97: Add description field to port.
# v1.98: Add support for object attributes with keys containing ~ or /.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -234,6 +235,7 @@ MINOR_94_PORT_NODENAME = 94
MINOR_95_DISABLE_POWER_OFF = 95
MINOR_96_INSPECTION_RULES = 96
MINOR_97_PORT_DESCRIPTION = 97
MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES = 98
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -241,7 +243,7 @@ MINOR_97_PORT_DESCRIPTION = 97
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_97_PORT_DESCRIPTION
MINOR_MAX_VERSION = MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

2
ironic/common/release_mappings.py
View File

@ -848,7 +848,7 @@ RELEASE_MAPPING = {
# make it below. To release, we will preserve a version matching
# the release as a separate block of text, like above.
'master': {
'api': '1.97',
'api': '1.98',
'rpc': '1.61',
'objects': {
'Allocation': ['1.1'],

98
ironic/tests/unit/api/controllers/v1/test_utils.py
View File

@ -90,6 +90,62 @@ class TestApiUtils(base.TestCase):
self.assertRaises(exception.PatchError,
utils.apply_jsonpatch, doc, patch)
def test_apply_jsonpatch_with_tilde(self):
"""Unescaped tilde fails."""
doc = {"foo": {"bar~baz": "value"}}
patch = [{"op": "replace", "path": "/foo/bar~baz",
"value": "new_value"}]
self.assertRaises(exception.PatchError,
utils.apply_jsonpatch, doc, patch)
def test_apply_jsonpatch_with_slash(self):
"""Unescaped forward slash fails."""
doc = {"foo": {"bar/baz": "value"}}
patch = [{"op": "replace", "path": "/foo/bar/baz",
"value": "new_value"}]
self.assertRaises(exception.PatchError,
utils.apply_jsonpatch, doc, patch)
def test_apply_jsonpatch_with_escaped_tilde(self):
"""Escaped tilde (~0) should work per RFC 6902."""
doc = {"foo": {"bar~baz": "value"}}
patch = [{"op": "replace", "path": "/foo/bar~0baz",
"value": "new_value"}]
expected = {"foo": {"bar~baz": "new_value"}}
result = utils.apply_jsonpatch(doc, patch)
self.assertEqual(expected, result)
def test_apply_jsonpatch_with_escaped_slash(self):
"""Escaped slash (~1) should work per RFC 6902."""
doc = {"foo": {"bar/baz": "value"}}
patch = [{"op": "replace", "path": "/foo/bar~1baz",
"value": "new_value"}]
expected = {"foo": {"bar/baz": "new_value"}}
result = utils.apply_jsonpatch(doc, patch)
self.assertEqual(expected, result)
def test_apply_jsonpatch_with_escaped_complex_path(self):
doc = {"foo": {"~bar/baz~": "value"}}
patch = [{"op": "replace", "path": "/foo/~0bar~1baz~0",
"value": "new_value"}]
expected = {"foo": {"~bar/baz~": "new_value"}}
result = utils.apply_jsonpatch(doc, patch)
self.assertEqual(expected, result)
def test_apply_jsonpatch_with_invalid_escape_sequence(self):
doc = {"foo": {"bar": "value"}}
patch = [{"op": "replace", "path": "/foo/bar~2baz",
"value": "new_value"}]
self.assertRaises(exception.PatchError,
utils.apply_jsonpatch, doc, patch)
def test_apply_jsonpatch_with_incomplete_escape(self):
doc = {"foo": {"bar": "value"}}
patch = [{"op": "replace", "path": "/foo/bar~",
"value": "new_value"}]
self.assertRaises(exception.PatchError,
utils.apply_jsonpatch, doc, patch)
def test_get_patch_values_no_path(self):
patch = [{'path': '/name', 'op': 'update', 'value': 'node-0'}]
path = '/invalid'
@ -300,10 +356,12 @@ class TestApiUtils(base.TestCase):
validate)
self.assertIn("big ouch", str(e))
def test_patch_validate_allowed_fields(self):
allowed_fields = ['one', 'two', 'three']
@mock.patch.object(api, 'request', autospec=False)
def test_patch_validate_allowed_fields(self, mock_request):
mock_request.version.minor = 94
allowed_fields = ['one', 'two', 'three', 'four/five']
# patch all
# patch all (except 'four/five')
self.assertEqual(
{'one', 'two', 'three'},
utils.patch_validate_allowed_fields([
@ -319,6 +377,13 @@ class TestApiUtils(base.TestCase):
{'path': '/one'},
], allowed_fields))
# patch one (special field not supported)
self.assertRaises(
exception.Invalid,
utils.patch_validate_allowed_fields,
[{'path': '/four~1five'}],
allowed_fields)
# patch invalid field
e = self.assertRaises(
exception.Invalid,
@ -330,8 +395,31 @@ class TestApiUtils(base.TestCase):
"one, two, three", str(e))
@mock.patch.object(api, 'request', autospec=False)
def test_sanitize_dict(self, mock_req):
mock_req.public_url = 'http://192.0.2.1:5050'
def test_patch_validate_allowed_special_fields(self, mock_request):
mock_request.version.minor = 98
allowed_fields = ['one', 'two', 'three', 'four/five', 'four~five']
# patch all
self.assertEqual(
{'one', 'two', 'three', 'four/five', 'four~five'},
utils.patch_validate_allowed_fields([
{'path': '/one'},
{'path': '/two'},
{'path': '/three'},
{'path': '/four~0five'},
{'path': '/four~1five'},
], allowed_fields))
# patch one (special field)
self.assertEqual(
{'four/five'},
utils.patch_validate_allowed_fields([
{'path': '/four~1five'},
], allowed_fields))
@mock.patch.object(api, 'request', autospec=False)
def test_sanitize_dict(self, mock_request):
mock_request.public_url = 'http://192.0.2.1:5050'
node = obj_utils.get_test_node(
self.context,

7
releasenotes/notes/support-special-characters-in-patch-field-e077fb994661362c.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
Updates the patch validation logic to support special characters
(``~`` and ``/``) in field keys, provided they align with the
escaping rules defined in RFC 6901 (JSON Pointer) as required for
the `path` field specified in RFC 6902 (JSON Patch).