invercargill-upstream-issues.md 7.9 KB

Upstream issues — Invercargill / invercargill-json

Friction points surfaced by the Percipio migration that must be fixed upstream of Statum, i.e. in the invercargill-1 / invercargill-json libraries (declared as dependencies in meson.build).

All of the items below are correctness/crash bugs in Invercargill's JSON mapping layer — specifically the ValueElementfrom_elementJsonElement / from_properties path. Statum merely calls these APIs (see call sites below), so it can only work around them, not fix them. Item numbers in parentheses refer to the original Percipio friction report.


Severe — process-crashing serialization

I-1. int64 values segfault during JSON serialisation (orig #7)

RESOLVED (verified 2026-08-31, WP1 server-side effort). The dispatch fix landed upstream in Invercargill-Json commit fc38c4c ("Fix bool and int serialisation issues") — from_element now reads the raw GValue behind a ValueElement and dispatches on the fundamental type. This effort did not need to modify Invercargill-Json; Statum/tests round-trips int64 (full 2^53+ precision) through both the public snapshot path and EncryptionProvider.author_properties/read_properties (the Percipio login crash path).

  • What happens: GObjectMapping.to_properties() wraps each scalar GObject property in new ValueElement(value) (src/GObjectMapping.vala:85). When that PropertyDictionary is later serialised via JsonElement.from_properties() — e.g. in EncryptionProvider.author_properties (src/Cryptography/EncryptionProvider.vala:77) or in the snapshot-to-client path — the int64 branch of from_element crashes at node.set_int(element.as<int64?>()).
  • GDB: backtrace confirmed the segfault in encryption_provider_author_propertiesJsonElement.from_properties.
  • Impact: catastrophic. The login action hard-crashed the whole server process (segfault) until every int64 was purged from slot state. Single most dangerous footgun.
  • Fix (Invercargill): repair the int64 path in from_element / ValueElement.as<int64?>() so a legitimately-held int64 ValueElement produces a valid Json.Node via set_int instead of crashing.

I-2. bool values segfault on the encryption serialisation path (orig #9)

RESOLVED (verified 2026-08-31, WP1 server-side effort). Same upstream fix as I-1 (Invercargill-Json fc38c4c); verified through the set_private_typedauthor_propertiesread_properties path by Statum/tests. A related Statum-side lifetime bug found while testing this path — EncryptionProvider.read_properties returned a JsonObject wrapping memory owned by a temporary JsonElement (use-after-free) — was fixed in Statum; it now returns values re-parsed into a PropertyDictionary it owns.

  • What happens: the same from_properties/from_element family crashes on a bool ValueElement. The public path does not crash on bools (it serialises the snapshot DTO through a different, robust serialiser), but the encryption/private path — set_private_typed (src/DirectiveBuilder.vala:56) → to_propertiesEncryptionProvider.author_properties (src/Cryptography/EncryptionProvider.vala:77) → from_properties — does.
  • Impact: had to strip all bools out of AuthPrivate and derive admin status from the signed public role instead.
  • Fix (Invercargill): make from_element/from_properties handle bool ValueElements without crashing, so the private/encryption path is as robust as the public path. (Same root cause as I-1: the ValueElement type dispatch in from_element is incomplete.)

Correctness — wrong wire type

I-3. int (32-bit) serialises as a JSON string, not a number (orig #8)

RESOLVED (verified 2026-08-31, WP1 server-side effort). Same upstream fix as I-1 (Invercargill-Json fc38c4c); Statum/tests asserts int fields serialise as unquoted JSON numbers and round-trip on both paths.

  • What happens: to_properties on a GObject with an int field produces a JSON string ("best_score": "-1") instead of a number. The int32 branch in from_element is not reached for ValueElements; it falls through to the assignable_to<string>() transform fallback.
  • Impact: slot numbers render as quoted strings; server-side get_int on a round-tripped value fails because the encrypted JSON stored a string. JS coercion mostly hides it for display, but it breaks round-tripping.
  • Fix (Invercargill): make from_element emit numbers for numeric ValueElements consistently (reach the int32/int64 branches rather than the string fallback).

Note: I-1, I-2 and I-3 share a single root cause — the ValueElementfrom_element type dispatch is unreliable for non-string scalars. Fixing the dispatch holistically would close all three at once.

Ergonomic / API

I-4. Properties interface has no typed setter / set_json (orig #16, secondary)

Mitigated Statum-side (2026-08-31): GObjectMapping.to_properties now returns PropertyDictionary directly (see statum-issues.md S-13), so Statum callers no longer need the cast. The upstream interface enhancement remains open.

  • What happens: GObjectMapping.to_properties() returns the Properties interface (src/GObjectMapping.vala:57), but adding a list/nested value requires casting to the concrete PropertyDictionary to use its indexer ((PropertyDictionary) to_properties(pub)).
  • Impact: fragile, undocumented cast.
  • Fix (Invercargill): give the Properties interface a typed setter (e.g. set_json(string key, Element element)) so callers need not cast to PropertyDictionary.
    • The primary mitigation for orig #16 is Statum-side (see statum-issues.md S-13: change to_properties to return PropertyDictionary directly); this item is the upstream API enhancement that would remove the cast at the source.

I-5. PropertyMapperBuilder's default constructor closure dangled after build

RESOLVED (2026-08-31, WP1 server-side effort). Fixed in this repository's copy of Invercargill as a pure bug fix — no API change. The builder now installs no default constructor closure, and PropertyMapper.materialise() falls back to Object.new(typeof(T)) when no constructor was set explicitly.

  • What happens: PropertyMapperBuilder<T>'s constructor defaulted its constructor field to a closure capturing the builder itself. The builder is usually transient (built and dropped inside PropertyMapper.build_for), while the returned PropertyMapper lives on, so every later materialise() call went through a closure pointing at freed memory — a use-after-free that surfaced as crashes/garbage instances when mappers were resolved and used after the building scope had ended.
  • Impact: latent lifetime bug on every mapper built via PropertyMapper.build_for that did not call set_constructor explicitly (the common case).
  • Fix (Invercargill): leave the builder's constructor null by default and let materialise() construct through Object.new(typeof(T)) — no closure outlives the builder.

Reproduction call sites (in Statum, for the Invercargill maintainers)

Bug Statum call site Invercargill symbol that crashes/misbehaves
I-1 GObjectMapping.vala:85 (new ValueElement(value)) → EncryptionProvider.vala:77 (JsonElement.from_properties) ValueElement.as<int64?>()Json.Node.set_int
I-2 DirectiveBuilder.vala:56EncryptionProvider.vala:77 from_properties / from_element on a bool
I-3 GObjectMapping.vala:85 → any from_properties/stringify from_element falls through to assignable_to<string>() for int32
I-4 GObjectMapping.vala:57 (return type) Properties interface lacks a setter