json: replace json-c with vendored nlohmann/json (closes #2366) - #2439
Merged
Conversation
ronaldtse
marked this pull request as draft
July 27, 2026 15:00
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2439 +/- ##
==========================================
+ Coverage 85.40% 85.44% +0.04%
==========================================
Files 126 125 -1
Lines 22966 22946 -20
==========================================
- Hits 19614 19607 -7
+ Misses 3352 3339 -13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ronaldtse
force-pushed
the
replace-json-c-with-nlohmann
branch
from
July 31, 2026 16:08
40a22a0 to
140cf58
Compare
ronaldtse
force-pushed
the
replace-json-c-with-nlohmann
branch
from
August 9, 2026 23:04
140cf58 to
40a22a0
Compare
ronaldtse
force-pushed
the
replace-json-c-with-nlohmann
branch
4 times, most recently
from
August 21, 2026 00:08
522436e to
13f2dfa
Compare
4 tasks
ni4
approved these changes
Aug 23, 2026
ni4
left a comment
ni4
left a comment
Contributor
There was a problem hiding this comment.
LGTM, quite progressive changes! Thanks.
Contributor
Author
|
@antonsviridenko could you take a look at this one when you have a moment? #2439 It replaces json-c with vendored nlohmann/json (closes #2366) and is the base of the remaining queue — two PRs are stacked on it, so merging this unblocks them. ni4 has already approved; it just needs a second review. CI is fully green (beyond the known oss-fuzz preview-leg reds), and the diff is mostly mechanical serialization rewrites in stream-dump/json-utils/fficli plus the vendored headers. Happy to walk through any part of it — the substantive decisions are in json-utils.h (the rnp::json helper API) and the removal of the json-c dependency from packaging/rnp-config. |
Contributor
|
@ronaldtse I'll try to take a look these weekends... |
ronaldtse
marked this pull request as ready for review
August 27, 2026 04:58
Vendor nlohmann/json v3.11.3 as a single-header under src/lib/nlohmann/nlohmann/ to remove the json-c runtime dependency (foundation phase of issue #2366). - src/lib/nlohmann/nlohmann/{json.hpp,json_fwd.hpp}: vendored (MIT, copyright Niels Lohmann 2013-2023, original preserved). - src/lib/CMakeLists.txt: add the include directory to librnp-obj. - CMakeLists.txt: document the choice and rationale. Add a new rnp::json::* API in json-utils.{h,cpp} over nlohmann::json: add/add_hex/array_add for writes, get_str/int/uint64/str_arr/get_obj for reads. Domain helpers (1 MiB hex cap, KeyID, Fingerprint) carry the same business rules as the legacy json-c implementation. The legacy json_object*-based API is retained as a bridge for the per-file migration that follows; removed in #12 once no consumer references json-c. A TODO.completion/ plan with 18 files documents the full migration. Build verified clean (configure + librnp + rnp + rnpkeys).
Migrate the first callsite in src/lib/rnp.cpp as a worked example of the strangler pattern. Legacy json_object* code is left in place elsewhere; rnp_supported_features no longer references json-c. - nlohmann::json features = nlohmann::json::array() replaces the json_object* + rnp::JSONObject RAII pair (nlohmann::json has value semantics, no RAII wrapper needed). - rnp::json::array_add replaces json_array_add for json-c types. - features.dump(4) replaces json_object_to_json_string_ext with JSON_C_TO_STRING_PRETTY (nlohmann defaults to compact; dump(4) matches json-c's 4-space indent). - features.dump(4).c_str() because ret_str_value takes const char*. Output format note: json-c escapes "/" by default and has minor formatting deltas; nlohmann::json does not. Tests parse JSON and assert on values, not bytes, so this is accepted. Tracked in TODO.completion/04-migrate-rnp-cpp.md for full-fidelity migration when needed. This is the first of 145 callsites in src/lib/rnp.cpp. Build verified clean.
Migrate the key-generation request parsing block in src/lib/rnp.cpp
(~140 lines, 7 functions):
- parse_preferences, parse_keygen_params, parse_protection,
parse_keygen_common_fields, parse_keygen_primary, parse_keygen_sub
- gen_json_grips, gen_json_primary_key, gen_json_subkey
- rnp_generate_key_json (top-level entry point)
Translation:
- json_object* → nlohmann::json & throughout (parameter type).
- json_get_str/int/uint64/str_arr → rnp::json::get_* (same names).
- json_object_object_del(jso, "k") → jso.erase("k").
- json_object_object_length(jso) → jso.empty() (inverted in the original
"no extra fields" check).
- json_get_obj → rnp::json::get_obj returns nlohmann::json*.
- json_object_object_foreach → structured bindings on jso.items().
- json_tokener_parse_verbose → nlohmann::json::parse with try/catch on
nlohmann::json::parse_error.
- json_object_to_json_string_ext(..., JSON_C_TO_STRING_PRETTY)
→ jso.dump(4).c_str() (nlohmann default compact; dump(4) matches
json-c's 4-space indent).
- rnp::JSONObject RAII wrappers removed (nlohmann::json has value
semantics).
- Nested object additions use direct assignment
(jso["primary"] = std::move(jsoprimary)) to avoid overload ambiguity
between add(... const std::string&) and add(... const char*).
- json_object_is_type(v, json_type_string) → v.is_string().
- json_object_get_string(v) → v.get_ref<const std::string&>().
The legacy json-c API in json-utils.h is still present as a bridge for
the remaining rnp.cpp callsites (dump_*, key_info_to_json, etc.) and
all other consumer files (fficli, rnpkeys, stream-dump, tests).
Build verified clean.
Migrate the import functions (~25 callsites): - add_key_status signature: json_object* keys → nlohmann::json &keys - add_sig_status signature: json_object* sigs → nlohmann::json &sigs - json_object_new_object → nlohmann::json::object() (value semantics) - json_object_new_array → nlohmann::json::array() - json_add → rnp::json::add throughout - json_array_add → rnp::json::array_add - json_object_object_add for nested objects → jso["key"] = std::move(obj) (avoids overload ambiguity with const std::string&) - rnp::JSONObject RAII wrappers removed - json_object_to_json_string_ext(jso, JSON_C_TO_STRING_PRETTY) → jso.dump(4).c_str() rnp_import_keys and rnp_import_signatures no longer reference json-c. Build verified clean.
Migrate the largest block in src/lib/rnp.cpp (~600 lines, ~80 callsites): - add_json_key_usage, add_json_key_flags: signatures now take nlohmann::json &; use nlohmann::json::array() + direct jso["usage"] assignment instead of jawrap.release() patterns. - add_json_mpis (variadic + key overload): nlohmann::json &. - add_json_sig_mpis: nlohmann::json &. - add_json_array_lookup: nlohmann::json &, nested array via auto &arr = jso[name] = nlohmann::json::array() (avoid copy-then-reference pitfall; nlohmann::json is value semantics). - add_json_user_prefs: nlohmann::json &. - add_json_subsig: nlohmann::json &. Nested objects/arrays use direct assignment to obtain a reference (jso["key"] = nlohmann::json::object()). Optional null fields (signer, mpis) use jso["key"] = nullptr to match json_object_object_add(jso, "k", NULL) semantics. - key_to_json: nlohmann::json &. Full body migrated. - rnp_key_to_json: nlohmann::json jso = nlohmann::json::object(), no RAII wrapper needed. - rnp_dump_src_to_json: still uses rnp::DumpContextJson (json_object*) — next step is to migrate stream-dump.cpp's DumpContextJson class. Output stability: dump(4) matches JSON_C_TO_STRING_PRETTY's 4-space indent. Slash escaping, Unicode, and trailing newline differences from json-c are accepted (tests parse JSON, not bytes). rnp_import_keys, rnp_import_signatures also migrated earlier in this branch (commit a516fb9). Build verified clean.
Migrate the entire packet-dump JSON layer (~700 lines, ~80 callsites): - DumpContextJson: now holds nlohmann::json* instead of json_object**. Constructor takes nlohmann::json*. - All method signatures: json_object* → nlohmann::json&. - dump_signature_subpackets returns nlohmann::json by value (moveable). - All json_add/json_array_add/json_add_hex calls → rnp::json::* equivalents. - Nested objects/arrays use direct assignment (jso["key"] = nlohmann::json::object()) for OCP: the type is decided by the call site, not by a helper overload set. - emplace_back(nlohmann::json::object()) replaces json_object_new_object + json_object_array_add pair. - rnp::JSONObject RAII wrappers and json_object_put cleanup gone (nlohmann::json is RAII). - json-c includes replaced with nlohmann/json.hpp. - stream-dump.h: same includes migration. - rnp.cpp rnp_dump_src_to_json: caller updated to pass nlohmann::json* and use jso.dump(4). rnp_dump_packets_to_json and rnp_key_packets_to_json now go through the new nlohmann::json pipeline end-to-end. Build verified clean.
- src/rnp/fficli.{h,cpp}: json.h → nlohmann/json.hpp.
- json_obj_get_str: json_object* → const nlohmann::json&. Uses
obj.contains() / obj.is_string() / get_ptr.
- cli_rnp_print_feature: parse with nlohmann::json::parse (try/catch),
iterate via jso[idx].get_ref. Removed json_object_put cleanup
(nlohmann::json is RAII).
- src/rnpkeys/rnpkeys.cpp: import_keys() and import_sigs() parse
results with nlohmann::json::parse, iterate via range-for, use
jsoninfo.value("public", "") for default-on-missing extraction.
Build verified clean.
- check_json_field_str/int/bool/pkt_type: now take const nlohmann::json& instead of json_object*. Use obj.contains() / obj.is_string() etc. - jso_get_field: returns const nlohmann::json* out param. - support.h: declaration updates. - No more json_object_* calls in support.cpp. Build verified clean (librnp + rnp + rnpkeys targets).
Applied a translator script (/tmp/json_translate.py, attached inline below)
to the 8 test files that use json-c:
src/tests/{ffi,ffi-key,ffi-key-sig,ffi-enc,s2k-iterations,streams,pipe,
partial-length}.cpp
Mechanical translation table applied:
- json_object* → nlohmann::json (value)
- json_object_new_object/array() → nlohmann::json::object()/array()
- json_tokener_parse(s) → nlohmann::json::parse(s)
- json_object_is_type(v, T) → v.is_T()
- json_object_get_string/int/int64/boolean(v) → v.get_*)>() etc.
- json_object_array_length(v) → v.size()
- json_object_array_get_idx(arr, i) → arr.at(i)
- json_object_array_add(arr, v) → arr.push_back(v)
- json_object_object_del(obj, k) → obj.erase(k)
- json_object_object_add(obj, k, v) → obj[k] = std::move(v)
- json_object_put(v) → /* removed */
- json_object_to_json_string_ext(s, FLAG) → s.dump() / s.dump(4)
- #include "json*.h" → #include <nlohmann/json.hpp>
KNOWN REMAINING MANUAL FIXUPS (build still fails on tests):
- json_object_object_get_ex(obj, "k", &val) — needs translation to
obj.contains("k") + separate val = &obj["k"] assignment. The script
leaves these for manual review because they require restructuring the
surrounding code.
- *ptr.member() patterns where ptr was a json_object* output parameter —
need ptr->member() instead.
- Some json_object_new_string/int/bool inside add() calls need direct
value conversion.
Production code (librnp, rnp, rnpkeys) is fully migrated and builds clean.
The test target does not build yet — finishing the migration is tracked
in TODO.completion/05-07.
This is the big-bang rewrite requested; the build will be red on tests
until the remaining manual fixups land.
After migrating all production source to nlohmann::json, remove the json-c dependency from build, packaging, and CI: Source cleanup: - src/lib/json-utils.h: legacy json_object* API deleted. Only the new rnp::json::* API over nlohmann::json remains. - src/lib/json-utils.cpp: legacy implementation deleted. - src/lib/rnp.cpp: drop json_object.h/json.h includes (already migrated). - src/lib/ffi-priv-types.h: same. CMake: - src/lib/CMakeLists.txt: drop find_package(JSON-C), drop JSON-C::JSON-C from librnp-obj link list, drop FindJSON-C.cmake from installed find modules. - src/rnp/CMakeLists.txt: drop find_package(JSON-C) and JSON-C::JSON-C link/include from rnp executable. - src/rnpkeys/CMakeLists.txt: same for rnpkeys executable. - cmake/rnp-config.cmake.in: drop find_dependency(JSON-C 0.11). - cmake/Modules/FindJSON-C.cmake: deleted. Packaging: - cmake/packaging.cmake: drop json-c from CPACK_FREEBSD_PACKAGE_DEPS. CI: - .github/workflows/ubuntu.yml, codeql.yml, coverity.yml, windows-msys2.yml, windows-native.yml: drop libjson-c-dev / json-c package installs. - .github/workflows/centos-and-fedora.yml: rename "Setup json-c" step. - ci/tests/pk-tests.sh: drop pkg_check_modules(JSONC ...) block. - ci/tests/downstream-consumer.sh: drop -ljson-c from link line, drop json-c from CMAKE_DEPS_PREFIX_PATH example. Production build verified clean with json-c NOT installed in pkg-config lookup (cmake configure + librnp + rnp + rnpkeys). Tests (src/tests/*.cpp) still reference json-c in the json_object_object_get_ex patterns that the mechanical translator script left for manual fixup. Test target will not build until those are migrated; tracked in TODO.completion/05-07.
Final pass on test files: applied multiple targeted translators to fix
the remaining mechanical patterns the first pass missed. Build is now
fully green for librnp + rnp + rnpkeys + rnp_tests.
Patterns fixed in this pass:
- json_object_object_get_ex(obj, "k", &val) →
(obj.contains("k") ? (val = obj["k"], true) : false) for value-typed
output, or (val = &(*obj)["k"], true) for pointer-typed output.
- *ptr.is_X() → ptr->is_X() for json_object** out-params now typed
nlohmann::json*.
- X.get_ref<const std::string &>() → append .c_str() when used as
const char* (strcmp/strdup/printf etc).
- json_object_get_X(get_json_obj(jso, "k")) — nested function-call arg
handled via balanced-paren matcher.
- json_object_array_get_idx(arr, i) → arr.at(i), also with nested parens.
- json_object_object_add/length → obj[k] = v / obj.size().
- *jso = NULL → *jso = nlohmann::json() (default-constructed).
- json_object_new_string(s) inside add() → s.
Remaining references to "json_object_put" in tests are inert comments
marking where the old RAII cleanup lived (now no-op for nlohmann::json).
src/lib/, src/librepgp/, src/rnp/, src/rnpkeys/, src/tests/ are all
json-c-free. cmake/, .github/workflows/, ci/ no longer reference json-c.
FindJSON-C.cmake is deleted.
The variadic add_json_mpis(nlohmann::json&, ...) fails to compile on MSVC because va_start requires the last named parameter to not be a reference. Switch to nlohmann::json* and update all 12 call sites.
After switching from json-c to nlohmann/json, fix five categories of behavioral deltas that the test suite caught: 1. JSON object key ordering. nlohmann::json uses std::map (alphabetical sort); json-c preserved insertion order. Switched all uses of `nlohmann::json` to `nlohmann::ordered_json` so packet-dump output matches the on-the-wire order tests expect. 2. Output format. json-c's JSON_C_TO_STRING_PRETTY uses 2-space indent and `":"` (no space after colon); nlohmann::json::dump uses `": "`. Added `rnp::json::dump_pretty()` helper that produces json-c-compatible output (2-space indent, structural `":` only). All FFI entry points now use it. Fixes substring-assertion tests like `strstr(json, "\"contents\":\"PGP\"")`. 3. `del` default on get_*. Legacy `json_get_str/int/uint64/str_arr` defaulted `del=true` (erase field after read); my new API had `del=false`. The parse_keygen_* "no unknown fields" check (`jso.empty()`) silently broke because consumed fields weren't removed. Restored `del=true` default. 4. Value-vs-reference semantics. json_object* was a borrowed pointer; `nlohmann::json` is value-semantic. Several call sites were copying when they needed a reference (e.g. `jsores["keys"] = jsokeys; ... add_key_status(jsokeys, ...)` — the local array wasn't the one inside jsores). Fixed `rnp_import_keys`, `rnp_import_signatures` to use `auto &X = jso[key]` references so mutations propagate. 5. Variadic `add_json_mpis` segfault. The original took `json_object*` as the named parameter; the nlohmann::json& version failed MSVC (va_start reference-arg check) and the nlohmann::json* version crashed on arm64 due to va_arg type-punning with const-qualified pointers. Replaced variadic with std::initializer_list<std::pair>. 6. Truthy checks on nlohmann::json values. `if (!jso)` does implicit bool conversion which throws type_error.302 on non-bool types. Translator script updated to convert `if(!X)`/assert_non_null(X) to `X.is_null()` checks when X is a nlohmann::ordered_json value (scope-aware). 7. `return NULL` in helpers returning nlohmann::ordered_json by value constructed an integer-typed json (NULL is often `0`), not a null- typed one. Changed to `return nlohmann::ordered_json()`. 8. Examples (`src/examples/generate.c`) used single-quoted JSON which json-c tolerated but nlohmann::json rejects. Replaced with proper double-quoted/escaped JSON. Test suite: 275/275 passing locally (was 258/275 before this commit).
The json-c-devel install was needed when pk-tests.sh checked for json-c via pkg_check_modules. That code was removed in this branch (json-c → nlohmann/json migration), so the install step is no longer needed. Replacing it with botan3-devel was wrong: CentOS 9 doesn't ship botan3-devel in its default repos.
ronaldtse
force-pushed
the
replace-json-c-with-nlohmann
branch
from
August 29, 2026 01:42
13f2dfa to
871fe76
Compare
Contributor
Author
|
@antonsviridenko the full queue is now green and ready — here's the situation: All 10 PRs are CI-green, rebased on main, and approved by ni4. They just need your second review to merge. One dependency to know about: #2439 (json-c → nlohmann/json, closes #2366) is the root of a small stack — once it lands, these two auto-retarget to main and are immediately mergeable (they're already approved by ni4 and green):
So if you review 2439 first and it merges, you can approve+merge those two right after without waiting. The remaining 7 are all independent, any order:
Happy to walk through any of them — the diffs are mostly clean now that the review-cycle bugs (protect_ex security flaw, FIPS test assumptions, FFI include, etc.) are all fixed. |
Contributor
|
otherwise LGTM |
Copy-paste error from the json-c removal: the set/unset pair that restores CMAKE_MODULE_PATH after the find-module dependencies appeared twice. The second copy was a no-op (the variable was already unset) but confusing. Pointed out by @antonsviridenko.
Contributor
Author
|
Thanks @antonsviridenko for the review — good catch on the duplicated |
Contributor
Author
|
@antonsviridenko the fix for your comment is pushed (4de0fae), the thread is resolved, and CI is fully green. Just need you to click "Approve" to submit your review — then we can merge this and the two stacked PRs behind it. Thanks! |
antonsviridenko
approved these changes
Aug 30, 2026
ni4
pushed a commit
that referenced
this pull request
Sep 1, 2026
Adds aarch64-linux-android cross-compile smoke tests for both the Botan and OpenSSL backends, mirroring the OHOS workflow (#2438) but with key additions: - Tests the OpenSSL backend, which the OHOS workflow does not cover; this is where the CMAKE_DL_LIBS linkage gap (#2473) manifested - Sets CMAKE_CROSSCOMPILING_EMULATOR=qemu-aarch64-static so the findopensslfeatures probe actually runs under emulation, catching the '-static' + dlopen linkage class of bugs - Post-nlohmann (#2439): no json-c dependency to cross-build - No SDK geo-restriction (unlike OHOS): the NDK downloads freely The NDK toolchain file, qemu-user-static, and cacheable dependency builds keep the workflow simple. Each backend is a separate matrix leg so a failure in one doesn't mask the other.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the json-c runtime dependency with nlohmann/json (vendored single-header) to simplify installation and use a more idiomatic C++ JSON API.
Closes #2366.
What changed
src/lib/nlohmann/nlohmann/{json.hpp,json_fwd.hpp}(MIT, copyright Niels Lohmann preserved). ~900KB, no install dependency.rnp::json::*helper API insrc/lib/json-utils.{h,cpp}overnlohmann::json. Free-function overloads for the domain concerns (hex blobs with 1 MiB cap, KeyID/Fingerprint encoding, type-safe gets with optional field removal). The legacyjson_object*API andrnp::JSONObjectRAII shim are deleted.nlohmann::json:src/lib/rnp.cpp—rnp_supported_features,rnp_generate_key_json(request parsing),rnp_import_keys,rnp_import_signatures,key_to_json,rnp_key_to_json,dump_*_to_json, helpers (add_json_*,add_json_mpis,add_json_sig_mpis,add_json_array_lookup,add_json_user_prefs,add_json_subsig).src/librepgp/stream-dump.{cpp,h}—DumpContextJsonclass.src/rnp/fficli.{cpp,h},src/rnpkeys/rnpkeys.cpp— CLI consumers.src/lib/ffi-priv-types.h— include swap.nlohmann::json(8 files).cmake/Modules/FindJSON-C.cmakedeleted.cmake/rnp-config.cmake.in:find_dependency(JSON-C 0.11)removed.cmake/packaging.cmake: FreeBSD deps updated (dropsjson-c).src/lib/CMakeLists.txt,src/rnp/CMakeLists.txt,src/rnpkeys/CMakeLists.txt,src/tests/CMakeLists.txt: droppedfind_package(JSON-C),JSON-C::JSON-Clink, and include dirs.CMakeLists.txt: documented nlohmann/json vendoring choice.libjson-c-dev/json-c:p/json-c/json-c-develfromubuntu.yml,codeql.yml,coverity.yml,windows-msys2.yml,windows-native.yml,centos-and-fedora.yml.ci/tests/pk-tests.sh: droppedpkg_check_modules(JSONC ...)block.ci/tests/downstream-consumer.sh: dropped-ljson-cfrom link line, updated docs.Output format note
json-c'sJSON_C_TO_STRING_PRETTYandnlohmann::json::dump(4)both use 4-space indent. Minor differences exist in slash escaping (json-c escapes/; nlohmann does not) and number formatting. Tests parse JSON and assert on values, so these are accepted. CLI golden-output (if any) may need fixture updates.Build status
cmake -B build -S .configures clean without json-c installed.cmake --build buildbuildslibrnp,rnp,rnpkeys,rnp_testscleanly.TODO.completion/STATUS.md). Examples:test_ffi_supported_features:[json.exception.type_error.302] type must be boolean, but is arraytest_ffi_key_to_json:get_json_obj(jso, "primary key grip")returns nullptrThe tests are catching real behavioral deltas from the migration (not just mechanical issues). Investigation needed in
rnp.cpp/stream-dump.cpp/test helpers — likely JSON shape differences vs the priorjson-coutput. Marking the PR as draft until these are resolved.TODO.completion/
The branch includes a
TODO.completion/directory with 18 planning files (00-overview.mdthrough17-final-cleanup.md), aSTATUS.mdsummarizing current state, and the_json_translate*.pyscripts used for the mechanical migration of test files. These can be deleted before merge if undesired, or kept as reference for future similar migrations.Test plan
ctestrun on this branch — identify all failures beyond the 7 spot-checkedtest_ffi_supported_featurestype_error (likely a get-on-array bug)test_ffi_key_to_jsonmissing "primary key grip" fieldrnp --list-packetsoutput before/after on a fixture keyringrnpkeys --listoutput before/afterTODO.completion/if not desired for the merged history