Skip to content

json: pin the measure-then-write invariant with a test - #294

Draft
TurkeyMan wants to merge 1 commit into
masterfrom
json-measure-write-tests
Draft

TurkeyMan wants to merge 1 commit into
masterfrom
json-measure-write-tests

Conversation

@TurkeyMan

@TurkeyMan TurkeyMan commented Sep 14, 2026

Copy link
Copy Markdown
Member

Why

write_json() is designed to be called twice: once with a null buffer to measure, once to fill a buffer of exactly that size. The sync JSON encoder does this for every value it puts on the wire. If the two passes ever disagree, the caller emits whatever was already in the tail of its buffer — char.init, i.e. 0xFF — inside an otherwise well-formed frame, and the peer reports a JSON parse error with nothing to diagnose it from.

This came out of chasing intermittent malformed /sync frames from an ESP32-S3 board. The theory was a measure/write mismatch; the test disproves it for every shape that path encodes, which is worth keeping so the next person doesn't re-derive it.

What

Tests only — no production code changed, because none needed changing. The guards that came out of the same investigation are in open-watt/openwatt#700.

One unittest block, one check() helper, two sets of inputs:

  • Quantity scalars — the unit spellings the field actually uses (W, kW, V, A, %, Wh, Hz, /s, /hr, …) crossed with 0, negatives, denormals, 1e100, -0.0, NaN and ±inf, plus an integer-valued quantity. Values the encoder refuses outright (negative measure) are skipped rather than asserted — refusing is legitimate, callers must handle it.
  • Composites — nested maps, nested arrays, escapes and control characters, large and small numbers, empty object/array/string; each at indents 0/1/2/4, both dense and pretty.

Every case asserts the measured length is positive and in range, that the write pass returns exactly the measured length, and that no char.init byte survives anywhere in the written range.

Verification

make CONFIG=unittest — 74/74 modules pass.

write_json() is used in two passes by callers that size a buffer from the
null-buffer pass and fill it with the second (the sync JSON encoder does
exactly this). If the two ever disagree, the caller ships whatever was
already in the tail of its buffer - char.init, 0xFF - inside an otherwise
valid frame, and the peer sees a JSON parse error with no explanation.

One block, covering both shapes that path encodes: quantity scalars
across the unit spellings the field uses (including NaN/inf and values
that refuse outright), and nested composites at every indent and density
a caller can ask for. Each case asserts the measured length, that the
write returns the same length, and that no uninitialised byte survives.
@TurkeyMan
TurkeyMan force-pushed the json-measure-write-tests branch from 8d2723b to 2b61139 Compare September 14, 2026 08:03
@TurkeyMan TurkeyMan changed the title json: pin the measure-then-write invariant with tests json: pin the measure-then-write invariant with a test Sep 14, 2026
TurkeyMan added a commit to open-watt/openwatt that referenced this pull request Sep 14, 2026
write_json() measures negative when it refuses a value - a unit with no
spelling, a toString that fails. Every caller here assigned that to
size_t and passed it straight to Array.extend(), which turns -1 into a
~4GB allocation; on a constrained board that is an OOM in the middle of
encoding a frame, and array_allocate() does not check the result. The
milder failure is just as bad: a short write leaves the tail of the
buffer as char.init (0xFF) inside an otherwise valid frame, so the peer
gets a JSON parse error pointing at bytes nobody emitted on purpose.

Measure into ptrdiff_t, roll the buffer back if the write disagrees with
the measurement, and substitute a valid literal - null for a value, ""
for a string - with a warning naming what happened. A frame that says
null is recoverable; one with uninitialised bytes in it is not.

Note this is hardening, not a fix for an observed corruption: tests added
alongside in urt (open-watt/urt#294) show measure and write agreeing for
every shape this path encodes.
TurkeyMan added a commit to open-watt/openwatt that referenced this pull request Sep 14, 2026
write_json() measures negative when it refuses a value - a unit with no
spelling, a toString that fails. Every caller here assigned that to
size_t and passed it straight to Array.extend(), which turns -1 into a
~4GB allocation; on a constrained board that is an OOM in the middle of
encoding a frame, and array_allocate() does not check the result. The
milder failure is just as bad: a short write leaves the tail of the
buffer as char.init (0xFF) inside an otherwise valid frame, so the peer
gets a JSON parse error pointing at bytes nobody emitted on purpose.

append_json() in manager.value, next to the other Variant conversions,
is the one copy: measure into ptrdiff_t, roll the buffer back if the
write disagrees with the measurement, substitute the caller's literal -
null for a value, "" for a string - and say so in the return. A frame
that says null is recoverable; one with uninitialised bytes is not.

Note this is hardening, not a fix for an observed corruption: tests added
alongside in urt (open-watt/urt#294) show measure and write agreeing for
every shape this path encodes.
TurkeyMan added a commit to open-watt/openwatt that referenced this pull request Sep 14, 2026
write_json() measures negative when it refuses a value - a unit with no
spelling, a toString that fails. Every caller here assigned that to
size_t and passed it straight to Array.extend(), which turns -1 into a
~4GB allocation; on a constrained board that is an OOM in the middle of
encoding a frame, and array_allocate() does not check the result. The
milder failure is just as bad: a short write leaves the tail of the
buffer as char.init (0xFF) inside an otherwise valid frame, so the peer
gets a JSON parse error pointing at bytes nobody emitted on purpose.

append_json() in manager.value, next to the other Variant conversions,
is the one copy: measure into ptrdiff_t, roll the buffer back if the
write disagrees with the measurement, substitute the caller's literal -
null for a value, "" for a string - and say so in the return. A frame
that says null is recoverable; one with uninitialised bytes is not.

Note this is hardening, not a fix for an observed corruption: tests added
alongside in urt (open-watt/urt#294) show measure and write agreeing for
every shape this path encodes.
@TurkeyMan
TurkeyMan marked this pull request as draft September 16, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant