Fix slash in path. #3573

xincunli-sonic · 2024-10-11T16:56:05Z

What I did

Addressing issue #20377. The issue caused by unescape in JsonPointer implementation which followed RFC 6901

pointer = jsonpointer.JsonPointer(path)

...

class JsonPointer(object):
    """A JSON Pointer that can reference parts of a JSON document"""

    # Array indices must not contain:
    # leading zeros, signs, spaces, decimals, etc
    _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$')
    _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)')

    def __init__(self, pointer):

        # validate escapes
        invalid_escape = self._RE_INVALID_ESCAPE.search(pointer)
        if invalid_escape:
            raise JsonPointerException('Found invalid escape {}'.format(
                invalid_escape.group()))

        parts = pointer.split('/')
        if parts.pop(0) != '':
            raise JsonPointerException('Location must start with /')

        parts = [unescape(part) for part in parts]
        self.parts = parts

How I did it

Re-escape / to ~1 to match the real key in json, and JsonPatch will handle it correctly.

How to verify it

admin@str2-7250-lc1-2:~$ cat link.json 
[
    {
        "op": "add",
        "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131",
        "value": {}
    }
]
admin@str2-7250-lc1-2:~$ sudo config apply-patch link.json 
sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER
sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER
sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER
Patch Applier: asic1: Patch application starting.
Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {}}]
Patch Applier: asic1 getting current config db.
Patch Applier: asic1: simulating the target full config after applying the patch.
Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: asic1: validating target config does not have empty tables,
                            since they do not show up in ConfigDb.
Patch Applier: asic1: sorting patch updates.
Patch Applier: The asic1 patch was converted into 0 changes.
Patch Applier: asic1: applying 0 changes in order.
Patch Applier: asic1: verifying patch updates are reflected on ConfigDB.
Patch Applier: asic1 patch application completed.
Patch applied successfully.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

wen587 · 2024-10-15T05:25:21Z

generic_config_updater/generic_updater.py

-    parts = pointer.parts
+
+    # Re-escapes `/` in path parts by converting `/` back to `~1`.
+    parts = [part.replace("/", "~1") for part in pointer.parts]


Is it an issue for multi-asic only?
I remember in single asic, the jsonpointer has built in feature to parse / to ~1

It was introduced by multi asic change, we have to extract the namespace from the patch, then JsonPointer will do escape ~1 back to /

Sorry. I don't get it.
By the code here, the extract_scope doesn't differentiate single or multi. How do we guarantee single asic is not broken?

sonic-utilities/config/main.py

Lines 1599 to 1600 in 319f58d

for change in patch:

scope, modified_path = extract_scope(change["path"])

For single asic, the portchannel interface does not have slash in port, but multi asic does have, hence, we could do escape slash to ~1 in jsonpatch.

Another reason is single asic logic is taking path as input parameter directly, but multi asic, we have to do combine with parts by Jsonpointer result..

qiluo-msft · 2024-10-28T23:36:50Z

        except Exception:

Can we use specific (not too general) exception type? #Closed

Refers to: tests/generic_config_updater/multiasic_change_applier_test.py:80 in 26ee2a4. [](commit_id = 26ee2a4, deletion_comment = False)

generic_config_updater/generic_updater.py

qiluo-msft · 2024-10-29T23:36:02Z

        except Exception:

still applicable.

In reply to: 2442871334

Refers to: tests/generic_config_updater/multiasic_change_applier_test.py:80 in 26ee2a4. [](commit_id = 26ee2a4, deletion_comment = False)

xincunli-sonic · 2024-10-29T23:40:51Z

discussed offline, change to specific error handling to avoid unexpected errors.

mlok-nokia · 2024-11-25T14:49:32Z

@xincunli-sonic This PR has cherry-pick conflict. Please help to address it so that can cherry-pick to 202405. Thanks

#### What I did Addressing issue [#20377](sonic-net/sonic-buildimage#20377). The issue caused by unescape in JsonPointer implementation which followed [RFC 6901](https://www.rfc-editor.org/rfc/rfc6901) ```python pointer = jsonpointer.JsonPointer(path) ... class JsonPointer(object): """A JSON Pointer that can reference parts of a JSON document""" # Array indices must not contain: # leading zeros, signs, spaces, decimals, etc _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$') _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)') def __init__(self, pointer): # validate escapes invalid_escape = self._RE_INVALID_ESCAPE.search(pointer) if invalid_escape: raise JsonPointerException('Found invalid escape {}'.format( invalid_escape.group())) parts = pointer.split('/') if parts.pop(0) != '': raise JsonPointerException('Location must start with /') parts = [unescape(part) for part in parts] self.parts = parts ``` #### How I did it Re-escape `/` to `~1` to match the real key in json, and [JsonPatch](https://www.rfc-editor.org/rfc/rfc6902#appendix-A.14) will handle it correctly. #### How to verify it ```shell admin@str2-7250-lc1-2:~$ cat link.json [ { "op": "add", "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {} } ] admin@str2-7250-lc1-2:~$ sudo config apply-patch link.json sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER Patch Applier: asic1: Patch application starting. Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {}}] Patch Applier: asic1 getting current config db. Patch Applier: asic1: simulating the target full config after applying the patch. Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields Patch Applier: asic1: validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: asic1: sorting patch updates. Patch Applier: The asic1 patch was converted into 0 changes. Patch Applier: asic1: applying 0 changes in order. Patch Applier: asic1: verifying patch updates are reflected on ConfigDB. Patch Applier: asic1 patch application completed. Patch applied successfully. ```

xincunli-sonic · 2024-11-26T22:58:38Z

