Re: rpki-client json.c add json_do_string()

2023-05-02 Thread Claudio Jeker
On Tue, May 02, 2023 at 09:34:43AM -0600, Todd C. Miller wrote:
> On Tue, 02 May 2023 14:13:27 +0200, Claudio Jeker wrote:
> 
> > Add a json_do_string() a function to print a JSON string.
> > This function does the needed encoding of control chars and escape chars.
> > I skipped the optional encoding of the forward slash (/) since this is
> > only needed if the json output is embedded in HTML/SGML/XML.
> > People putting JSON into such documents need to pass this through an extra
> > step.
> 
> Unless you can guarantee that other control characters cannot occur
> you probably want to output them in unicode hex form.  For example,
> something like:
> 
> default:
>   const char hex[] = "0123456789abcdef";
>   if (iscntrl(c)) {
>   /* Escape control characters like \u */
>   if (!eb)
>   eb = fprintf(jsonfh, "\\u00%c%c", hex[c >> 4],
>   hex[c & 0x0f]) < 0;
>   }

I think this is not really needed. All strings are previously checked for
any bad characters. In general I think covering more than \n, \r and \t makes
little sense (but I added the other ones just for completeness).
Maybe the best is to check but error out for iscntrl(c).
 
> In this case you probably want to assign c as an unsigned char.
> E.g.
> 
> while ((c = (unsigned char)*v++) != '\0') {
>   ...
> }
> 
> Or better yet just declare c as unsigned char instead of int.

Good point. I guess it is better to use unsigned char here.

-- 
:wq Claudio



Re: rpki-client json.c add json_do_string()

2023-05-02 Thread Todd C . Miller
On Tue, 02 May 2023 14:13:27 +0200, Claudio Jeker wrote:

> Add a json_do_string() a function to print a JSON string.
> This function does the needed encoding of control chars and escape chars.
> I skipped the optional encoding of the forward slash (/) since this is
> only needed if the json output is embedded in HTML/SGML/XML.
> People putting JSON into such documents need to pass this through an extra
> step.

Unless you can guarantee that other control characters cannot occur
you probably want to output them in unicode hex form.  For example,
something like:

default:
const char hex[] = "0123456789abcdef";
if (iscntrl(c)) {
/* Escape control characters like \u */
if (!eb)
eb = fprintf(jsonfh, "\\u00%c%c", hex[c >> 4],
hex[c & 0x0f]) < 0;
}

In this case you probably want to assign c as an unsigned char.
E.g.

while ((c = (unsigned char)*v++) != '\0') {
...
}

Or better yet just declare c as unsigned char instead of int.

 - todd



Re: rpki-client json.c add json_do_string()

2023-05-02 Thread Theo Buehler
On Tue, May 02, 2023 at 02:41:52PM +0200, Claudio Jeker wrote:
> On Tue, May 02, 2023 at 02:29:20PM +0200, Theo Buehler wrote:
> > On Tue, May 02, 2023 at 02:13:27PM +0200, Claudio Jeker wrote:
> > > Add a json_do_string() a function to print a JSON string.
> > > This function does the needed encoding of control chars and escape chars.
> > > I skipped the optional encoding of the forward slash (/) since this is
> > > only needed if the json output is embedded in HTML/SGML/XML.
> > > People putting JSON into such documents need to pass this through an extra
> > > step.
> > > 
> > > This implements json_do_printf() now as a wrapper around json_do_string().
> > > All users of json_do_printf(name, "%s", value) can be converted to
> > > json_do_string(). I will do this is a subsequent step.
> > 
> > I'm ok with this. Some comments below.
> 
> How about this version? By moving the !eb check into the while loop the
> if checks fall away and so it is just one fprintf() per case. I think
> trying to optimize this further is not that benefitial. I wonder if the
> compiler wont do that better anyway.

Agreed. ok



Re: rpki-client json.c add json_do_string()

2023-05-02 Thread Claudio Jeker
On Tue, May 02, 2023 at 02:29:20PM +0200, Theo Buehler wrote:
> On Tue, May 02, 2023 at 02:13:27PM +0200, Claudio Jeker wrote:
> > Add a json_do_string() a function to print a JSON string.
> > This function does the needed encoding of control chars and escape chars.
> > I skipped the optional encoding of the forward slash (/) since this is
> > only needed if the json output is embedded in HTML/SGML/XML.
> > People putting JSON into such documents need to pass this through an extra
> > step.
> > 
> > This implements json_do_printf() now as a wrapper around json_do_string().
> > All users of json_do_printf(name, "%s", value) can be converted to
> > json_do_string(). I will do this is a subsequent step.
> 
> I'm ok with this. Some comments below.

