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 78C39A0032; Fri, 24 Jun 2022 13:29:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 173234069D; Fri, 24 Jun 2022 13:29:49 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id CA938400EF for ; Fri, 24 Jun 2022 13:29:47 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [RFC PATCH 2/6] telemetry: fix escaping of invalid json characters X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Fri, 24 Jun 2022 13:29:46 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87174@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [RFC PATCH 2/6] telemetry: fix escaping of invalid json characters Thread-Index: AdiHu+djOrVC3819QQeFtyjrvjx2hgAAJ+SQ References: <20220623164245.561371-1-bruce.richardson@intel.com> <20220623164245.561371-3-bruce.richardson@intel.com> <98CBD80474FA8B44BF855DF32C47DC35D8716B@smartserver.smartshare.dk> <20220623113934.372b059e@hermes.local> <98CBD80474FA8B44BF855DF32C47DC35D8716C@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" Cc: "Stephen Hemminger" , , , 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 > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Friday, 24 June 2022 13.17 >=20 > On Fri, Jun 24, 2022 at 09:00:38AM +0100, Bruce Richardson wrote: > > On Thu, Jun 23, 2022 at 08:48:21PM +0200, Morten Br=F8rup wrote: > > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > > > Sent: Thursday, 23 June 2022 20.40 > > > > > > > > On Thu, 23 Jun 2022 20:34:07 +0200 > > > > Morten Br=F8rup wrote: > > > > > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > > > > Sent: Thursday, 23 June 2022 18.43 > > > > > > > > > > > > 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. > > > > > > > > > > Correct. Other chars are optional to escape. > > > > > > > > For json_writer (which I wrote for iproute2 and could have been > used > > > > here). > > > > The switch handles: \t \n \r \f \b \\ " ' as special cases. > > > > > > RFC 8259 chapter 7 says: > > > > > > All Unicode characters may be placed within the > > > quotation marks, except for the characters that MUST be = escaped: > > > quotation mark, reverse solidus, and the control characters > (U+0000 > > > through U+001F). > > > > > > I have no preference for either, as long as '/' and other non- > control characters are not (unnecessarily) escaped. > > > > > > Using tested and maintained code like json_writer could be > beneficial. If you hold the copyright, there should be no license > issues. > > > > > > > I will take a look at json_writer. >=20 > Took a quick look at json_writer, and it's certainly an option. The > main > gap compared to what we have in our current implementation is that > json_writer is designed around a stream for output rather than an > output > buffer. Now while we can use fmemopen to make our buffer act as a > stream > for writing, and the write apis should prevent it overflowing, we = still > hit > the issue of the result of truncation not being valid json. The = current > implementation tries to handle truncation more gracefully in that any > fields which don't fit just don't get added. >=20 > I'll think about it a bit more, and see if there is a way that it can > be > made to work more cleanly. It sounds like json_writer provides a more advanced API, adding a lot of = overhead for wrapping it into the Telemetry library. Since we only need = a very simple encoder, perhaps copy-paste-modify is more viable. Or just = proceed with your RFC code. Regardless, the API and underlying code probably needs extra scrutiny, = so it doesn't become an attack vector into the control plane of a DPDK = application. >=20 > /Bruce >=20 > PS: just changing the output from a string to a stream on the output > socket > I don't believe is an option either, as the socket type used for > telemetry > is a SOCK_SEQPACKET where message boundaries are preserved, and a > single > read will return the entire telemetry reply.