Render, copy and serialise a JSON number from the text it was parsed from by djbclark · Pull Request #5 · djbclark/libntech

djbclark · 2026-08-17T11:52:51Z

Fixes #4 and #2. They are one PR because they are not independently landable — see "Why these are stacked" below.

The mistake, once

Every defect here is a shape of the same thing: a JSON number was rebuilt from a C numeric type instead of being taken as the text the parser already kept. JsonWriteCompact() always emitted the stored lexeme; everything else round-tripped through long or double and disagreed with it. The fix is to render, copy and serialise from one source of truth.

Severity — this takes a host off its policy at policy load

Not at render time. A policy whose only content is a data var promise is sufficient on stock 3.27.1:

body common control { bundlesequence => { "test" }; }
bundle agent test
{
  vars:
      "d" data => readjson("$(sys.policy_entry_dirname)/numbers.json", 100000);
  reports:
      "loaded";
}

with 9223372036854775808 anywhere in numbers.json. reports: never references d; there is no iteration and no mustache. cf-promises exits inside LoadPolicy, so cf-agent cannot confirm promises and falls back to failsafe.

StringToLongExitOnError          <- "9223372036854775808"
JsonCopy                          (JsonPrimitiveCopy, inlined)
JsonObjectCopy
RvalNewRewriter
VerifyVarPromise
ExpandPromise
BundleResolvePromiseType
PolicyResolve
LoadPolicy
main                              (cf-promises)

Storing a JSON container as a CFEngine variable deep-copies it, so JsonPrimitiveCopy() runs on every data variable at load. Augments reach the same sink with no user policy at all — a def.json of {"vars":{"danger":9223372036854775808}} dies in LoadAugmentsFiles during context discovery, before policy is parsed. So does host_specific.json.

A cf-promises built from cfengine/core's corresponding fixes on a stock libntech still dies, because JsonPrimitiveCopy() lives here.

The quieter half

JsonIntegerCreate() takes an int while JsonPrimitiveGetAsInteger() returns a long, so copying silently narrowed anything between them — at variable storage, before any render, with no error and no log:

JSON number	stored on stock
`2000000000000`	`-1454759936`
`9223372036854775807`	`-1`
`1786965915908` (epoch milliseconds)	`259520772`

Millisecond timestamps have exceeded INT_MAX since 2001, so this needs no oversized value and no unusual input — a readjson() of ordinary telemetry loads, validates, does not failsafe, and uses a different number.

Why these are stacked, and why the bottom two cannot be dropped

bf57367's reclassification of exponent numbers as REAL routes 1e-8, 2e0 and 1e400 onto the real rendering path. Without the two commits beneath it that path is still "%.2f":

value	without the bottom two	as submitted
`0.00049`	`0.00`	`0.00049`
`1e-8`	`0.00`	`1e-8`
`1.5e3`	`1500.00`	`1.5e3`
`1e400`	`inf`	`1e400`

Stock never emitted inf — with no decimal point 1e400 is misclassified INTEGER and terminates the process instead. inf is therefore a defect the upper commits would introduce on their own, and the lower ones remove. Merging only part of this series is worse than merging none of it.

Happy to squash to a single commit at merge if you prefer; they are separated for review, not because the boundaries matter afterwards.

Behaviour changes you will want to look at

Every real renders from its parsed text, not only the broken ones. 0.5 renders as 0.5 rather than 0.50; 3.14159 as 3.14159 rather than 3.14. Values authored with trailing zeros (1.50) keep them, because the lexeme carried them. This is the same text JsonWriteCompact() already produced.
A real written in exponent notation renders in that form — 1.5e3 rather than 1500.00.
JsonRealCreate(0.5) renders 0.5000, not 0.50, because it now renders from the "%.4f" string it stored. Pinned by its own test, since every other case starts from a parsed lexeme.
type() reports "data real" for exponent-without-dot numbers where it used to say "data int". On stock that was an unusable state — type() said int while any copy, render or iteration of the same value terminated the agent.

Verification

Each of the six commits was checked out, built with a top-level make -j2, had the test binary deleted to force a relink, and ran the full tests/unit suite. All six pass independently. Tip: 39/39, with json_test at 75/75.

Two things worth knowing if you reproduce this:

make check inside tests/unit does not rebuild ../libutils, so a test binary can silently link the previous archive. A top-level make and an explicit rm -f of the binary are needed before believing a before/after result.
StringToLongExitOnError is also called benignly during start-up via GetSysVars. An unconditioned breakpoint stops there and prints a misleading stack; the breakpoint has to be conditioned on the value.

