[libvirt] [PATCH] util: json: Remove yajl bits from virJSONValueToStr

Daniel P. Berrangé berrange at redhat.com
Tue Apr 3 11:23:13 UTC 2018


On Sat, Mar 31, 2018 at 11:01:15AM +0200, Peter Krempa wrote:
> Rather than depending on yajl bits for creating the JSON structure
> replace it by few virBuffer bits. This will make the JSON formatter
> libary agnostic.

I don't think this is a good idea as it means we have to reinvent the
wheel to ensure that we are correctly formatting & escaping JSON. This
patch gets escaping wrong which illustrates the point :-(

It has also discarded the utf8 validation that the old code did.


> +static void
> +virJSONValueToStringAddString(virBufferPtr buf,
> +                              virJSONValuePtr string)
> +{
> +    const char *t;
> +
> +    virBufferAddLit(buf, "\"");
> +
> +    for (t = string->data.string; *t; t++) {
> +        switch (*t) {
> +        case '"':
> +            virBufferAddLit(buf, "\\\"");
> +            break;
> +        case '\\':
> +            virBufferAddLit(buf, "\\\\");
> +            break;
> +        case '\n':
> +            virBufferAddLit(buf, "\\n");
> +            break;
> +        case '\t':
> +            virBufferAddLit(buf, "\\t");
> +            break;

Missing \r, \f, \b. Also missing hex escaping of
non printable characters.

> +        default:
> +            virBufferAdd(buf, t, 1);
> +            break;
> +        }
> +    }
> +
> +    virBufferAddLit(buf, "\"");
> +}
> +
> +
> +#define VIR_JSON_PRETTY_NEWLINE \
> +    if (pretty) \
> +        virBufferAddLit(buf, "\n")
> 
>  static int
>  virJSONValueToStringOne(virJSONValuePtr object,
> -                        yajl_gen g)
> +                        virBufferPtr buf,
> +                        bool pretty)
>  {
>      size_t i;
> 
> -    VIR_DEBUG("object=%p type=%d gen=%p", object, object->type, g);
> -
> -    switch (object->type) {
> +    switch ((virJSONType) object->type) {
>      case VIR_JSON_TYPE_OBJECT:
> -        if (yajl_gen_map_open(g) != yajl_gen_status_ok)
> -            return -1;
> +        virBufferAddLit(buf, "{");
> +        VIR_JSON_PRETTY_NEWLINE;
> +        virBufferAdjustIndent(buf, 2);
> +
>          for (i = 0; i < object->data.object.npairs; i++) {
> -            if (yajl_gen_string(g,
> -                                (unsigned char *)object->data.object.pairs[i].key,
> -                                strlen(object->data.object.pairs[i].key))
> -                                != yajl_gen_status_ok)
> -                return -1;
> -            if (virJSONValueToStringOne(object->data.object.pairs[i].value, g) < 0)
> +            virBufferStrcat(buf, "\"", object->data.object.pairs[i].key, "\":", NULL);

Missing escaping of key.

> +
> +            if (pretty)
> +                virBufferAddLit(buf, " ");
> +
> +            if (virJSONValueToStringOne(object->data.object.pairs[i].value,
> +                                        buf, pretty) < 0)
>                  return -1;
> +
> +            if (i != object->data.object.npairs - 1) {
> +                virBufferAddLit(buf, ",");
> +                VIR_JSON_PRETTY_NEWLINE;
> +            }
>          }
> -        if (yajl_gen_map_close(g) != yajl_gen_status_ok)
> -            return -1;
> +
> +        virBufferAdjustIndent(buf, -2);
> +        VIR_JSON_PRETTY_NEWLINE;
> +        virBufferAddLit(buf, "}");
>          break;
> +
>      case VIR_JSON_TYPE_ARRAY:
> -        if (yajl_gen_array_open(g) != yajl_gen_status_ok)
> -            return -1;
> +        virBufferAddLit(buf, "[");
> +        VIR_JSON_PRETTY_NEWLINE;
> +        virBufferAdjustIndent(buf, 2);
> +
>          for (i = 0; i < object->data.array.nvalues; i++) {
> -            if (virJSONValueToStringOne(object->data.array.values[i], g) < 0)
> +            if (virJSONValueToStringOne(object->data.array.values[i], buf, pretty) < 0)
>                  return -1;
> +
> +            if (i != object->data.array.nvalues - 1) {
> +                virBufferAddLit(buf, ",");
> +                VIR_JSON_PRETTY_NEWLINE;
> +            }
>          }
> -        if (yajl_gen_array_close(g) != yajl_gen_status_ok)
> -            return -1;
> +
> +        virBufferAdjustIndent(buf, -2);
> +        VIR_JSON_PRETTY_NEWLINE;
> +        virBufferAddLit(buf, "]");
>          break;
> 
>      case VIR_JSON_TYPE_STRING:
> -        if (yajl_gen_string(g, (unsigned char *)object->data.string,
> -                            strlen(object->data.string)) != yajl_gen_status_ok)
> -            return -1;
> +        virJSONValueToStringAddString(buf, object);
>          break;
> 
>      case VIR_JSON_TYPE_NUMBER:
> -        if (yajl_gen_number(g, object->data.number,
> -                            strlen(object->data.number)) != yajl_gen_status_ok)
> -            return -1;
> +        virBufferAdd(buf, object->data.number, -1);
>          break;
> 
>      case VIR_JSON_TYPE_BOOLEAN:
> -        if (yajl_gen_bool(g, object->data.boolean) != yajl_gen_status_ok)
> -            return -1;
> +        if (object->data.boolean)
> +            virBufferAddLit(buf, "true");
> +        else
> +            virBufferAddLit(buf, "false");
>          break;
> 
>      case VIR_JSON_TYPE_NULL:
> -        if (yajl_gen_null(g) != yajl_gen_status_ok)
> -            return -1;
> +        virBufferAddLit(buf, "null");
>          break;
> 
>      default:
> +        virReportEnumRangeError(virJSONType, object->type);
>          return -1;
>      }
> 
>      return 0;
>  }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list