Re: [PATCH v2 1/5] telemetry: escape special char when tel string

[fengchengwen <fengchengwen@huawei.com>]

On 2022/6/18 1:05, Stephen Hemminger wrote:
> On Fri, 17 Jun 2022 12:25:04 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
>> On Fri, Jun 17, 2022 at 01:16:08PM +0200, Morten Brørup wrote:
>>>> From: Chengwen Feng [mailto:fengchengwen@huawei.com]
>>>> Sent: Friday, 17 June 2022 11.46
>>>>
>>>> This patch supports escape special characters (including: \",\\,/,\b,
>>>> /f,/n,/r,/t) when telemetry string.
>>>> This patch is used to support telemetry xxx-dump commands which the
>>>> string may include special characters.
>>>>
>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>  lib/telemetry/telemetry.c | 96 +++++++++++++++++++++++++++++++++++++--
>>>>  1 file changed, 93 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
>>>> index c6fd03a5ab..0f762f633e 100644
>>>> --- a/lib/telemetry/telemetry.c
>>>> +++ b/lib/telemetry/telemetry.c
>>>> @@ -215,6 +215,94 @@ container_to_json(const struct rte_tel_data *d,
>>>> char *out_buf, size_t buf_len)
>>>>  	return used;
>>>>  }
>>>>
>>>> +static bool
>>>> +json_is_special_char(char ch)
>>>> +{
>>>> +	static unsigned char is_spec[256] = { 0 };
>>>> +	static bool init_once;
>>>> +
>>>> +	if (!init_once) {
>>>> +		is_spec['\"'] = 1;
>>>> +		is_spec['\\'] = 1;
>>>> +		is_spec['/'] = 1;
>>>> +		is_spec['\b'] = 1;
>>>> +		is_spec['\f'] = 1;
>>>> +		is_spec['\n'] = 1;
>>>> +		is_spec['\r'] = 1;
>>>> +		is_spec['\t'] = 1;
>>>> +		init_once = true;
>>>> +	}
>>>> +
>>>> +	return (bool)is_spec[(unsigned char)ch];
>>>> +}  
>>
>> According to the json spec at [1], the characters that need to be escaped
>> are:
>> a) any characters <0x20
>> b) inverted commas/quote character \"
>> c) the "reverse solidus character", better known to you and I as the
>> back-slash.
>>
>> Therefore, I think this table generation could be simplified, but also
>> expanded using this. For completeness we should also see about handling all
>> control characters if they are encountered.
>>
>> [1] https://www.rfc-editor.org/rfc/rfc8259.txt
>>
>> /Bruce
> 
> Since it is trivial could be initializer?
> 
> static const uint8_t is_spec[256] = {
>    [0 ... 0x20] = 1,
>    ['\"' ] = 1,
>    ['\\' ] = 1,
>    ['/'] = 1,
> 
> etc
> 
> Or we could change the telemetry API to disallow control characters?

I was thinking about converting 0~0x20, but I don't think there's a scenario.

I prefer change the telemetry API to disallow control characters, and this may not
be a violation of the ABI, if yes, the dpdk-telemetry.py will returns an error.

So I think we could add declaring and checking functions to make sure telemetry string
do not allow control characters.

> 
> 
> .
>