Process, and what is not here

This does not follow CONTRIBUTING's process section, deliberately: there is no Jira ticket, so the commits carry Changelog: trailers but no Ticket: trailer rather than an invented number. Happy to add one and rewrite if you open an issue for it.

Still open, and not addressed here:

JsonPrimitiveGetAsInteger() remains fatal by construction. Nothing reachable from parsed data calls it any more in this repository, but it is a footgun on the public header — worth noting so nobody "fixes" the copy path by going back to it.
JsonRealCreate() still formats with "%.4f", so reals built in memory are lossy at construction. Pre-existing and out of scope; this series only stops them being reformatted a second time.
libntech has no mustache test at all — nothing under tests/unit references MustacheRender(), yet this series changes it. cfengine/core's spec-driven tests/unit/mustache_test.c does exercise it and takes new cases as pure data in tests/unit/data/mustache_extra.json. I would like to add number cases there, but they must land after core takes a libntech carrying this change — before that, such a case would terminate core's mustache_test rather than fail it.

cfengine/core needs a matching change; four call sites there reach the same fatal conversion from parsed data (rlist.c, iteration.c, generic_agent.c, unix_iface.c). I have those ready and will offer them separately once the direction here is settled.

JsonPrimitiveToString() and mustache's real case converted the parsed number to a double and formatted it with StringFromDouble(), which uses "%.2f". Any real with more than two decimals came out wrong: 0.00049 rendered as 0.00 0.001 rendered as 0.00 3.14159265 rendered as 3.14 This is a wrong value rather than a rounded one, and it is not confined to diagnostics: mustache is how JSON data reaches rendered configuration files, so a rate, a ratio or a threshold silently becomes zero in the file that gets written. It also disagreed with two other renderings of the same element. JsonWriteCompact() emits the parsed string unchanged, so serialising and rendering the same document produced different numbers; and reals built in memory by JsonRealCreate() are stored with "%.4f", so the library disagreed with itself about how much of a real to keep. Both call sites now return the number as it was parsed, which is what JsonWriteCompact() already does and what the string case beside them already does. This is exact rather than merely more precise, and it removes the disagreement instead of moving it to a different decimal place. One behaviour change worth a reviewer's attention: a real written in exponent notation now renders in the form it was written, so "1.5e3" renders as "1.5e3" rather than "1500.00". That is the same text JsonWriteCompact() already produces for it, but it is a visible change for anyone templating such a value. Not addressed here, and still truncating: cfengine/core's libpromises/rlist.c and libpromises/iteration.c call StringFromDouble() on JSON reals for the same purpose, so the fix is only complete once those are changed too. StringFromDouble() itself is left alone -- it is a general utility with callers outside this concern. tests/unit: 39/39 pass. Changelog: Title

The preceding change had no test behind it. This covers JsonPrimitiveToString() for reals and requires the rendered string to equal both the number as parsed and what JsonWriteCompact() emits for the same element, which is the property the change is really about. Verified in both directions. With libutils/json.c and libutils/mustache.c restored to 0c0620d the test fails on the first case: "0.00049" != "0.00" Cases with more than two decimals are the visible defect; "0.5" and "0.25" are included because "%.2f" reformatted those too, returning "0.50" and disagreeing with the serialiser on a value it had not lost any precision on. Exponent forms are deliberately absent: a number written with an exponent and no decimal point is still classified as an integer at this commit, so it does not reach this path yet. A real built in memory rather than parsed is covered separately, because it renders from a different string. JsonRealCreate() stores its argument with "%.4f", so after the preceding change JsonRealCreate(0.5) renders as "0.5000" where it used to render as "0.50". That is the stored value and it agrees with JsonWriteCompact(), so it is the correct result, but every other case here starts from a parsed lexeme and none of them would notice a regression that broke only the in-memory producer. The mustache renderer is fixed by the same change and is not covered here, because libntech has no mustache test to extend -- nothing under tests/unit references MustacheRender(). cfengine/core's spec-driven tests/unit/mustache_test.c does exercise it, and number cases can be added there as data, but only once core takes a libntech carrying this change: until then such a case would terminate core's mustache_test rather than fail it. tests/unit: 39/39 pass. Changelog: None

