From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Bruce Richardson" <bruce.richardson@intel.com>
Cc: "Stephen Hemminger" <stephen@networkplumber.org>, <dev@dpdk.org>,
<ciara.power@intel.com>, <fengchengwen@huawei.com>
Subject: RE: [RFC PATCH 2/6] telemetry: fix escaping of invalid json characters
Date: Fri, 24 Jun 2022 13:29:46 +0200 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87174@smartserver.smartshare.dk> (raw)
In-Reply-To: <YrWdGYgO1GJU+0yl@bricha3-MOBL.ger.corp.intel.com>
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 24 June 2022 13.17
>
> 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ørup 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ørup <mb@smartsharesystems.com> 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.
>
> 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.
>
> 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.
>
> /Bruce
>
> 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.
next prev parent reply other threads:[~2022-06-24 11:29 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-23 16:42 [RFC PATCH 0/6] add json string escaping to telemetry Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 1/6] test/telemetry_json: print success or failure per subtest Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 2/6] telemetry: fix escaping of invalid json characters Bruce Richardson
2022-06-23 18:34 ` Morten Brørup
2022-06-23 18:39 ` Stephen Hemminger
2022-06-23 18:48 ` Morten Brørup
2022-06-24 8:00 ` Bruce Richardson
2022-06-24 11:16 ` Bruce Richardson
2022-06-24 11:29 ` Morten Brørup [this message]
2022-06-24 15:06 ` Stephen Hemminger
2022-06-24 8:03 ` Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 3/6] telemetry: use json string function for string outputs Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 4/6] test/telemetry_json: add test for string character escaping Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 5/6] telemetry: add escaping of strings in arrays Bruce Richardson
2022-06-23 16:42 ` [RFC PATCH 6/6] test/telemetry-json: add test case for escaping " Bruce Richardson
2022-06-23 19:04 ` [RFC PATCH 0/6] add json string escaping to telemetry Morten Brørup
2022-06-24 8:13 ` Bruce Richardson
2022-06-24 9:12 ` Morten Brørup
2022-06-24 9:17 ` Bruce Richardson
2022-06-24 10:22 ` Morten Brørup
2022-07-14 15:42 ` Morten Brørup
2022-07-25 16:38 ` Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 00/13] telemetry JSON escaping and other enhancements Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 01/13] test/telemetry_json: print success or failure per subtest Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 02/13] telemetry: fix escaping of invalid json characters Bruce Richardson
2022-07-26 18:25 ` Morten Brørup
2022-07-27 8:21 ` Bruce Richardson
2022-07-27 1:13 ` fengchengwen
2022-07-27 8:27 ` Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 03/13] test/telemetry_json: add test for string character escaping Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 04/13] telemetry: add escaping of strings in arrays Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 05/13] test/telemetry-json: add test for escaping " Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 06/13] telemetry: limit characters allowed in dictionary names Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 07/13] telemetry: add escaping of strings in dicts Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 08/13] test/telemetry_json: add test for string escaping in objects Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 09/13] telemetry: limit command characters Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 10/13] test/telemetry_data: refactor for maintainability Bruce Richardson
2022-08-23 12:33 ` Power, Ciara
2022-07-25 16:35 ` [PATCH v2 11/13] test/telemetry_data: add test cases for character escaping Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 12/13] telemetry: eliminate duplicate code for json output Bruce Richardson
2022-07-25 16:35 ` [PATCH v2 13/13] telemetry: make help command more helpful Bruce Richardson
2022-07-26 14:36 ` [PATCH v2 00/13] telemetry JSON escaping and other enhancements Morten Brørup
2022-07-27 1:51 ` fengchengwen
2022-07-27 9:12 ` Bruce Richardson
2022-07-27 9:49 ` Morten Brørup
2022-08-23 12:35 ` Power, Ciara
2022-09-09 9:35 ` [PATCH v3 " Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 01/13] telemetry: limit characters allowed in dictionary names Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 02/13] test/telemetry_json: print success or failure per subtest Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 03/13] telemetry: fix escaping of invalid json characters Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 04/13] test/telemetry_json: add test for string character escaping Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 05/13] telemetry: add escaping of strings in arrays Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 06/13] test/telemetry-json: add test for escaping " Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 07/13] telemetry: add escaping of strings in dicts Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 08/13] test/telemetry_json: add test for string escaping in objects Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 09/13] telemetry: limit command characters Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 10/13] test/telemetry_data: refactor for maintainability Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 11/13] test/telemetry_data: add test cases for character escaping Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 12/13] telemetry: eliminate duplicate code for json output Bruce Richardson
2022-09-09 9:35 ` [PATCH v3 13/13] telemetry: make help command more helpful Bruce Richardson
2022-09-13 0:35 ` [PATCH v3 00/13] telemetry JSON escaping and other enhancements fengchengwen
2022-09-26 11:52 ` David Marchand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98CBD80474FA8B44BF855DF32C47DC35D87174@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).