[WIP] Replace YAJL with c-json for JSON parsing and generation by giuseppe · Pull Request #2088 · containers/crun

giuseppe · 2026-05-07T12:33:37Z

Switch from the unmaintained YAJL library to yyjson for all JSON operations. Update the build system (configure.ac, Makefile.am) to use yyjson, and update the libocispec submodule to the version with yyjson support.

Needs: containers/libocispec#171

giuseppe · 2026-05-07T12:33:55Z

alternative for #2068

packit-as-a-service · 2026-05-07T12:38:19Z

Ephemeral COPR build failed. @containers/packit-build please check.

gemini-code-assist

Code Review

This pull request replaces the YAJL JSON library with yyjson across the project, updating build configurations, dependencies, and source code for container status management, hooks, and language bindings. The review feedback focuses on improving robustness and diagnostic quality, specifically by adding safety checks for string conversions to prevent potential crashes, enhancing error reporting during JSON parsing, and ensuring consistent use of internal parsing wrappers.

gemini-code-assist · 2026-05-07T12:39:30Z

-    status->scope = tmp ? xstrdup (YAJL_GET_STRING (tmp)) : NULL;
+    tmp = json_object_object_get (doc, "scope");
+    status->scope = tmp ? xstrdup (json_object_get_string (tmp)) : NULL;
  }


yyjson_get_str returns NULL if the value is not a string. If tmp exists but is of a different type (e.g., an object or array), xstrdup(NULL) will be called, which typically results in a crash. An explicit check should be performed, similar to how rootfs or bundle are handled.

val = yyjson_get_str (tmp); status->scope = val ? xstrdup (val) : NULL;

gemini-code-assist · 2026-05-07T12:39:30Z

-    tmp = yajl_tree_get (tree, owner, yajl_t_string);
-    status->owner = tmp ? xstrdup (YAJL_GET_STRING (tmp)) : NULL;
+    tmp = json_object_object_get (doc, "owner");
+    status->owner = tmp ? xstrdup (json_object_get_string (tmp)) : NULL;


Similar to the scope field, yyjson_get_str might return NULL if the type is incorrect, leading to a potential crash in xstrdup. A safety check is required.

val = yyjson_get_str (tmp); status->owner = val ? xstrdup (val) : NULL;

gemini-code-assist · 2026-05-07T12:39:30Z

-    const char *external_descriptors[] = { "external_descriptors", NULL };
-    tmp = yajl_tree_get (tree, external_descriptors, yajl_t_string);
-    status->external_descriptors = tmp ? xstrdup (YAJL_GET_STRING (tmp)) : NULL;
+    tmp = json_object_object_get (doc, "external_descriptors");


Similar to the scope and owner fields, yyjson_get_str might return NULL if the type is incorrect, leading to a potential crash in xstrdup. A safety check is required.

val = yyjson_get_str (tmp); status->external_descriptors = val ? xstrdup (val) : NULL;

gemini-code-assist · 2026-05-07T12:39:30Z

+json_gen_error_to_crun_error (int status, libcrun_error_t *err)
 {
-  switch (yajl_status)
+  switch (status)
    {
-    case yajl_gen_status_ok:
+    case json_gen_status_ok:
      return 0;

-    case yajl_gen_keys_must_be_strings:
-      return crun_make_error (err, 0, "generate JSON document: gen keys must be strings");
-
-    case yajl_max_depth_exceeded:
-      return crun_make_error (err, 0, "generate JSON document: max depth exceeded");
-
-    case yajl_gen_in_error_state:
-      return crun_make_error (err, 0, "generate JSON document: complete JSON document generated");
-
-    case yajl_gen_generation_complete:
-      return crun_make_error (err, 0, "generate JSON document: called while in error state");
-
-    case yajl_gen_invalid_number:
-      return crun_make_error (err, 0, "generate JSON document: invalid number");
-
-    case yajl_gen_no_buf:
-      return crun_make_error (err, 0, "generate JSON document: no buffer provided");
-
-    case yajl_gen_invalid_string:
-      return crun_make_error (err, 0, "generate JSON document: invalid string");
-
    default:
-      return crun_make_error (err, 0, "generate JSON document");
+      return crun_make_error (err, 0, "generate JSON document: error");
    }
 }


The new implementation of json_gen_error_to_crun_error is significantly less descriptive than the previous yajl_error_to_crun_error. It loses specific error information such as depth exceeded, invalid numbers, or keys not being strings. If the json_gen_status enum provides these states, they should be mapped to specific error messages to maintain diagnostic quality.