Two shapes of number that RFC 8259 allows, and that JsonParse() accepts, could terminate the process the moment anything rendered them. Both reach StringToLongExitOnError(), which calls DoCleanupAndExit(): * A number in exponent notation with no decimal point. The parser already tracks seen_exponent, but classified REAL vs INTEGER on seen_dot alone, so "1e-8", "1E5" and even "2e0" became integer primitives holding a lexeme strtol() cannot read. * An integer whose magnitude does not fit in a long. JSON puts no limit on the magnitude of a number, so "9223372036854775808" and larger are valid input that no long can hold. The first is fixed where it is caused: a number written with an exponent is a real, whether or not it also has a fractional part. The second cannot be fixed by classification, because the value really is an integer -- it just is not one this platform can represent. So JsonPrimitiveToString() and mustache's integer case now return the number as it was parsed, rather than converting to long and back. That is what JsonWriteCompact() already does for the same element, so it also removes a case where rendering and serialising the same document disagreed. Rendering a large integer is now exact rather than fatal. Measured on 3.27.1 before the change, via a data file read with readjson() and rendered with string_mustache(): cf-promises exits, so cf-agent cannot validate the policy and falls back to failsafe. A single valid JSON value therefore takes a host off its policy. After the change the same inputs render, and "1e-8", "2e0", "9223372036854775808" and 10^30 all survive. Values that already worked -- 42, LONG_MAX, 0.5 -- are unchanged. tests/unit: 39/39 pass, including json_test. Changelog: Title

test_parse_exponent_numbers checks that a number in exponent notation parses as a real whether or not it also has a decimal point, that rendering it returns instead of terminating the process, and that serialisation still emits the number as it was parsed. The type is asserted before anything renders, so a classification regression fails an assertion instead of exiting the test binary. test_primitive_to_string_numbers checks that JsonPrimitiveToString() returns an integer as it was parsed, agreeing with JsonWriteCompact() for the same element, including integers whose magnitude does not fit in a long. Those two cannot both fail gracefully, and the order they are registered in decides how much a regression tells you. Converting an oversized integer through long reaches StringToLongExitOnError(), which terminates the whole test binary, so every test registered after that one is never reached and reports nothing. The tests that fail by assertion are therefore registered first, and the ones whose subject is the fatal conversion last, so that a regression produces the assertion failures before it produces the abort. The harness still reports the abort as a failure, which is what a regression must guarantee, but it says less than an assertion does. Both tests fail against the code as it was before the previous commit: the exponent test on the type assertion, the rendering test by process exit on '9223372036854775808'. Changelog: None

JsonPrimitiveCopy() rebuilt each number from a C numeric type, so a copy did not equal its original in three separate ways: * JsonIntegerCreate() takes an int while JsonPrimitiveGetAsInteger() returns a long, so every value outside int range was silently narrowed. 9223372036854775807 copied as -1, and 2000000000000 -- an unremarkable number -- copied as -1454759936. * JsonRealCreate() formats with "%.4f", so 0.00049 copied as 0.0005 and 3.14159265 copied as 3.1416. * JsonPrimitiveGetAsInteger() reaches StringToLongExitOnError() for a magnitude no long can hold, so copying such a document terminated the process rather than reporting anything. The first of these is the worst, because it is silent: no error, no log, just a different number downstream. A JSON document carrying a millisecond timestamp is enough to hit it -- those have exceeded INT_MAX since 2001 -- and nothing reports that the value changed. Copying is not a niche operation. In cfengine/core, storing a JSON container in a CFEngine variable deep-copies it, so this runs at policy load for every data variable. Measured on 3.27.1, a policy whose only content is a var promise reading a document with one oversized integer is enough: cf-promises exits during LoadPolicy, so cf-agent cannot confirm the policy and falls back to failsafe. Nothing has to render or iterate the value; declaring it is sufficient. The same sink is reached from augments, where no user policy is involved at all. Measured against the copy of a parsed document, before this change: 9223372036854775807 -> -1 2000000000000 -> -1454759936 0.00049 -> 0.0005 3.14159265 -> 3.1416 0.5 -> 0.5000 After it, all of those copy unchanged, as do 9223372036854775808 and 1e-8, which previously exited. test_copy_preserves_numbers covers all of it. Verified in both directions with a forced relink: it fails against the code without this change and passes with it. Changelog: Title

