Experimental Physics and
Industrial Control System

Zimoch Dirk via Core-talk <core-talk at aps.anl.gov> · Fri, 9 Feb 2024 09:46:26 +0000

Unfortunately, yail uses one flag (yajl_gen_json5) for two different things:
"Setting this flag allows YAJL to output the JSON5 representation of these
special numbers [NaN and infinity] instead of returning with an error, and to
emit map keys that are valid javascript identifiers without quotes."

It would be nicer to allow NaN/infinity _and_ quoted keys. This is still valid
JSON5, because one is allowed to quote keys even if they are valid identifiers.
And it is backward compatible to older JSON parsers as long as the data does not
contain NaN or infinity. 

In this mode, if the data contains NaN/infinity, it would fail in the parser,
not in the generator, making it the user's responsibility either to make sure no
NaN/infinity is contained in their data or to upgrade their parser to JSON5.

I think, a quite minimal invasive change would be to add another flag to
yajl_gen_option, e.g. yajl_gen_quoted_keys = 0x40, change yajl_gen_string() to
check for the flag:
if (g->flags & yajl_gen_json5 && !(g->flags & yajl_gen_quoted_keys)
And set that flag in jprint.cpp: 
#  if EPICS_VERSION_INT>=VERSION_INT(7,0,6,1)
        yajl_gen_config(handle, yajl_gen_json5|yajl_gen_quoted_keys,
(int)opts.json5);

Simpler but more invasive: change yajl_gen_status() to always quote by removing:
    if (g->flags & yajl_gen_json5 &&
        (g->state[g->depth] == yajl_gen_map_key ||
         g->state[g->depth] == yajl_gen_map_start) &&
        yajl_string_validate_identifier(str, len)) {
        /* No need to quote this key */
        g->print(g->ctx, (const char *) str, len);
    }
    else {

There are other subtle changes with JSON5 though which may confuse older
parsers: hex escape with \x, null bytes with \0, vertical tab with \v
To switch that off as well, change yajl_gen_status() even more:

        yajl_string_encode(g->print, g->ctx, str, len, g->flags &
yajl_gen_escape_solidus, false);

Dirk

On Wed, 2024-02-07 at 21:56 +0000, Johnson, Andrew N. via Core-talk wrote:
> If you follow the links in the PR that Michael posted you’ll see that this fix was to support NaN and Inf values which strict JSON doesn’t handle at all. 
> 
> I’d be fine with a PR that adds a flag to choose standard which you want, but adding Inf and NaN support to the JSON output would be a bit more work. They would have to be explicitly checked for and generated as strings. There are a couple of ways to do that, either make yajl_gen do it (in libCom/src/yajl) or have pvDataCPP add the checks. The latter would then work with 3.14.12.x and 3.15 versions of Base.
> 
> - Andrew
> 
> -- 
> Complicity is easy, Simplexity takes real work
> 
> > On Feb 7, 2024, at 12:54 PM, Michael Davidsaver via Core-talk <core-talk at aps.anl.gov> wrote:
> > 
> > On 2/7/24 02:28, NICOLE Remi via Core-talk wrote:
> > > Hello everyone,
> > > I planned on upgrading EPICS from 7.0.7 to 7.0.8 in our build system,
> > > but had some errors in our PV access tests.
> > > Apparently, in 7.0.8, the 'pvget my:pv -M json' now returns JSON5:
> > >     my:pv:name
> > > {value:42.0,alarm:{severity:0,status:0,message:""},display:{description
> > > :"My PV description"}}
> > > Where before in 7.0.7, it returnd standard JSON:
> > >     my:pv:name
> > > {"value":42.0,"alarm":{"severity":0,"status":0,"message":""},"display":
> > > {"description":"My PV description"}}
> > > Was this a conscious choice? I don't see the change in the release
> > > notes. I think for interoperability, standard JSON would be better.
> > 
> > This change was the result of:
> > 
> > https://github.com/epics-base/pvDataCPP/pull/93
> > 
> > Further changing to add "-M json5" seems reasonable
> > to me.  I will let Andrew say more.
> > 

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System