This requires the previous PR be cherry picked into 202405, after it done, I will reuquest the current PR backport again.

mlok-nokia · 2024-11-27T14:47:10Z

This requires the previous PR be cherry picked into 202405, after it done, I will reuquest the current PR backport again.

Thanks.

Addressing issue [#20377](sonic-net/sonic-buildimage#20377). The issue caused by unescape in JsonPointer implementation which followed [RFC 6901](https://www.rfc-editor.org/rfc/rfc6901) ```python pointer = jsonpointer.JsonPointer(path) ... class JsonPointer(object): """A JSON Pointer that can reference parts of a JSON document""" _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$') _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)') def __init__(self, pointer): invalid_escape = self._RE_INVALID_ESCAPE.search(pointer) if invalid_escape: raise JsonPointerException('Found invalid escape {}'.format( invalid_escape.group())) parts = pointer.split('/') if parts.pop(0) != '': raise JsonPointerException('Location must start with /') parts = [unescape(part) for part in parts] self.parts = parts ``` Re-escape `/` to `~1` to match the real key in json, and [JsonPatch](https://www.rfc-editor.org/rfc/rfc6902#appendix-A.14) will handle it correctly. ```shell admin@str2-7250-lc1-2:~$ cat link.json [ { "op": "add", "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {} } ] admin@str2-7250-lc1-2:~$ sudo config apply-patch link.json sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER Patch Applier: asic1: Patch application starting. Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {}}] Patch Applier: asic1 getting current config db. Patch Applier: asic1: simulating the target full config after applying the patch. Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields Patch Applier: asic1: validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: asic1: sorting patch updates. Patch Applier: The asic1 patch was converted into 0 changes. Patch Applier: asic1: applying 0 changes in order. Patch Applier: asic1: verifying patch updates are reflected on ConfigDB. Patch Applier: asic1 patch application completed. Patch applied successfully. ```

bingwang-ms · 2025-02-04T22:24:50Z

Included in 202405 by PR #3649

Fix slash in path.

f5910fc

xincunli-sonic mentioned this pull request Oct 11, 2024

[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' sonic-net/sonic-buildimage#20377

Closed

Fix format

26ee2a4

xincunli-sonic requested review from wen587 and qiluo-msft October 11, 2024 17:01

wen587 reviewed Oct 15, 2024

View reviewed changes

wen587 previously approved these changes Oct 23, 2024

View reviewed changes

qiluo-msft reviewed Oct 28, 2024

View reviewed changes

generic_config_updater/generic_updater.py Outdated Show resolved Hide resolved

Addressing comments

816dc9c

xincunli-sonic dismissed wen587’s stale review via 816dc9c October 29, 2024 22:32

Add specific error in test case.

27dc498

qiluo-msft approved these changes Nov 8, 2024

View reviewed changes

qiluo-msft merged commit 7d013df into sonic-net:master Nov 8, 2024
7 checks passed

arlakshm added the Request for 202405 Branch label Nov 13, 2024

mssonicbld added the Cherry Pick Conflict_202405 label Nov 13, 2024

xincunli-sonic added Request for 202405 Branch and removed Cherry Pick Conflict_202405 Request for 202405 Branch labels Dec 2, 2024

mssonicbld added the Cherry Pick Conflict_202405 label Dec 3, 2024

bingwang-ms added the Approved for 202405 Branch label Feb 4, 2025

bingwang-ms added Included in 202405 Branch and removed Approved for 202405 Branch Cherry Pick Conflict_202405 labels Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix slash in path. #3573

Fix slash in path. #3573

xincunli-sonic commented Oct 11, 2024 •

edited

Loading

wen587 Oct 15, 2024 •

edited

Loading

xincunli-sonic Oct 15, 2024

wen587 Oct 16, 2024

xincunli-sonic Oct 16, 2024 •

edited

Loading

xincunli-sonic Oct 17, 2024

qiluo-msft commented Oct 28, 2024 •

edited

Loading

qiluo-msft commented Oct 29, 2024

xincunli-sonic commented Oct 29, 2024 •

edited

Loading

mlok-nokia commented Nov 25, 2024

xincunli-sonic commented Nov 26, 2024 •

edited

Loading

mlok-nokia commented Nov 27, 2024 •

edited

Loading

bingwang-ms commented Feb 4, 2025

	for change in patch:
	scope, modified_path = extract_scope(change["path"])

Fix slash in path. #3573

Fix slash in path. #3573

Conversation

xincunli-sonic commented Oct 11, 2024 • edited Loading

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

wen587 Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

xincunli-sonic Oct 15, 2024

Choose a reason for hiding this comment

wen587 Oct 16, 2024

Choose a reason for hiding this comment

xincunli-sonic Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

xincunli-sonic Oct 17, 2024

Choose a reason for hiding this comment

qiluo-msft commented Oct 28, 2024 • edited Loading

qiluo-msft commented Oct 29, 2024

xincunli-sonic commented Oct 29, 2024 • edited Loading

mlok-nokia commented Nov 25, 2024

xincunli-sonic commented Nov 26, 2024 • edited Loading

mlok-nokia commented Nov 27, 2024 • edited Loading

bingwang-ms commented Feb 4, 2025

xincunli-sonic commented Oct 11, 2024 •

edited

Loading

wen587 Oct 15, 2024 •

edited

Loading

xincunli-sonic Oct 16, 2024 •

edited

Loading

qiluo-msft commented Oct 28, 2024 •

edited

Loading

xincunli-sonic commented Oct 29, 2024 •

edited

Loading

xincunli-sonic commented Nov 26, 2024 •

edited

Loading

mlok-nokia commented Nov 27, 2024 •

edited

Loading