JsonSelect() converted an all-digit array index with StringToLongExitOnError(), which calls DoCleanupAndExit(). The index may come from data rather than from our own code, so a caller could terminate the process by asking for an element that cannot exist. An index too large for a long cannot select anything, so it is now treated as absent, which is what an out-of-range index always did. StringIsNumeric() still guards the branch to digits only, so a successful StringToLong() cannot yield a negative value and the cast to size_t cannot wrap. test_select_oversized_array_index covers it. The test cannot fail gracefully against the unfixed code: the conversion exits the test binary partway through the case. The harness reports that as a failure, which is what a regression needs to guarantee. Verified in both directions by reverting the fix and forcing a relink -- note that `make check` inside tests/unit does not rebuild ../libutils, so a top-level make is required or the test silently links the old library. Changelog: Title

Fork issues #4 and #2 now carry the corrected load-time framing, and djbclark/libntech#5 opens the six-commit stack for review. The mail is the last outward step and is deliberately not sent: its substance changed after the panel -- load-time rather than render-time, a silent epoch-ms integrity path nobody had characterised, and the augments entry that needs no user policy -- so the operator should read it before it goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

djbclark · 2026-08-17T14:26:56Z

Now tracked upstream in Jira as CFE-4724, covering this whole six-commit stack (the truncation fix and the fatal-exit fix together, since they are not independently landable).

djbclark · 2026-08-17T19:55:18Z

Offered upstream as NorthernTechHQ/libntech#294

Branch fix/json-number-handling,
cut from master 0c0620d (the current tip), byte-identical tree to
fix/json-number-fatal-exit. Recommitted in the project's own style: past-tense
subjects and Ticket: CFE-4724 on every one of the six commits, with the
existing Changelog: trailers preserved.

It merges cleanly with #293
(the JSON string codec, CFE-4730) — checked with git merge-tree: zero
conflicts. The two are independently landable in either order.

Re-verified independently before offering

macOS 26.6.1 arm64:

make -j2                     rc=0, 0 warnings
tests/unit make check        rc=0, 39/39 test binaries PASS
json_test                    All 75 tests passed

Discrimination, reverting only libutils/json.c and libutils/mustache.c
to stock 0c0620d and keeping the new tests:

json_test rc=1 — the run never finishes.
  test_real_renders_as_parsed                    Test failed
  test_real_created_in_memory_renders_as_stored  Test failed
  test_parse_exponent_numbers                    Test failed
  "0.00049" != "0.00"   /   "0.5000" != "0.50"
  test_primitive_to_string_numbers: Starting test
     error: Conversion error (34 - Overflow) on '9223372036854775808'

74 tests start unfixed and the binary is killed by the library under test before
it can report; 76 start fixed and all 75 pass. That the test process itself is
terminated is the sharpest demonstration of the defect. Sources restored
byte-identical by sha256, clean tree, clean rebuild.

The one warning visible when building the test binary (json_test.c:2618,
JsonNullCreate without a prototype) is pre-existing from upstream 1d26c08
and untouched by this series.

The PR body offers to split per defect or drop the JsonSelect() commit if
maintainers prefer a different shape.

The stack had only ever been a FORK PR (djbclark/libntech#5) -- it was never offered to NorthernTechHQ. That was the real blocker on B-10's core half, not the core code. Six commits, cut from master 0c0620d (current tip), past-tense subjects and Ticket: CFE-4724 throughout. Merges cleanly with #293 (merge-tree, zero conflicts), so the two land independently. Discrimination is the sharpest of the three shipped today: reverting only json.c and mustache.c to stock makes the library TERMINATE ITS OWN TEST BINARY at test_primitive_to_string_numbers -- 74 tests start unfixed and the run never reports, against 76 start / 75 pass fixed. B-10's core half stays pending, now for a documented reason: it needs #294 merged and a submodule bump upstream (core#7).

djbclark added 6 commits August 17, 2026 07:43

Navigation Menu

Render, copy and serialise a JSON number from the text it was parsed from - #5

Render, copy and serialise a JSON number from the text it was parsed from#5
djbclark wants to merge 6 commits into
masterfrom
fix/json-number-fatal-exit

djbclark commented Aug 17, 2026

Uh oh!

djbclark commented Aug 17, 2026

Uh oh!

djbclark commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

djbclark commented Aug 17, 2026

The mistake, once

Severity — this takes a host off its policy at policy load

The quieter half

Why these are stacked, and why the bottom two cannot be dropped

Behaviour changes you will want to look at

Verification

Process, and what is not here

Uh oh!

djbclark commented Aug 17, 2026

Uh oh!

djbclark commented Aug 17, 2026

Offered upstream as NorthernTechHQ/libntech#294

Re-verified independently before offering

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant