ygot icon indicating copy to clipboard operation
ygot copied to clipboard

PathToStrings API Issue with special characters in input path

Open schandrashek opened this issue 4 years ago • 3 comments

PathToStrings doesn't work as expected with paths having special characters.

Eg: /components/component[name=abc[123]]

Here, the string returned from the API is /components/component[name=abc[123\]].

The issue here is in the elemToString() function, where only a ']' is correctly escaped and there's no escape for '['.

schandrashek avatar Oct 14 '21 19:10 schandrashek

Quoting an earlier version of the spec referenced in the PR whic introduced this code, https://github.com/openconfig/ygot/pull/133/commits/9a80a268939e9137abf863a1400513ea23a1b4c3:

in paths with list keys, only closing square brackets and \ characters must be escaped if they are part of a key value. The = character may not appear in key names in YANG, so [name=k1=v1] is not ambiguous, i.e., name is assigned the value of k1=v1. Further, the sequence [name=[foo] is not ambiguous, name is assigned the value of [foo. On the other hand setting name to the value [] would be encoded as [name=[\]]. Escape can optionally be applied to the opening square brackets.

The current version of gnmi-path-conventions.md doesn't seem to explicitly mention about escaping special characters. I think it would be better to be consistent with escaping both '[' and ']'.

schandrashek avatar Oct 14 '21 20:10 schandrashek

I agree that it's inconsistent that, as in your referred commit, both = and ] are escaped, despite the former being unambiguous, and [ is not escaped even though it's also a special character. @robshakir Do you mind if I fix this inconsistency by escaping [ as well and then adding the decision to the gNMI spec? I think I agree with Shachindra here.

wenovus avatar Oct 18 '21 18:10 wenovus

Adding to 1.0 since this is backwards-incompatible.

wenovus avatar Oct 18 '21 19:10 wenovus