From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 65808A00C5; Wed, 27 Jul 2022 03:13:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F2D0140E03; Wed, 27 Jul 2022 03:13:23 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id F0C5A40A89 for ; Wed, 27 Jul 2022 03:13:21 +0200 (CEST) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4Lswjj0r0Kz9sv0 for ; Wed, 27 Jul 2022 09:12:09 +0800 (CST) Received: from [127.0.0.1] (10.67.100.224) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Wed, 27 Jul 2022 09:13:18 +0800 Subject: Re: [PATCH v2 02/13] telemetry: fix escaping of invalid json characters To: Bruce Richardson , CC: Ciara Power , Keith Wiles References: <20220623164245.561371-1-bruce.richardson@intel.com> <20220725163543.875775-1-bruce.richardson@intel.com> <20220725163543.875775-3-bruce.richardson@intel.com> From: fengchengwen Message-ID: Date: Wed, 27 Jul 2022 09:13:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20220725163543.875775-3-bruce.richardson@intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2022/7/26 0:35, Bruce Richardson wrote: > For string values returned from telemetry, escape any values that cannot > normally appear in a json string. According to the json spec[1], the > characters than need to be handled are control chars (char value < 0x20) > and '"' and '\' characters. > > To handle this, we replace the snprintf call with a separate string > copying and encapsulation routine which checks each character as it > copies it to the final array. > > [1] https://www.rfc-editor.org/rfc/rfc8259.txt > > Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality") > Bugzilla ID: 1037 > > Signed-off-by: Bruce Richardson > --- > lib/telemetry/telemetry.c | 11 +++++--- > lib/telemetry/telemetry_json.h | 48 +++++++++++++++++++++++++++++++++- > 2 files changed, 55 insertions(+), 4 deletions(-) > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > index c6fd03a5ab..7188b1905c 100644 > --- a/lib/telemetry/telemetry.c > +++ b/lib/telemetry/telemetry.c > @@ -232,9 +232,14 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) > MAX_CMD_LEN, cmd ? cmd : "none"); > break; > case RTE_TEL_STRING: > - used = snprintf(out_buf, sizeof(out_buf), "{\"%.*s\":\"%.*s\"}", > - MAX_CMD_LEN, cmd, > - RTE_TEL_MAX_SINGLE_STRING_LEN, d->data.str); > + prefix_used = snprintf(out_buf, sizeof(out_buf), "{\"%.*s\":", > + MAX_CMD_LEN, cmd); The cmd need also escaped. But I notice the [PATCH v2 06/13] limit it. Suggest move 06 at the head of patchset. > + cb_data_buf = &out_buf[prefix_used]; > + buf_len = sizeof(out_buf) - prefix_used - 1; /* space for '}' */ > + > + used = rte_tel_json_str(cb_data_buf, buf_len, 0, d->data.str); > + used += prefix_used; > + used += strlcat(out_buf + used, "}", sizeof(out_buf) - used); > break; > case RTE_TEL_DICT: > prefix_used = snprintf(out_buf, sizeof(out_buf), "{\"%.*s\":", > diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h > index db70690274..13df5d07e3 100644 > --- a/lib/telemetry/telemetry_json.h > +++ b/lib/telemetry/telemetry_json.h > @@ -44,6 +44,52 @@ __json_snprintf(char *buf, const int len, const char *format, ...) > return 0; /* nothing written or modified */ > } > > +static const char control_chars[0x20] = { > + ['\n'] = 'n', > + ['\r'] = 'r', > + ['\t'] = 't', > +}; > + > +/** > + * @internal > + * Does the same as __json_snprintf(buf, len, "\"%s\"", str) > + * except that it does proper escaping as necessary. > + * Drops any invalid characters we don't support > + */ > +static inline int > +__json_format_str(char *buf, const int len, const char *str) > +{ > + char tmp[len]; Could reuse buf otherthan tmp > + int tmpidx = 0; > + > + tmp[tmpidx++] = '"'; > + while (*str != '\0') { > + if (*str < (int)RTE_DIM(control_chars)) { > + int idx = *str; /* compilers don't like char type as index */ > + if (control_chars[idx] != 0) { > + tmp[tmpidx++] = '\\'; > + tmp[tmpidx++] = control_chars[idx]; Why not espace all control chars? > + } > + } else if (*str == '"' || *str == '\\') { > + tmp[tmpidx++] = '\\'; > + tmp[tmpidx++] = *str; > + } else > + tmp[tmpidx++] = *str; > + /* we always need space for closing quote and null character. > + * Ensuring at least two free characters also means we can always take an > + * escaped character like "\n" without overflowing > + */ > + if (tmpidx > len - 2) > + return 0; Suggest add log here to help find out problem. > + str++; > + } > + tmp[tmpidx++] = '"'; > + tmp[tmpidx] = '\0'; > + > + strcpy(buf, tmp); > + return tmpidx; > +} > + > /* Copies an empty array into the provided buffer. */ > static inline int > rte_tel_json_empty_array(char *buf, const int len, const int used) > @@ -62,7 +108,7 @@ rte_tel_json_empty_obj(char *buf, const int len, const int used) > static inline int > rte_tel_json_str(char *buf, const int len, const int used, const char *str) > { > - return used + __json_snprintf(buf + used, len - used, "\"%s\"", str); > + return used + __json_format_str(buf + used, len - used, str); > } > > /* Appends a string into the JSON array in the provided buffer. */ >