How about this version? By moving the !eb check into the while loop the
if checks fall away and so it is just one fprintf() per case. I think
trying to optimize this further is not that benefitial. I wonder if the
compiler wont do that better anyway.

-- 
:wq Claudio

Index: json.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/json.c,v
retrieving revision 1.1
diff -u -p -r1.1 json.c
--- json.c  27 Apr 2023 07:57:25 -  1.1
+++ json.c  2 May 2023 12:39:36 -
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "json.h"
@@ -179,16 +180,56 @@ void
 json_do_printf(const char *name, const char *fmt, ...)
 {
va_list ap;
+   char *str;
 
-   do_comma_indent();
+   va_start(ap, fmt);
+   if (!eb) {
+   if (vasprintf(, fmt, ap) == -1)
+   errx(1, "json printf failed");
+   json_do_string(name, str);
+   free(str);
+   }
+   va_end(ap);
+}
 
+void
+json_do_string(const char *name, const char *v)
+{
+   int c;
+
+   do_comma_indent();
do_name(name);
if (!eb)
eb = fprintf(jsonfh, "\"") < 0;
-   va_start(ap, fmt);
-   if (!eb)
-   eb = vfprintf(jsonfh, fmt, ap) < 0;
-   va_end(ap);
+   while ((c = *v++) != '\0' && !eb) {
+   /* skip escaping '/' since our use case does not require it */
+   switch(c) {
+   case '"':
+   eb = fprintf(jsonfh, "\\\"") < 0;
+   break;
+   case '\\':
+   eb = fprintf(jsonfh, "") < 0;
+   break;
+   case '\b':
+   eb = fprintf(jsonfh, "\\b") < 0;
+   break;
+   case '\f':
+   eb = fprintf(jsonfh, "\\f") < 0;
+   break;
+   case '\n':
+   eb = fprintf(jsonfh, "\\n") < 0;
+   break;
+   case '\r':
+   eb = fprintf(jsonfh, "\\r") < 0;
+   break;
+   case '\t':
+   eb = fprintf(jsonfh, "\\t") < 0;
+   break;
+   default:
+   eb = putc(c, jsonfh) == EOF;
+   break;
+   }
+   }
if (!eb)
eb = fprintf(jsonfh, "\"") < 0;
 }
Index: json.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/json.h,v
retrieving revision 1.1
diff -u -p -r1.1 json.h
--- json.h  27 Apr 2023 07:57:25 -  1.1
+++ json.h  30 Apr 2023 15:11:36 -
@@ -26,6 +26,7 @@ void  json_do_object(const char *);
 void   json_do_end(void);
 void   json_do_printf(const char *, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
+void   json_do_string(const char *, const char *);
 void   json_do_hexdump(const char *, void *, size_t);
 void   json_do_bool(const char *, int);
 void   json_do_uint(const char *, unsigned long long);



Re: rpki-client json.c add json_do_string()

2023-05-02 Thread Theo Buehler
On Tue, May 02, 2023 at 02:13:27PM +0200, Claudio Jeker wrote:
> Add a json_do_string() a function to print a JSON string.
> This function does the needed encoding of control chars and escape chars.
> I skipped the optional encoding of the forward slash (/) since this is
> only needed if the json output is embedded in HTML/SGML/XML.
> People putting JSON into such documents need to pass this through an extra
> step.
> 
> This implements json_do_printf() now as a wrapper around json_do_string().
> All users of json_do_printf(name, "%s", value) can be converted to
> json_do_string(). I will do this is a subsequent step.

I'm ok with this. Some comments below.

> -- 
> :wq Claudio
> 
> Index: json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/json.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 json.c
> --- json.c27 Apr 2023 07:57:25 -  1.1
> +++ json.c1 May 2023 20:07:26 -
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

I'd move this below stdio.h

>  #include 
>  #include 
>  
> @@ -179,16 +180,64 @@ void
>  json_do_printf(const char *name, const char *fmt, ...)
>  {
>   va_list ap;
> + char *str;
>  
> - do_comma_indent();
> + va_start(ap, fmt);
> + if (!eb) {
> + if (vasprintf(, fmt, ap) == -1)
> + errx(1, "json printf failed");
> + json_do_string(name, str);
> + free(str);
> + }
> + va_end(ap);
> +}
>  
> +void
> +json_do_string(const char *name, const char *v)
> +{
> + int c;
> +
> + do_comma_indent();
>   do_name(name);
>   if (!eb)
>   eb = fprintf(jsonfh, "\"") < 0;
> - va_start(ap, fmt);
> - if (!eb)
> - eb = vfprintf(jsonfh, fmt, ap) < 0;
> - va_end(ap);
> + while ((c = *v++) != '\0') {

Should this include && !eb?


> + /* skip escaping '/' since our use case does not require it */
> + switch(c) {
> + case '"':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\\"") < 0;

It's fine as it is, but the repetition could be avoided by assigning the
escaped string or c to a temporary variable and do the printing after
the switch. Your call.

> + break;
> + case '\\':
> + if (!eb)
> + eb = fprintf(jsonfh, "") < 0;
> + break;
> + case '\b':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\b") < 0;
> + break;
> + case '\f':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\f") < 0;
> + break;
> + case '\n':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\n") < 0;
> + break;
> + case '\r':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\r") < 0;
> + break;
> + case '\t':
> + if (!eb)
> + eb = fprintf(jsonfh, "\\t") < 0;
> + break;
> + default:
> + if (!eb)
> + eb = putc(c, jsonfh) == EOF;
> + break;
> + }
> + }
>   if (!eb)
>   eb = fprintf(jsonfh, "\"") < 0;
>  }
> Index: json.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/json.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 json.h
> --- json.h27 Apr 2023 07:57:25 -  1.1
> +++ json.h30 Apr 2023 15:11:36 -
> @@ -26,6 +26,7 @@ voidjson_do_object(const char *);
>  void json_do_end(void);
>  void json_do_printf(const char *, const char *, ...)
>   __attribute__((__format__ (printf, 2, 3)));
> +void json_do_string(const char *, const char *);
>  void json_do_hexdump(const char *, void *, size_t);
>  void json_do_bool(const char *, int);
>  void json_do_uint(const char *, unsigned long long);
> 



