Add internal JSON backend layer, autotools integration and backend placeholders#53
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 121ccb9147
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| g->out.push_back('{'); | ||
| endToken(g); | ||
| g->stack.push_back({Frame::Map, true, true}); |
There was a problem hiding this comment.
Calling endToken() in yajl_gen_map_open (and similarly in array open) flips the parent map to expectingKey=true before the nested value is finished, then close flips it again; this leaves the parent in the wrong state after nested objects/arrays. In practice, serializing { "k": { ... }, "next": ... } emits wrong separators and the final yajl_gen_map_close/yajl_gen_get_buf path fails, which breaks JSON generation paths like Transaction::toJSON that rely on nested maps.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aff84034d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if test "x$JSON_CXX_BACKEND" = "xnone" && (test "x$with_json_cxx" = "xauto" || test "x$with_json_cxx" = "xglaze"); then | ||
| AC_CHECK_HEADER([glaze/glaze.hpp], [JSON_CXX_BACKEND="glaze"; JSON_CXX_BACKEND_SOURCE="system"]) | ||
| fi | ||
| if test "x$JSON_CXX_BACKEND" = "xnone" && test "x$with_json_prefer" != "xsystem" && test -f "${srcdir}/third_party/nlohmann_json/include/nlohmann/json.hpp"; then |
There was a problem hiding this comment.
When an explicit C++ backend is requested (for example --with-json-cxx=jsoncpp) and that backend is not found, this fallback still picks vendored nlohmann if --with-json-prefer is not system. Because third_party/nlohmann_json/include/nlohmann/json.hpp is shipped in-tree, configure succeeds instead of reaching the explicit-request failure path, so builds can silently use a different backend than the one the user asked for.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4380f2fdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbb0756333
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if test "x$JSON_C_BACKEND" != "xnone"; then | ||
| AC_DEFINE([WITH_JSON_SUPPORT], [1], [Define if a JSON backend is enabled for JSON generation]) | ||
| JSON_BACKEND_CFLAGS="$JSON_BACKEND_CFLAGS $JSON_C_BACKEND_DEFINE $JSON_CXX_BACKEND_DEFINE -DWITH_JSON_SUPPORT" |
There was a problem hiding this comment.
This gate disables WITH_JSON_SUPPORT whenever JSON_C_BACKEND resolves to none, but the new resolver never includes YAJL in its C-backend candidates. In environments that only have YAJL (a common existing setup), --with-json-c=auto now resolves to none, so Transaction::toJSON and ModSecurity::processContentOffset are compiled out and return the "not compiled with JSON support" path despite YAJL being present. That is a functional regression from the previous YAJL-based JSON generation behavior.
Useful? React with 👍 / 👎.
1 participant
Motivation
Description
--with-json-c,--with-json-cxx,--with-json-prefer,--with-json-mode,--enable-json-backend-debug) and export build variablesJSON_BACKEND_CFLAGS,JSON_BACKEND_LDADD,JSON_BACKEND_LDFLAGS,JSON_C_BACKEND,JSON_CXX_BACKEND, etc., with configure summary output.src/json/:json_backend.h/.cc(yajl-compatible writer shim), event-walkerjson_walk.h/.cc, error/value helpers and many backend registration placeholders undersrc/json/backends/(yyjson, json-c, jansson, cjson, nlohmann, jsoncpp, jsoncons, simdjson, glaze, yajl, etc.).JSON_BACKENDStosrc/Makefile.amand inject$(JSON_BACKEND_CFLAGS|LDFLAGS|LDADD)into build flags and libadd; update source callsites to useWITH_JSON_SUPPORTand the new internal API (replace direct YAJL includes/usages inmodsecurity.ccandtransaction.cc).docs/json-backends/*explaining autotools integration and per-backend notes,scripts/import-nlohmannjson.sh(andimport-json-c.shstub), vendoredthird_party/nlohmann_jsonsingle-header (recommended tag), and placeholderthird_party/*/README.mdfiles.Testing
./configurewith the new JSON options and inspected the configuration summary to confirm backends and preferences are reported; option parsing and validation messages are present.make) with the default configure selection to ensure the new sources integrate into the build system and compile; the build completed successfully.Codex Task