Re: [RFC PATCH 2/6] telemetry: fix escaping of invalid json characters

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 24 Jun 2022 13:29:46 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > 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.

I wrote it based on the model used by some Java library.
Other JSON libraries were more concerned with parsing JSON.