rpki-client json.c add json_do_string()

2023-05-02 Thread Claudio Jeker
Add a json_do_string() a function to print a JSON string.
This function does the needed encoding of control chars and escape chars.
I skipped the optional encoding of the forward slash (/) since this is
only needed if the json output is embedded in HTML/SGML/XML.
People putting JSON into such documents need to pass this through an extra
step.

This implements json_do_printf() now as a wrapper around json_do_string().
All users of json_do_printf(name, "%s", value) can be converted to
json_do_string(). I will do this is a subsequent step.

-- 
:wq Claudio

Index: json.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/json.c,v
retrieving revision 1.1
diff -u -p -r1.1 json.c
--- json.c  27 Apr 2023 07:57:25 -  1.1
+++ json.c  1 May 2023 20:07:26 -
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -179,16 +180,64 @@ void
 json_do_printf(const char *name, const char *fmt, ...)
 {
va_list ap;
+   char *str;
 
-   do_comma_indent();
+   va_start(ap, fmt);
+   if (!eb) {
+   if (vasprintf(, fmt, ap) == -1)
+   errx(1, "json printf failed");
+   json_do_string(name, str);
+   free(str);
+   }
+   va_end(ap);
+}
 
+void
+json_do_string(const char *name, const char *v)
+{
+   int c;
+
+   do_comma_indent();
do_name(name);
if (!eb)
eb = fprintf(jsonfh, "\"") < 0;
-   va_start(ap, fmt);
-   if (!eb)
-   eb = vfprintf(jsonfh, fmt, ap) < 0;
-   va_end(ap);
+   while ((c = *v++) != '\0') {
+   /* skip escaping '/' since our use case does not require it */
+   switch(c) {
+   case '"':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\\"") < 0;
+   break;
+   case '\\':
+   if (!eb)
+   eb = fprintf(jsonfh, "") < 0;
+   break;
+   case '\b':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\b") < 0;
+   break;
+   case '\f':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\f") < 0;
+   break;
+   case '\n':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\n") < 0;
+   break;
+   case '\r':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\r") < 0;
+   break;
+   case '\t':
+   if (!eb)
+   eb = fprintf(jsonfh, "\\t") < 0;
+   break;
+   default:
+   if (!eb)
+   eb = putc(c, jsonfh) == EOF;
+   break;
+   }
+   }
if (!eb)
eb = fprintf(jsonfh, "\"") < 0;
 }
Index: json.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/json.h,v
retrieving revision 1.1
diff -u -p -r1.1 json.h
--- json.h  27 Apr 2023 07:57:25 -  1.1
+++ json.h  30 Apr 2023 15:11:36 -
@@ -26,6 +26,7 @@ void  json_do_object(const char *);
 void   json_do_end(void);
 void   json_do_printf(const char *, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
+void   json_do_string(const char *, const char *);
 void   json_do_hexdump(const char *, void *, size_t);
 void   json_do_bool(const char *, int);
 void   json_do_uint(const char *, unsigned long long);