gemini-code-assist · 2026-05-07T12:39:30Z

+  *out = json_tokener_parse (jsondata);
  if (*out == NULL)
-    return crun_make_error (err, 0, "cannot parse the data: `%s`", errbuf);
+    return crun_make_error (err, 0, "cannot parse the data");


When yyjson_read fails, it returns NULL, but the specific reason for the failure is lost. The previous yajl implementation provided an error buffer with details. You should use yyjson_read_opts with a yyjson_read_err pointer to capture and report the specific parsing error message and position.

yyjson_read_err read_err; *out = yyjson_read_opts (jsondata, strlen (jsondata), YYJSON_READ_NUMBER_AS_RAW, NULL, &read_err); if (*out == NULL) return crun_make_error (err, 0, "cannot parse the data: %s at position %zu", read_err.msg, read_err.pos);

gemini-code-assist · 2026-05-07T12:39:30Z

      return crun_make_error (err, 0, "`pid` missing in `%s`", file);
-    status->pid = strtoull (YAJL_GET_NUMBER (tmp), NULL, 10);
+    status->pid = (pid_t) json_object_get_int (tmp);
  }


yyjson_get_int returns a standard int. While pid_t is typically an int on Linux, using yyjson_get_sint (which returns int64_t) is safer to avoid potential overflow or truncation if the JSON contains a value that exceeds the range of a signed 32-bit integer.

status->pid = (pid_t) yyjson_get_sint (tmp);

gemini-code-assist · 2026-05-07T12:39:30Z

+  kconf->config_doc = json_tokener_parse (config);
+  if (kconf->config_doc == NULL)
+    return crun_make_error (err, 0, "cannot parse `%s`", KRUN_VM_FILE);


Instead of calling yyjson_read directly, consider using the project's parse_json_file wrapper. This ensures consistent error reporting into the libcrun_error_t structure used throughout the codebase.

ret = parse_json_file (&kconf->config_doc, config, NULL, err); if (UNLIKELY (ret < 0)) return ret;

gemini-code-assist · 2026-05-07T12:39:30Z

+    if (UNLIKELY (doc == NULL))
      return crun_make_error (err, 0, "cannot parse descriptors file `%s`", DESCRIPTORS_FILENAME);



For consistency with other parts of the library and to leverage centralized error handling, parse_json_file should be used here instead of a direct call to yyjson_read.

ret = parse_json_file (&doc, buffer, NULL, err); if (UNLIKELY (ret < 0)) return ret;

CentOS Stream 8 reached EOL in December 2021 and ships json-c 0.13.1 which is too old for the >= 0.14 requirement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2026-05-07T17:32:03Z

@jnovy this is the PR for using c-json

packit-as-a-service · 2026-05-07T17:45:19Z

TMT tests failed. @containers/packit-build please check.

Switch the JSON backend from yajl to json-c (>= 0.14), which is widely available as a system package. Update the libocispec submodule to the use-c-json branch which provides the json-c-based generated parsers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the use-c-json branch from 01d9d91 to 98e311b Compare May 7, 2026 12:35

giuseppe changed the title ~~[WIP] Replace YAJL with yyjson for JSON parsing and generation~~ [WIP] Replace YAJL with c-json for JSON parsing and generation May 7, 2026

gemini-code-assist Bot reviewed May 7, 2026

View reviewed changes

giuseppe force-pushed the use-c-json branch 2 times, most recently from 79f8f4d to a372db4 Compare May 7, 2026 13:14

tests: drop centos8-build CI job

5fc99f0

CentOS Stream 8 reached EOL in December 2021 and ships json-c 0.13.1 which is too old for the >= 0.14 requirement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the use-c-json branch 2 times, most recently from 49bd656 to 6f841bd Compare May 7, 2026 14:29

giuseppe force-pushed the use-c-json branch from 6f841bd to 3b19b18 Compare May 7, 2026 17:34

giuseppe force-pushed the use-c-json branch from 3b19b18 to 987ff92 Compare May 7, 2026 20:28

		if (UNLIKELY (doc == NULL))
		return crun_make_error (err, 0, "cannot parse descriptors file `%s`", DESCRIPTORS_FILENAME);

Conversation

giuseppe commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giuseppe commented May 7, 2026

Uh oh!

packit-as-a-service Bot commented May 7, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

giuseppe commented May 7, 2026

Uh oh!

packit-as-a-service Bot commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

giuseppe commented May 7, 2026 •

edited

Loading