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 422F4A00C4; Wed, 27 Jul 2022 10:27:57 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E760140141; Wed, 27 Jul 2022 10:27:56 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id CD15C400D7 for ; Wed, 27 Jul 2022 10:27:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658910475; x=1690446475; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=1kfCGgku7FnfYXS3G6Znc8slwQBuQPEKGp4YdnOYovU=; b=nTCMdI4sKo5Kqk6GPmxU12pKNMxu82tcwYFQTxxh1k2MELnlgbjSli0d fkX7eZF4GtMdhQwPq617Us1nqibByGiV9/KMLHtN30S9umo4YFPL0h5iE +szA+vBMpkeRXlshzbJd/cObDzl5Y5PCSwEXIyZuNcTEFo1e5fOzGUibB VVniLJbQ3kr0QRu3TNq3GUXsMbNK408KJ2h9CtpO4Gka+PZOR29oa267S xTggrLtjtwCU6Gi/JII1vWtWM0Lyzh6OX04x1zDzvfTOUF0izfsOLpnyP 3yLvSrTFx//QvojkB2ty/IObpdC6CmmDLspSxC/xfu629AvbGVQrKiibj w==; X-IronPort-AV: E=McAfee;i="6400,9594,10420"; a="288939825" X-IronPort-AV: E=Sophos;i="5.93,195,1654585200"; d="scan'208";a="288939825" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jul 2022 01:27:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,195,1654585200"; d="scan'208";a="597347321" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orsmga007.jf.intel.com with ESMTP; 27 Jul 2022 01:27:35 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28; Wed, 27 Jul 2022 01:27:35 -0700 Received: from orsmsx602.amr.corp.intel.com (10.22.229.15) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28; Wed, 27 Jul 2022 01:27:34 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.28 via Frontend Transport; Wed, 27 Jul 2022 01:27:34 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.170) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.28; Wed, 27 Jul 2022 01:27:34 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aFrlE8QWm6oJakSwOGBtJiHhwP5jjVJHv3lZ+L88c72EM5IU2wTW1BI+trCF2/OMefZi+UXQ9x/A5BrLZN/Fzlt9b0KtLCdbpPk43V8A8lmfT4yhSpj6+m3E/QfilOcxVurZoHq46lfGqT0TBhiU/tPNoBV+BMpZYyet9oUxr+g06O+HL6WUo8w23NfQioXFLc4uW7qmgUWOTRzoE1dLEejuDb2/vH5Rf6iaUrnaSQ44ioml95RbiyywaJ7EhrDbeu7c6DoXiNQ6s+wr2DFojgzdmeNXrQFCWNWIwkfguNrxurch/qqAPySjZfCJEXUIt2xQ4ITkwQrVEMn3y/JajA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4EbL/4VKDKxWgvyOs8LBercqeLAkDDOoMfL3NCKMCK8=; b=iPdrgQY+6juXTrSrXrw5+wHyPwTzV3Xu9W9c3WRyqaAhaNI7K47LBG1ROtWfrCbv/Xdxxggd4X7xKlejDJrml6a8JZi6cr1EiIuYlXlEc+z8NdOaMzhZt2E6kMivIzjB+12gICAxjI2ax4jRzaL8tN5GYdciiDy5T/GqGIE+f1IseA17GcNW4UFWPT7N9lzvVdoUrIqHggoZpeRtctt3iERUWXoMrVE2JT9ZcMhsC0NBE/sG8nxKqhlPLp9xubD5WEdGgW+Yv7DX1oa7Fbdt1Kg/+/gC3HlpVC4xWu1l+FwcR+rCEwweOVnvR42UZv03V9kaLD2HpdCXD3JkxecJaQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) by MW3PR11MB4556.namprd11.prod.outlook.com (2603:10b6:303:5b::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5458.19; Wed, 27 Jul 2022 08:27:32 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::8e4:e1e9:a851:4b0d]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::8e4:e1e9:a851:4b0d%5]) with mapi id 15.20.5458.024; Wed, 27 Jul 2022 08:27:32 +0000 Date: Wed, 27 Jul 2022 09:27:26 +0100 From: Bruce Richardson To: fengchengwen CC: , Ciara Power , Keith Wiles Subject: Re: [PATCH v2 02/13] telemetry: fix escaping of invalid json characters Message-ID: References: <20220623164245.561371-1-bruce.richardson@intel.com> <20220725163543.875775-1-bruce.richardson@intel.com> <20220725163543.875775-3-bruce.richardson@intel.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: LO4P123CA0002.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:150::7) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 0ec3e0ff-c3e9-4d3c-da8c-08da6fa9d9f4 X-MS-TrafficTypeDiagnostic: MW3PR11MB4556:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: G6dfi0rIUU+A7kA9nIh9OzMsPM6X0IRCX83mvxOi/K0Io3nk5AuXsEkHJKeg9hz6uW4lW4uYrFX2NCm8uY2py7WGMtzKrChiaqURRjfeshXagFpoPjqQI9uv08SJz1RSlcWQb3JyZzJ80I4KpvUc2GSu5cRK7ot0MNcOzKA5ZzvGaAi2RZoI4wYCNop9KdgYX5wxKhwiZPrlG7A5ADXKwSEsF2sYzrwIhyDzQtwdoe+UWAcdl6N+d0+3EKwn+FbokNgdjzPLfyDOjucs5n6Uf3P1Xp8TTUkjz8EseWFzNGbf8238ghNM0Okv+xoeuXmB9yRk6bC6CQmeg77BzNH/DFUj3wvNwxplUfH5hEFmX1rVG3yxNZgn/dw341LinoALaBFp39JALKjqp2gCJiHEBKr1goxAsFMpsfYoa2LC5aCpgEIfB7fa1W3ECpblKpwYjAG2SuXWjHM8v6dGernTDJSfEyRbFAw/7gfxD+uNf21lGy5FyxjbbnHLMZDvsaw1kNvR/jvkpeSiCE3OH4ZWI/UJ3lNe/GaaLVZOmxYapqfSBGNVyb5NmZx5xSzirSLPIEY47L9ecRZQ6qP+ZG3Y98HheubjrrmqpMr/DfgYZ049bNhPCMXenSs77IgjEF8dx6irzP+oszENNBR04mAbC55ArbayJ0P7EVbsVf8EIF1mLM+iKfEuVSX+TgtC9jyyBL8SJAl9uXhefJZhMQvnG0oRs4En9DuHfxXR20pxtuChWaQ1TX4JsNC+C3QrY4q70I073KVCXbz5xo1UeLa/tw== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1629.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230016)(136003)(346002)(376002)(39860400002)(366004)(396003)(66556008)(66476007)(6486002)(38100700002)(8676002)(5660300002)(66946007)(966005)(82960400001)(8936002)(478600001)(4326008)(83380400001)(53546011)(6506007)(26005)(6512007)(186003)(107886003)(54906003)(6666004)(41300700001)(44832011)(6916009)(2906002)(316002)(86362001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EXNSiJtwtHIEqdmt1i9VHxG/R9AP8mtV2qIxOmTHtS/Es9PQrcjEAeTs+99Z?= =?us-ascii?Q?DRPZBoVx0dhgmt1EZfApj1lV/CuWPvIbsjDnH1KxlvjXHZsZX9F1q3SPuWwM?= =?us-ascii?Q?aXBcvWBkmNlAohW02Oa6Ovj5rDIUjd3+0/nvM0aVQ6KrlHlNNDPgZvATA09w?= =?us-ascii?Q?ZO3U2mTk38nihq0UiodDgP4561vb5e0bg58CXDpBrt4JaDkpX1Zh8LVSEmUU?= =?us-ascii?Q?d+upUxzLCb2AEbcfWA2Y4Z/gTgmNoxNcKBEQLLa5pJGbTZKDLQ6u4pbMMXSA?= =?us-ascii?Q?maTZZHA3gwkdKQr84yUELs5ZDvA3ps3iSgXG+EbWbukh4aCNCKQWP6qfGHVl?= =?us-ascii?Q?mIluOHw+TNKdUJvHKs8DudVh9jS8dDLW4dAS1dUZCZX6m9oewKIEu2cpuovH?= =?us-ascii?Q?J/ap9HYBMMlbZS46X/IKXQxcPXYYAFHv7Er221Bu9sWtOgraATzijv3hjyJ0?= =?us-ascii?Q?mOiDJjRdpMZRV1HzO3pIC1K27hmc/t2FPR4gNffgoIE31kWUs7FB1I6KE/Wg?= =?us-ascii?Q?xxAVkixjJm3x6qaHiR7ZwbeGwMK55MU+1Ov4VuG1Aeu08qZ+UEzAdWU05xrb?= =?us-ascii?Q?2G50Ds3YkvenvpPzePqJ27XELYISqaRCe22rw76I87uIllFT/T4pLdGLtnmi?= =?us-ascii?Q?U2Oxyl8X2mDiEBCp1slko2Bput7yt1PSMqC1WLyQuF3dBD5nAOzIb/AR3iCo?= =?us-ascii?Q?JJL9+mhAhnmLz/zT0v1DqHtACUlVOPsni+/CHjx8sJvIK2tHq49lJNbizbSs?= =?us-ascii?Q?UgCwjMLaygzNoUX9nby+mpfLSEohhzcW22BIs718Uk2AHtFWdSL0s698DVjD?= =?us-ascii?Q?Xw8OMmoWSBgDg2ecxD3QAPe8263LGCCUHNNoQi5rqQMFeg4Wg179meUgO+Yv?= =?us-ascii?Q?SkzzPZew7UoQZ+6584pQt3FrmmNiet0Fxou91hyqCi86D6Qb4ESioq2SlgVq?= =?us-ascii?Q?dxE8fqW/6thMN2QDDC9qb+PBWrU2J1QOFngL5s6NUDOsReHFBHHTYHqZKzbT?= =?us-ascii?Q?gynbp3pyQu/JLicL9z3oC0XiPW18bxO236YtZWLy2Iyw44vUciLHhwDgbZOd?= =?us-ascii?Q?7PeY5hiw37kF0lk2ghPPW21BDOngPme1bHhIDt2dOJ0C9f+bk9WhtST4qVZf?= =?us-ascii?Q?p3O7v2bM80p/uYQjpINZDfEEhwqOfSizO4QXH+A02pUcZm0+7jHmY+mlatBy?= =?us-ascii?Q?1YeSAqWDtWfDTqUp0FqBgSlTtCr4/Ji6k2mOy69VwBiZtDB5UsqTv4jnqT1W?= =?us-ascii?Q?MUQRePCZZIachTUFLC9EFKEPe7wIH4FQMRpaAh5iHVdXbfP0HoUn4/rswnlu?= =?us-ascii?Q?LrDgmIXLwZHIptU4SNcKZzqfTpVS6LQDdZnaMtgsG17hvDMSWnnQ9FeEWbib?= =?us-ascii?Q?SQYYv66CUai5NxIHMZq6qGsS4hcRmbiSxCahTP/tldYnAQMhmE34I5bTtFoe?= =?us-ascii?Q?ozvTkL3F9dDdMTpBkV/p6wi3f/7+gSBRkLt+g23ddKl+h6MGx7zQ1xTnPnao?= =?us-ascii?Q?jz8PmanuqOUdSvZDDF/042ffr1WZsBkTXn31eZ7Zpke8plAnB1l55POfwrBr?= =?us-ascii?Q?tFVPiTGgosX5pm/3knw7nyExzKldMbmrvTIdGde2xUa+MxUmNQKIQx9va2kn?= =?us-ascii?Q?HQ=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 0ec3e0ff-c3e9-4d3c-da8c-08da6fa9d9f4 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2022 08:27:32.8048 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 5+OOQiyUbsMKbLw1DC8By9afd9AHAUiBmvTBFHZ6Zxg2NyGxY0QZ2F+MJk0Bz8mU8bJTOdvrUgAkyBHTeTM560/Y/+YpVeWJsky2htYQTq4= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4556 X-OriginatorOrg: intel.com 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 Wed, Jul 27, 2022 at 09:13:18AM +0800, fengchengwen wrote: > 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. > Right. I'll try some patch reordering in the next version of this set. > > + 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 > The approach here is to guarantee that we always output valid json. Therefore, we build up the output in a temporary buffer until we are sure that it's all correct and can fit, before moving it into the final buffer. That way, if there are any issues, the original buffer is unmodified, and we can return the bytes-appended as 0. > > + 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? > Because only certain characters have valid escape codes, and any other characters would have to be replaced with unicode values. These should not be ever appearing in our text output fields anyway. > > + } > > + } 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. > Telemetry is operating in a background thread, so not sure logging is a good idea in such cases. I'd look for other opinions on this... > > + 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. */ > > >