diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 0df96e676d..f18fed751f 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -285,7 +285,7 @@ flash regardless of target. #### Compile-time fresh-install defaults (`src/helpers/MQTTDefaults.h`) -Optional PlatformIO `build_flags` override defaults written when `/mqtt_prefs` is first created. They do **not** change existing saved prefs on upgrade or reflash (unless `/mqtt_prefs` is erased). +Optional PlatformIO `build_flags` override defaults used when `/mqtt.json` is first created. They do **not** change existing saved prefs on upgrade or reflash (unless `/mqtt.json` is erased). | Macro | Default | Notes | |-------|---------|-------| diff --git a/MQTT_INTERNALS.md b/MQTT_INTERNALS.md index d0de03985d..bc1e13b2c8 100644 --- a/MQTT_INTERNALS.md +++ b/MQTT_INTERNALS.md @@ -8,7 +8,8 @@ Developer-facing notes on how the MQTT observer feature is structured in the cod - `src/helpers/bridges/MQTTBridge.h` - MQTT bridge class definition - `src/helpers/bridges/MQTTBridge.cpp` - MQTT bridge implementation - `src/helpers/MQTTPresets.h` - Preset definitions, CA certificates, and lookup functions -- `src/helpers/MQTTDefaults.h` - Compile-time defaults for fresh `/mqtt_prefs` +- `src/helpers/MQTTDefaults.h` - Compile-time defaults for fresh `/mqtt.json` +- `src/helpers/MQTTPrefsSerializer.h` - Versioned, semantic observer JSON schema - `src/helpers/MQTTMessageBuilder.h` - JSON message formatting utilities - `src/helpers/MQTTMessageBuilder.cpp` - JSON message formatting implementation - `src/helpers/JWTHelper.h` - JWT token generation for Ed25519-based authentication @@ -30,8 +31,9 @@ The observer feature is kept out of upstream-tracked files through three mechani `resolveAlertScope`, `beginDeferredOtaUpdate`). The example apps override them behind `#ifdef WITH_MQTT_BRIDGE`. - **Separate settings file** — observer settings (MQTT slots, WiFi, timezone, SNMP, - radio watchdog, fault alerts) live in the `MQTTPrefs` struct persisted to - `/mqtt_prefs`, keeping `NodePrefs` / `/com_prefs` aligned with the upstream layout. + radio watchdog, fault alerts) live in the runtime `MQTTPrefs` object and are + field-serialized to `/mqtt.json`, keeping `NodePrefs` / `/prefs.json` aligned with + upstream. The two files are independent transactions, not one atomic snapshot. Remaining integration points in upstream files: - `examples/simple_repeater/MyMesh.{h,cpp}`, `examples/simple_room_server/MyMesh.{h,cpp}` - @@ -164,75 +166,132 @@ which packet events non-MQTT bridges capture. The MQTT bridge ignores `bridge.so favour of independent `mqtt.rx` / `mqtt.tx` controls. Everything MQTT-specific lives under `mqtt.*` (shared settings), `mqttN.*` (per-slot broker config), `wifi.*`, and `timezone.*`. -### `/mqtt_prefs` file format - -`/mqtt_prefs` is written with an 8-byte `MQTTPrefsHeader` (`magic`, `version`, -`payload_len`) followed by the raw `MQTTPrefs` payload. The magic is -`{0xF5, 'M', 'Q', 'P'}` — its leading non-ASCII byte can never collide with the first -bytes of a legacy (headerless) file, whose payload begins with the `mqtt_origin` -string. Bump `MQTT_PREFS_VERSION` when the payload layout changes incompatibly; a file -whose version this firmware doesn't recognize is left untouched and the in-memory prefs -fall back to defaults (no downgrade, no misread). `saveMQTTPrefs()` also refuses to -write while such a file is present (`_mqtt_prefs_hold`), so a `set` command after a -firmware downgrade can't clobber the newer config — observer settings changed in that -state simply don't persist. The frozen legacy layouts are pinned with `static_assert`s -in `MQTTPrefsStorage.h`, so every target build re-verifies the fleet's file offsets. - -Adding a field to the current version stays backward compatible: append it to the end -of `MQTTPrefs`, give the older exact payload length an explicit decoder boundary, and -leave the missing tail at its default. The packet-filter addition follows that rule: -the prior 2864-byte v1 payload loads with all six filters set to `all`, while the -full payload is 2876 bytes. - -#### The downgrade contract - -**Within a version tag the layout is append-only, and a longer payload is always -readable.** A file written by a later build starts with this binary's exact baseline, -so `classify()` reads that prefix and ignores the tail. A downgraded node keeps its -WiFi credentials, broker slots, and every other setting it understands; the only thing -it loses is the settings the newer build added. - -That asymmetry is the whole point. Refusing the file costs the operator the network -itself — `/mqtt_prefs` holds `wifi_ssid`/`wifi_password` as well as the broker config, -so a node that falls back to defaults has no WiFi, no portal, and no OTA, recoverable -only over serial. Reading it costs a feature's settings. Losing later settings is the -acceptable half of that trade; losing the node is not. - -The tail survives until something actually writes. `saveMQTTPrefs()` rewrites at this -binary's own length, so a rollback that changes no observer setting and is later rolled -forward keeps the newer fields intact — only an explicit `set` while downgraded drops -them. The boot log says so when it happens. - -**A change that is not a pure append MUST bump `MQTT_PREFS_VERSION`.** The version -check is what makes the rule above safe: a different tag is still refused outright and -the file preserved, because the bytes may no longer mean what this binary thinks. Never -reorder, resize, or repurpose an existing field within a version. - -Note the rule is only as old as the build that implements it. Firmware already deployed -carries the *previous* decoder, which rejects any longer v1 payload — so rolling back -from this build to one shipped before it still falls back to defaults. -`MQTTPrefsCodec::payloadLenFor()` covers that gap from the writing side: it returns the -shortest length that still round-trips the configuration, so a node keeps writing 2864 -bytes until a packet filter actually holds something, and clearing the last non-default -filter puts it back. That mitigation can be retired once no supported downgrade target -predates the contract; the contract itself is the durable half. - -Shorter payloads keep their existing, stricter treatment: a short length must match a -boundary that really shipped (`MQTT_PREFS_V1_*_PAYLOAD_SIZE`), because raw prefs have -no checksum and an arbitrary short size cannot be trusted to mean anything. +### `/mqtt.json` file format + +Observer preferences use the same `ConfigSerializer` object notation as upstream +`/prefs.json`: semantic unquoted keys, quoted strings, decimal numbers, and nested +objects. Schema version 1 requires the root `version:1` field, which must be the +first property of the root object. The main shape is: + +```text +{version:1, + wifi:{ssid:"...",password:"...",power_save:1}, + time:{timezone:"...",utc_offset:0,ntp_server:"..."}, + mqtt:{origin:"...",iata:"SEA",packets_enabled:1,raw_enabled:0, + tx_enabled:2,rx_enabled:1, + status:{enabled:1,interval_ms:300000}, + neighbors:{enabled:0,interval_ms:86400000}, + owner:{public_key:"...",email:"..."}, + slot1:{preset:"analyzer-us",host:"",port:0,username:"",password:"", + token:"",topic:"",audience:"",packet_filter:65535}, + ... slot2 through slot6 ...}, + snmp:{enabled:0,community:"public"}, + radio:{watchdog_min:5}, + alert:{enabled:0,psk_hex:"",wifi_minutes:30,mqtt_minutes:240, + rate_limit_min:60,hashtag:"",region:""}} +``` + +Keys may contain digits after their first character, which permits the readable +`slot1` ... `slot6` names. Every schema key fits `ConfigSerializer`'s 15-character +visible-key limit. Known strings and numbers use strict parsing: duplicates, overlong +strings, malformed decimals, and overflow reject the complete file. A supported file +with a semantically out-of-range value is repaired to that field's safe default and +rewritten atomically. Unknown fields are ignored only within the serializer's general +limits (15-character keys, 127-byte decoded values, and six nested object levels below the root). + +Loading always starts with defaults and parses into a separate heap scratch object. +The live preferences change only after the complete file parses, has an explicit +supported version, and passes validation. A missing/future version, syntax error, +overlength value, or allocation/read failure leaves `/mqtt.json` untouched, runs this +boot on defaults, and holds observer saves so a later CLI/WebConfig write cannot erase +the opaque source. If an observer setter cannot commit its JSON transaction, it restores +the pre-command in-memory preferences and does not restart or apply a bridge change as +though the setting were durable. + +Saves stream to `/mqtt.json.tmp` through a sticky short-write detector while computing +size and checksum. The firmware closes and rereads the temp, verifies size/checksum, +parses it into another scratch object, then publishes with +`/mqtt.json` -> `/mqtt.json.bak` and temp -> primary. Failure safety is paid for in +transient heap: a CLI setter holds one `MQTTPrefs` rollback snapshot (2876 bytes) for the +whole command, and the save allocates one more for normalization defaults (released +before file I/O) and then one for verification — about 5.6 KiB peak above baseline. +Each allocation is checked, so exhaustion refuses the setting rather than crashing. + +If publishing fails partway, the transaction is rolled back rather than left for boot +recovery: the backup is renamed back over the empty primary and the verified temp is +discarded. This is what makes the setter's "change rolled back" reply true — without it +the next boot would promote that temp and activate the value the CLI just refused. When +the rollback itself cannot complete, the save reports an indeterminate outcome instead, +the artifacts stay for recovery, and the CLI says the flash state is unresolved. Further +saves are refused until then, because a transaction cannot start while a temp or backup +exists. Boot recovery selects the usable +primary/temp/backup without overwriting an opaque future or corrupt primary. A valid +future-version temp that reached the rename phase wins over the stale backup and is held +for newer firmware. If a temp claims a future version but uses grammar this firmware +cannot parse, or cannot be classified because scratch allocation fails, recovery renames +nothing at all: it reads the last usable backup straight out of `/mqtt.json.bak`, leaves +the primary name empty, and holds all observer writes. The empty primary name is the only +record that the candidate had already passed the backup rename, so publishing the backup +would make the candidate indistinguishable from a stale artifact — and the next boot, the +one with enough heap or new enough firmware to finally read it, would delete it. Leaving +the names alone means that boot promotes the candidate through the ordinary temp rule, or +falls back to the backup if it turns out to be definitively corrupt. A +definitively corrupt/incomplete current-version temp may be discarded for that backup. +If power fails during the very first migration, there is no JSON primary or backup yet; +recovery discards a definitively invalid temp so the intact `/mqtt_prefs` source can be +loaded and the migration retried. An uncertain/future temp is still preserved and held. + +Unknown slot preset names are repaired to `none`, never to a build-specific default +broker. Duplicate known presets from historical firmware are preserved on load; current +CLI and WebConfig setters prevent creating new duplicates without silently changing an +existing deployment during migration. + +Schema changes that reinterpret existing names or values must increment `version`. +Additive fields may remain in version 1 only when old readers can safely ignore them +within the limits above. A future version is treated as opaque rather than partially +loading its known-looking fields. Literal schema keys are compile-time checked against +the 15-character visible-key limit so a new version-1 field cannot accidentally violate +that downgrade contract. + +Every future version must also keep `version` as the first root property. Older firmware +detects a future file by probing that field with its own grammar, and the probe stops at +the first construct it cannot tokenize (an array, an overlong key, a value type it does +not expect). Syntax a newer schema introduces *before* the version field therefore makes +its files look corrupt rather than future, which costs them the preservation guarantee +above: as the temp of an interrupted commit such a file is discardable instead of +retained. `test_mqtt_prefs_serializer` pins both the ordering and that consequence. + +#### Downgrade and rollback + +The old `/mqtt_prefs` binary is read only for one-time migration and is deliberately +not updated or deleted. When it exists and is usable, it is the exact pre-migration +rollback snapshot. Firmware older than this JSON change will therefore see stale +observer settings after a downgrade. A fresh JSON-only install has no binary observer +configuration to recover on downgrade. + +The two files are intentionally not reconciled. If an operator changes observer settings +while running old firmware, those changes update only `/mqtt_prefs`. Rolling forward to +this firmware makes the existing `/mqtt.json` authoritative again, so rollback-era edits +are ignored unless the operator exports and reapplies them. Conversely, JSON-only settings +cannot be recovered by old firmware. This is a rollback snapshot, not bidirectional sync. + +`LegacyV1MQTTPrefs` and the older pre-slot/3-slot/6-slot structs are frozen migration +ABIs with size/offset assertions. The runtime `MQTTPrefs` layout is not an on-flash ABI. ### Settings upgrade / migration `loadPrefs()` handles every historical on-device format one-time at boot: -- **`/mqtt_prefs`** — if the file has the version header it is read directly. Otherwise +- **`/mqtt_prefs` -> `/mqtt.json`** — if the legacy file has the version header its + frozen v1 layout is field-copied. Otherwise it is a legacy headerless file and its layout is detected by size: pre-slot (`OldMQTTPrefs`), 3-slot (`ThreeSlotMQTTPrefs`), or the 6-slot layout shipped on `observer-firmware` back when it was named `mqtt-bridge-implementation-flex` (`Legacy6SlotMQTTPrefs`). Each is field-copied into - the current compact `MQTTPrefs` and re-saved with the version header — which also + the runtime `MQTTPrefs` and saved as schema-versioned JSON — which also drops the vestigial `_legacy_*` fields the flex layout carried mid-struct. This is a one-time rewrite; every deployed device performs it on its first boot of versioned - firmware, after which all reads take the header path. + firmware, after which `/mqtt.json` is authoritative. The binary source remains as + a rollback snapshot and is never dual-written. The pre-slot (`OldMQTTPrefs`) copy maps the old single-broker keys onto slots: `mqtt.analyzer.us = on` → slot 1 `analyzer-us`, `mqtt.analyzer.eu = on` → slot 2 `analyzer-eu`, and a configured `mqtt.server` / `mqtt.port` / `mqtt.username` / @@ -241,7 +300,7 @@ no checksum and an arbitrary short size cannot be trusted to mean anything. - **`/com_prefs`** — a file written by fork firmware that predates the `MQTTPrefs` split (a zero-filled MQTT gap plus a trailing observer block) is detected by size; the trailing SNMP / radio-watchdog / fault-alert settings and the `rx_boosted_gain` / - `flood_max_*` fields are recovered, carried into `/mqtt_prefs`, and both files are + `flood_max_*` fields are recovered, carried into `/mqtt.json`, and the active files are rewritten in the current formats. - Settings the pre-split firmware stored *inside* the `/com_prefs` MQTT gap (the MQTT slot/WiFi config itself) are **not** recovered — users upgrading from firmware that diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 7e209cfb2c..226455033e 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -1050,7 +1050,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc // _prefs.agc_reset_interval = 7; // 28 seconds (secs/4) #endif // Observer defaults (radio_watchdog, alert.*, snmp.*) moved to applyMQTTDefaults() - // in MQTTDefaults.h — they live in /mqtt_prefs now, not NodePrefs. + // in MQTTDefaults.h — they live in /mqtt.json now, not NodePrefs. // bridge defaults _prefs.bridge_enabled = 1; // enabled @@ -1066,7 +1066,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.gps_interval = 0; _prefs.advert_loc_policy = ADVERT_LOC_PREFS; - // MQTT/WiFi/timezone/radio_watchdog defaults live in /mqtt_prefs now (see applyMQTTDefaults). + // MQTT/WiFi/timezone/radio_watchdog defaults live in /mqtt.json now (see applyMQTTDefaults). _prefs.adc_multiplier = 0.0f; // 0.0f means use default board multiplier diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index a232ef5202..e4c3a3f7d9 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -285,6 +285,11 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks void savePrefs() override { _cli.savePrefs(_fs); } +#ifdef WITH_MQTT_BRIDGE + bool saveObserverPrefs() override { + return _cli.saveObserverPrefs(_fs); + } +#endif void sendFloodScoped(const TransportKey& scope, mesh::Packet* pkt, uint32_t delay_millis, uint8_t path_hash_size); diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 9fe5a9c8a8..02efbd43d8 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -891,7 +891,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.radio_fem_txgain = 0; // Observer defaults (alert.*, etc.) moved to applyMQTTDefaults() — they live - // in /mqtt_prefs now, not NodePrefs. + // in /mqtt.json now, not NodePrefs. // bridge defaults (same as repeater) _prefs.bridge_enabled = 1; // enabled @@ -900,7 +900,7 @@ MyMesh::MyMesh(mesh::MainBoard &board, mesh::Radio &radio, mesh::MillisecondCloc _prefs.bridge_baud = 115200; // baud rate _prefs.bridge_channel = 1; // channel 1 - // MQTT/WiFi/timezone defaults live in /mqtt_prefs now (see applyMQTTDefaults). + // MQTT/WiFi/timezone defaults live in /mqtt.json now (see applyMQTTDefaults). next_post_idx = 0; next_client_idx = 0; diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index b0eb4e8f33..65de77ede9 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -289,6 +289,11 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks void savePrefs() override { _cli.savePrefs(_fs); } +#ifdef WITH_MQTT_BRIDGE + bool saveObserverPrefs() override { + return _cli.saveObserverPrefs(_fs); + } +#endif void sendFloodScoped(const TransportKey& scope, mesh::Packet* pkt, uint32_t delay_millis, uint8_t path_hash_size); diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 427d456bfd..bf01552778 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -6,6 +6,7 @@ #include "MQTTPrefsAtomicStore.h" #include #include +#include #ifndef BRIDGE_MAX_BAUD #define BRIDGE_MAX_BAUD 115200 @@ -23,6 +24,7 @@ #include "MQTTDefaults.h" #include "MQTTPrefsCodec.h" #include "MQTTPrefsRecovery.h" +#include "MQTTPrefsSerializer.h" #endif // Believe it or not, this std C function is busted on some platforms! @@ -94,7 +96,8 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { _prefs->bridge_pkt_src = 1; // Default to RX (logRx) for new installs } #ifdef WITH_MQTT_BRIDGE - // Load observer preferences (MQTT/WiFi/timezone/SNMP/alert) from /mqtt_prefs. + // Load observer preferences (MQTT/WiFi/timezone/SNMP/alert) from /mqtt.json, + // migrating the old /mqtt_prefs binary when JSON does not exist yet. // Readers (MQTTBridge, AlertReporter, observer CLI) use _mqtt_prefs directly — // these fields no longer exist in NodePrefs, so there is nothing to sync. MQTTPrefsAtomicStore::LegacyUpgradeGate legacy_upgrade( @@ -119,13 +122,13 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { } } // mqtt_rx_enabled: new field appended to end of MQTTPrefs. On upgrade from older firmware, - // the shorter /mqtt_prefs file won't contain it, so it keeps the default value (1 = on) + // a shorter legacy /mqtt_prefs file won't contain it, so it keeps the default value (1 = on) // set by setMQTTPrefsDefaults(). No explicit migration needed. #endif // Republish legacy binary prefs as /prefs.json. Old-format files also carried a // trailing observer block, which loadPrefsInt() recovered into _legacy_tail; wait - // for loadMQTTPrefs() to commit that to /mqtt_prefs first. The legacy file is left + // for loadMQTTPrefs() to commit that to /mqtt.json first. The legacy file is left // on flash either way, so a deferred or failed save just retries on the next boot. #ifdef WITH_MQTT_BRIDGE if (loaded_from_legacy || _com_prefs_needs_upgrade) { @@ -134,7 +137,7 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { legacy_upgrade.recordComPrefsRewrite(); _com_prefs_needs_upgrade = false; } else { - MESH_DEBUG_PRINTLN("Prefs: deferring /prefs.json migration until /mqtt_prefs commits"); + MESH_DEBUG_PRINTLN("Prefs: deferring /prefs.json migration until /mqtt.json commits"); } } #else @@ -376,13 +379,17 @@ bool CommonCLI::savePrefs(FILESYSTEM* fs, bool save_mqtt) { #ifdef WITH_MQTT_BRIDGE // Observer config (MQTT/WiFi/timezone/SNMP/alert) is persisted separately. The // observer CLI writes _mqtt_prefs directly, so no NodePrefs->MQTTPrefs sync runs. - // Runs regardless of the NodePrefs result so a failed JSON write cannot strand it. + // Ordinary NodePrefs callers leave save_mqtt false; migration is the only + // workflow that may explicitly combine these independent transactions. if (save_mqtt) saveMQTTPrefs(fs); #endif return success; } #ifdef WITH_MQTT_BRIDGE +static const uint32_t MQTT_JSON_FNV1A_OFFSET_BASIS = 2166136261u; +static const uint32_t MQTT_JSON_FNV1A_PRIME = 16777619u; + // Set default values for MQTT preferences (used when file doesn't exist or is corrupted) static void setMQTTPrefsDefaults(MQTTPrefs* prefs) { applyMQTTDefaults(prefs); @@ -396,6 +403,170 @@ static File openMqttPrefsRead(FILESYSTEM* fs, const char* path = "/mqtt_prefs") #endif } +enum class JsonPrefsLoadResult : uint8_t { + Loaded, + LoadedWithRepairs, + UnsupportedVersion, + FutureClaimed, + Invalid, + NoMemory, +}; + +static bool jsonPrefsLoaded(JsonPrefsLoadResult result) { + return result == JsonPrefsLoadResult::Loaded || + result == JsonPrefsLoadResult::LoadedWithRepairs; +} + +static JsonPrefsLoadResult loadMqttJsonFile(FILESYSTEM* fs, const char* path, + MQTTPrefs* output) { + if (output == nullptr) return JsonPrefsLoadResult::Invalid; + applyMQTTDefaults(output); + + // Probe the root version without applying the v1 schema. A future version + // may legitimately change an existing field's type, and must still be held + // opaquely rather than misclassified as corrupt and rolled back. + File version_file = openMqttPrefsRead(fs, path); + if (!version_file) return JsonPrefsLoadResult::Invalid; + MQTTPrefsVersionProbe version_probe; + const bool version_parsed = version_probe.loadSerial(version_file); + version_file.close(); + if (version_probe.hasFutureVersion()) { + return version_parsed ? JsonPrefsLoadResult::UnsupportedVersion + : JsonPrefsLoadResult::FutureClaimed; + } + + File file = openMqttPrefsRead(fs, path); + if (!file) return JsonPrefsLoadResult::Invalid; + MQTTPrefsSerializer serializer(output); + const bool parsed = serializer.loadSerial(file); + file.close(); + if (!parsed) return JsonPrefsLoadResult::Invalid; + if (serializer.hasFutureVersion()) return JsonPrefsLoadResult::UnsupportedVersion; + bool repaired = false; + if (!serializer.apply(&repaired)) return JsonPrefsLoadResult::Invalid; + return repaired ? JsonPrefsLoadResult::LoadedWithRepairs : JsonPrefsLoadResult::Loaded; +} + +// Parse `path` through a heap scratch object and publish it over `dest` only +// once the whole file is known good, so a late failure cannot leave the live +// preferences half-loaded. +static JsonPrefsLoadResult adoptMqttJsonFile(FILESYSTEM* fs, const char* path, + MQTTPrefs* dest) { + MQTTPrefs* scratch = new (std::nothrow) MQTTPrefs; + if (scratch == nullptr) return JsonPrefsLoadResult::NoMemory; + const JsonPrefsLoadResult result = loadMqttJsonFile(fs, path, scratch); + if (jsonPrefsLoaded(result)) memcpy(dest, scratch, sizeof(*dest)); + delete scratch; + return result; +} + +static MQTTPrefsRecovery::FileState mqttJsonFileState(FILESYSTEM* fs, const char* path) { + if (!fs->exists(path)) return MQTTPrefsRecovery::FileState::Missing; + MQTTPrefs* scratch = new (std::nothrow) MQTTPrefs; + if (scratch == nullptr) return MQTTPrefsRecovery::FileState::Indeterminate; + const JsonPrefsLoadResult result = loadMqttJsonFile(fs, path, scratch); + delete scratch; + if (result == JsonPrefsLoadResult::UnsupportedVersion) { + // Syntax and the mandatory version field were valid, so this can be a + // fully verified transaction written by newer firmware. Keep it distinct + // from a torn/corrupt temp during recovery. + return MQTTPrefsRecovery::FileState::FutureUsable; + } + if (result == JsonPrefsLoadResult::FutureClaimed) { + return MQTTPrefsRecovery::FileState::FutureClaimed; + } + return result == JsonPrefsLoadResult::Loaded || + result == JsonPrefsLoadResult::LoadedWithRepairs + ? MQTTPrefsRecovery::FileState::Usable + : MQTTPrefsRecovery::FileState::Preserve; +} + +// hold: observer writes must not replace whatever was left on disk. +// run_from_backup: nothing was renamed and /mqtt.json does not exist; this boot +// reads the last committed image straight out of /mqtt.json.bak. +struct MqttJsonRecovery { + bool hold; + bool run_from_backup; +}; + +static MqttJsonRecovery recoverMqttJsonFiles(FILESYSTEM* fs) { + const MQTTPrefsRecovery::FileState primary = mqttJsonFileState(fs, "/mqtt.json"); + const MQTTPrefsRecovery::FileState temp = mqttJsonFileState(fs, "/mqtt.json.tmp"); + const MQTTPrefsRecovery::FileState backup = mqttJsonFileState(fs, "/mqtt.json.bak"); + const MQTTPrefsRecovery::Action action = MQTTPrefsRecovery::select(primary, temp, backup); + + if (action == MQTTPrefsRecovery::Action::KeepPrimary) { + if (primary == MQTTPrefsRecovery::FileState::Usable) { + if (!MQTTPrefsRecovery::uncertain(temp) && + temp != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.tmp"); + } + if (!MQTTPrefsRecovery::uncertain(backup) && + backup != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.bak"); + } + } + return {MQTTPrefsRecovery::uncertain(primary) || + MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup), + false}; + } + if (action == MQTTPrefsRecovery::Action::DiscardTemp) { + if (fs->remove("/mqtt.json.tmp")) { + MESH_DEBUG_PRINTLN("MQTT: discarded incomplete first-migration JSON temp"); + return {false, false}; + } + MESH_DEBUG_PRINTLN("MQTT: could not discard incomplete /mqtt.json temp; source held"); + return {true, false}; + } + if (action == MQTTPrefsRecovery::Action::PromoteTemp) { + if (fs->rename("/mqtt.json.tmp", "/mqtt.json")) { + if (temp == MQTTPrefsRecovery::FileState::Usable && + backup != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.bak"); + } + MESH_DEBUG_PRINTLN("MQTT: recovered /mqtt.json from transaction temp"); + return {MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup), + false}; + } + MESH_DEBUG_PRINTLN("MQTT: could not recover /mqtt.json temp; files preserved"); + return {true, false}; + } + if (action == MQTTPrefsRecovery::Action::UseBackupHeld) { + // Deliberately rename nothing. The empty primary name is what records that + // the interrupted commit had already moved the old image aside, and a boot + // that cannot classify the candidate must not spend that record: a later + // boot with more heap, or firmware that understands the candidate, promotes + // it through the ordinary rule instead of deleting it as a stale artifact. + MESH_DEBUG_PRINTLN("MQTT: unresolved /mqtt.json candidate; running the backup in place and holding writes"); + return {true, true}; + } + if (action == MQTTPrefsRecovery::Action::RunDefaultsHeld) { + // Same reasoning, with no image this firmware can run: renaming either file + // would spend transaction state that a later boot still needs. + MESH_DEBUG_PRINTLN("MQTT: unresolved /mqtt.json candidate and no runnable backup; " + "using defaults (files preserved)"); + return {true, false}; + } + if (action == MQTTPrefsRecovery::Action::PromoteBackup) { + if (fs->rename("/mqtt.json.bak", "/mqtt.json")) { + if (backup == MQTTPrefsRecovery::FileState::Usable && + !MQTTPrefsRecovery::uncertain(temp) && + temp != MQTTPrefsRecovery::FileState::Missing) { + fs->remove("/mqtt.json.tmp"); + } + MESH_DEBUG_PRINTLN("MQTT: recovered /mqtt.json from transaction backup"); + return {MQTTPrefsRecovery::uncertain(temp) || + MQTTPrefsRecovery::uncertain(backup), + false}; + } + MESH_DEBUG_PRINTLN("MQTT: could not recover /mqtt.json backup; files preserved"); + return {true, false}; + } + return {false, false}; +} + static MQTTPrefsRecovery::FileState mqttPrefsFileState(FILESYSTEM* fs, const char* path) { if (!fs->exists(path)) return MQTTPrefsRecovery::FileState::Missing; File file = openMqttPrefsRead(fs, path); @@ -432,6 +603,14 @@ static bool recoverMqttPrefsFiles(FILESYSTEM* fs) { } return false; } + if (action == MQTTPrefsRecovery::Action::DiscardTemp) { + if (fs->remove("/mqtt_prefs.tmp")) { + MESH_DEBUG_PRINTLN("MQTT: discarded incomplete legacy transaction temp"); + return false; + } + MESH_DEBUG_PRINTLN("MQTT: could not discard incomplete /mqtt_prefs temp; source held"); + return true; + } if (action == MQTTPrefsRecovery::Action::PromoteTemp) { if (fs->rename("/mqtt_prefs.tmp", "/mqtt_prefs")) { // A usable temp is now the committed primary. Its backup is necessarily @@ -446,6 +625,13 @@ static bool recoverMqttPrefsFiles(FILESYSTEM* fs) { MESH_DEBUG_PRINTLN("MQTT: could not recover /mqtt_prefs temp; files preserved"); return true; } + if (action == MQTTPrefsRecovery::Action::UseBackupHeld || + action == MQTTPrefsRecovery::Action::RunDefaultsHeld) { + // Unreachable for the binary format, whose classifier never reports an + // uncertain state. Hold rather than fall through to "nothing to do" if a + // later classifier change makes it reachable. + return true; + } if (action == MQTTPrefsRecovery::Action::PromoteBackup) { if (fs->rename("/mqtt_prefs.bak", "/mqtt_prefs")) { // Symmetric case: a usable backup is now primary, so any interrupted @@ -463,30 +649,33 @@ static bool recoverMqttPrefsFiles(FILESYSTEM* fs) { return false; } -// Filesystem adapter for MQTTPrefsAtomicStore. It writes the new image to -// /mqtt_prefs.tmp and verifies its size. Publishing is a recoverable SPIFFS +// Filesystem adapter for the ConfigSerializer image. It writes to +// /mqtt.json.tmp and verifies its size and checksum. Publishing is a recoverable SPIFFS // transaction: primary -> .bak, then tmp -> primary, then best-effort backup // cleanup. A power loss at every boundary leaves at least one recoverable file. -class MQTTPrefsFileStore { +class MQTTPrefsJsonFileStore { public: - explicit MQTTPrefsFileStore(FILESYSTEM* fs) : _fs(fs) {} + explicit MQTTPrefsJsonFileStore(FILESYSTEM* fs) : _fs(fs) {} bool begin() { _finished = false; _open = false; _owns_temp = false; + _owns_backup = false; _bytes_written = 0; - // Recovery owns stale artifacts. Do not delete them here: a failed commit - // may have moved the old primary to .bak and left a verified temp that the - // next boot must choose between. Refusing the save is safer than erasing an - // image this firmware cannot decode. - if (_fs->exists("/mqtt_prefs.tmp") || _fs->exists("/mqtt_prefs.bak")) return false; + _expected_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; + // Recovery owns stale artifacts. Do not delete them here: a power cut, or a + // failed commit that could not be rolled back, may have moved the old + // primary to .bak and left a verified temp that the next boot must choose + // between. Refusing the save is safer than erasing an image this firmware + // cannot decode. + if (_fs->exists("/mqtt.json.tmp") || _fs->exists("/mqtt.json.bak")) return false; #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM) - _file = _fs->open("/mqtt_prefs.tmp", FILE_O_WRITE); + _file = _fs->open("/mqtt.json.tmp", FILE_O_WRITE); #elif defined(RP2040_PLATFORM) - _file = _fs->open("/mqtt_prefs.tmp", "w"); + _file = _fs->open("/mqtt.json.tmp", "w"); #else - _file = _fs->open("/mqtt_prefs.tmp", "w", true); + _file = _fs->open("/mqtt.json.tmp", "w", true); #endif _open = _file; _owns_temp = _open; @@ -496,6 +685,9 @@ class MQTTPrefsFileStore { size_t write(const uint8_t* bytes, size_t size) { if (!_open) return 0; const size_t written = _file.write(bytes, size); + for (size_t i = 0; i < written; ++i) { + _expected_crc = (_expected_crc ^ bytes[i]) * MQTT_JSON_FNV1A_PRIME; + } _bytes_written += written; return written; } @@ -505,12 +697,33 @@ class MQTTPrefsFileStore { _file.close(); _open = false; #if defined(RP2040_PLATFORM) - File verify = _fs->open("/mqtt_prefs.tmp", "r"); + File verify = _fs->open("/mqtt.json.tmp", "r"); #else - File verify = _fs->open("/mqtt_prefs.tmp"); + File verify = _fs->open("/mqtt.json.tmp"); #endif if (!verify) return false; - const bool complete = verify.size() == _bytes_written; + uint32_t actual_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; + size_t actual_size = 0; + bool read_failed = false; + uint8_t buf[64]; + while (verify.available() > 0) { + // Arduino File implementations normally return a byte count, but some + // Stream implementations use -1 for a read error. Keep that sentinel + // signed so it cannot become a huge size_t and overrun this buffer. + const int count = static_cast(verify.read(buf, sizeof(buf))); + if (count <= 0) { + read_failed = count < 0; + break; + } + actual_size += static_cast(count); + for (int i = 0; i < count; ++i) { + actual_crc = (actual_crc ^ buf[i]) * MQTT_JSON_FNV1A_PRIME; + } + } + const bool complete = !read_failed && + verify.size() == _bytes_written && + actual_size == _bytes_written && + actual_crc == _expected_crc; verify.close(); if (!complete) return false; _finished = true; @@ -521,30 +734,76 @@ class MQTTPrefsFileStore { if (!_finished) return false; // SPIFFS refuses rename(tmp, existing_dest). Move the existing image to a // recoverable backup first, then publish temp into the now-empty primary. - // Never remove either image after a failed boundary; boot recovery selects - // the completed temp or restores the backup. - if (_fs->exists("/mqtt_prefs.bak")) return false; - if (_fs->exists("/mqtt_prefs") && !_fs->rename("/mqtt_prefs", "/mqtt_prefs.bak")) { - return false; + // A power cut at either boundary is resolved by boot recovery; a failure + // that returns here is undone by rollbackFailedCommit(). + if (_fs->exists("/mqtt.json.bak")) return false; + if (_fs->exists("/mqtt.json")) { + if (!_fs->rename("/mqtt.json", "/mqtt.json.bak")) return false; + _owns_backup = true; } - if (!_fs->rename("/mqtt_prefs.tmp", "/mqtt_prefs")) return false; + if (!_fs->rename("/mqtt.json.tmp", "/mqtt.json")) return false; // Cleanup failure is non-fatal: the new primary is published and recovery // will remove a known-good stale backup on a later boot. - if (_fs->exists("/mqtt_prefs.bak")) _fs->remove("/mqtt_prefs.bak"); + if (_fs->exists("/mqtt.json.bak")) _fs->remove("/mqtt.json.bak"); + return true; + } + + // Undo a commit that failed midway. Without this, a failed publish can leave + // the old primary parked in .bak and the verified temp still holding the new + // image, which boot recovery then promotes — so a setter that told the + // operator the change was rolled back would be wrong after the next reset. + // + // Republishing the backup is the operation that matters: once the primary + // name is occupied, recovery keeps it and treats the temp as stale, so the + // temp removal below is only housekeeping. Returns false when the + // pre-transaction state could not be restored and the outcome of the next + // boot is therefore uncertain. + bool rollbackFailedCommit() { + // Only ever restore the backup this transaction made. Anything else under + // that name predates the transaction and is recovery's to resolve. + if (_owns_backup && !_fs->exists("/mqtt.json") && + !_fs->rename("/mqtt.json.bak", "/mqtt.json")) { + return false; + } + _owns_backup = false; + if (_owns_temp && _fs->exists("/mqtt.json.tmp") && !_fs->remove("/mqtt.json.tmp")) { + // A verified temp still outranks a missing primary during recovery, so + // this is only harmless if the primary name is occupied again. + return _fs->exists("/mqtt.json"); + } return true; } - void abort() { + // Drop a temp that was written and byte-verified but then rejected. Returns + // false when it is still on flash and no primary outranks it: recovery + // promotes a lone temp, so the caller cannot claim the change was undone. + bool discardFinishedTemp() { + if (_open) _file.close(); + _open = false; + bool removed = true; + if (_owns_temp && _fs->exists("/mqtt.json.tmp")) { + removed = _fs->remove("/mqtt.json.tmp"); + } + _finished = false; + _owns_temp = false; + return removed || _fs->exists("/mqtt.json"); + } + + // Same disposition contract as discardFinishedTemp(). Only unfinished staging + // is disposable here: once finish() has verified the temp, + // rollbackFailedCommit() has already decided its fate, and a temp that + // survived that is one recovery must resolve after reset. + bool abort() { if (_open) _file.close(); _open = false; - // Once finish() has verified the temp, commit may already have moved the - // primary to .bak. Keep the temp on a commit failure so recovery can - // publish it (or fall back to .bak) after reset. - if (_owns_temp && !_finished && _fs->exists("/mqtt_prefs.tmp")) { - _fs->remove("/mqtt_prefs.tmp"); + bool removed = true; + if (_owns_temp && !_finished && _fs->exists("/mqtt.json.tmp")) { + removed = _fs->remove("/mqtt.json.tmp"); } _finished = false; _owns_temp = false; + _owns_backup = false; + return removed || _fs->exists("/mqtt.json"); } private: @@ -553,32 +812,105 @@ class MQTTPrefsFileStore { bool _open = false; bool _finished = false; bool _owns_temp = false; + // This transaction moved the old primary to /mqtt.json.bak, so a failed + // publish may put it back. Never true for a backup it did not create. + bool _owns_backup = false; size_t _bytes_written = 0; + uint32_t _expected_crc = MQTT_JSON_FNV1A_OFFSET_BASIS; +}; + +class MQTTPrefsStoreStream : public Stream { +public: + explicit MQTTPrefsStoreStream(MQTTPrefsJsonFileStore* store) : _store(store) {} + + size_t write(uint8_t byte) override { return write(&byte, 1); } + size_t write(const uint8_t* buffer, size_t size) override { + if (!_ok || _store == nullptr) return 0; + const size_t written = _store->write(buffer, size); + if (written != size) _ok = false; + return written; + } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + bool ok() const { return _ok; } + +private: + MQTTPrefsJsonFileStore* _store; + bool _ok = true; }; #endif // WITH_MQTT_BRIDGE #ifdef WITH_MQTT_BRIDGE -static const char* mqttPrefsSaveResultName(MQTTPrefsAtomicStore::Result result) { - switch (result) { - case MQTTPrefsAtomicStore::Result::BeginFailed: return "begin"; - case MQTTPrefsAtomicStore::Result::HeaderWriteFailed: return "header write"; - case MQTTPrefsAtomicStore::Result::PayloadWriteFailed: return "payload write"; - case MQTTPrefsAtomicStore::Result::FinishFailed: return "close"; - case MQTTPrefsAtomicStore::Result::CommitFailed: return "rename"; - case MQTTPrefsAtomicStore::Result::Committed: return "committed"; - } - return "unknown"; -} - void CommonCLI::loadMQTTPrefs( FILESYSTEM* fs, MQTTPrefsAtomicStore::LegacyUpgradeGate* legacy_upgrade) { setMQTTPrefsDefaults(&_mqtt_prefs); + const MqttJsonRecovery recovery = recoverMqttJsonFiles(fs); + _mqtt_prefs_hold = recovery.hold; + + // An interrupted commit left a candidate this boot cannot classify, so + // recovery renamed nothing. Read the last committed image out of the backup + // rather than publishing it: the transaction filenames must survive this boot + // intact for a later one to promote the candidate. + if (recovery.run_from_backup) { + _legacy_tail.valid = false; + const JsonPrefsLoadResult backup_result = + adoptMqttJsonFile(fs, "/mqtt.json.bak", &_mqtt_prefs); + if (jsonPrefsLoaded(backup_result)) { + MESH_DEBUG_PRINTLN("MQTT: running the /mqtt.json backup; candidate preserved and writes held"); + } else { + setMQTTPrefsDefaults(&_mqtt_prefs); + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json backup became unreadable; using defaults (files preserved)"); + } + return; + } + + if (_mqtt_prefs_hold && !fs->exists("/mqtt.json") && + (fs->exists("/mqtt.json.tmp") || fs->exists("/mqtt.json.bak"))) { + _legacy_tail.valid = false; + MESH_DEBUG_PRINTLN("MQTT: unresolved /mqtt.json recovery files; using defaults (files preserved)"); + return; + } + + // The JSON file is authoritative once it exists. Never fall back to the + // stale binary snapshot when JSON is corrupt, unreadable, or from a future + // schema: preserve it and run defaults until an operator resolves it. + if (fs->exists("/mqtt.json")) { + const JsonPrefsLoadResult json_result = + adoptMqttJsonFile(fs, "/mqtt.json", &_mqtt_prefs); + if (jsonPrefsLoaded(json_result)) { + _legacy_tail.valid = false; + if (json_result == JsonPrefsLoadResult::LoadedWithRepairs) { + MESH_DEBUG_PRINTLN("MQTT: repaired out-of-range values in /mqtt.json"); + if (!_mqtt_prefs_hold && !saveMQTTPrefs(fs)) { + _mqtt_prefs_hold = true; + MESH_DEBUG_PRINTLN("MQTT: could not persist /mqtt.json repairs; source held"); + } + } + return; + } + _mqtt_prefs_hold = true; + _legacy_tail.valid = false; + if (json_result == JsonPrefsLoadResult::NoMemory) { + MESH_DEBUG_PRINTLN("MQTT: no memory to validate /mqtt.json; source preserved"); + } else if (json_result == JsonPrefsLoadResult::UnsupportedVersion) { + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json uses a future version; using defaults (file preserved)"); + } else if (json_result == JsonPrefsLoadResult::FutureClaimed) { + MESH_DEBUG_PRINTLN( + "MQTT: /mqtt.json claims a future version but uses unknown grammar; " + "using defaults (file preserved)"); + } else { + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json is invalid or unreadable; using defaults (file preserved)"); + } + return; + } + // Complete or preserve an interrupted SPIFFS transaction before decoding. // A failed recovery leaves the artifacts untouched and blocks this boot from // replacing them with defaults through a later CLI save. - _mqtt_prefs_hold = recoverMqttPrefsFiles(fs); + _mqtt_prefs_hold = _mqtt_prefs_hold || recoverMqttPrefsFiles(fs); bool has_observer_fields = false; bool mqtt_rewrite_pending = false; bool migrated_legacy_mqtt = false; @@ -600,24 +932,34 @@ void CommonCLI::loadMQTTPrefs( } else if (plan.source == MQTTPrefsCodec::Source::Current) { file = openMqttPrefsRead(fs); MQTTPrefsHeader header; - if (!file || file.read((uint8_t *)&header, sizeof(header)) != sizeof(header) || - file.read((uint8_t *)&_mqtt_prefs, plan.payload_len) != plan.payload_len) { + LegacyV1MQTTPrefs* old_prefs = new (std::nothrow) LegacyV1MQTTPrefs; + if (old_prefs) memset(old_prefs, 0, sizeof(*old_prefs)); + if (!old_prefs || !file || file.read((uint8_t *)&header, sizeof(header)) != sizeof(header) || + file.read((uint8_t *)old_prefs, plan.payload_len) != plan.payload_len) { setMQTTPrefsDefaults(&_mqtt_prefs); _mqtt_prefs_hold = true; MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs read failed, using defaults (file preserved)"); + } else if (!MQTTPrefsCodec::isPlausibleV1(*old_prefs, plan.payload_len)) { + setMQTTPrefsDefaults(&_mqtt_prefs); + _mqtt_prefs_hold = true; + MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs v1 content failed plausibility checks; source preserved"); } else { + MQTTPrefsCodec::migrateV1(*old_prefs, plan.payload_len, &_mqtt_prefs); has_observer_fields = plan.observer_fields_present; + mqtt_rewrite_pending = true; + migrated_legacy_mqtt = true; // Written by a later build with appended fields. Everything this // binary knows loaded normally; say so, because the next `set` will // rewrite the file at this length and drop the newer settings. if (file_size - sizeof(MQTTPrefsHeader) > plan.payload_len) { MESH_DEBUG_PRINTLN( "MQTT: /mqtt_prefs written by newer firmware (%u > %u bytes); " - "config loaded, newer settings ignored and dropped on next save", + "known settings loaded; newer binary fields remain in the rollback snapshot", (unsigned)(file_size - sizeof(MQTTPrefsHeader)), (unsigned)plan.payload_len); } } + delete old_prefs; if (file) file.close(); } else if (plan.rewrite_legacy) { bool migrated = false; @@ -674,16 +1016,18 @@ void CommonCLI::loadMQTTPrefs( case MQTTPrefsCodec::Source::LegacySixSlotAudience: case MQTTPrefsCodec::Source::LegacySixSlotAudienceRx: case MQTTPrefsCodec::Source::LegacySixSlot: { - Legacy6SlotMQTTPrefs old_prefs = {}; - if (file.read((uint8_t *)&old_prefs, plan.payload_len) == plan.payload_len) { + Legacy6SlotMQTTPrefs* old_prefs = new (std::nothrow) Legacy6SlotMQTTPrefs; + if (old_prefs) memset(old_prefs, 0, sizeof(*old_prefs)); + if (old_prefs && file.read((uint8_t *)old_prefs, plan.payload_len) == plan.payload_len) { if (MQTTPrefsCodec::isPlausibleLegacy(plan.source, - (const uint8_t *)&old_prefs, plan.payload_len)) { - MQTTPrefsCodec::migrateLegacySixSlot(old_prefs, plan.source, &_mqtt_prefs); + (const uint8_t *)old_prefs, plan.payload_len)) { + MQTTPrefsCodec::migrateLegacySixSlot(*old_prefs, plan.source, &_mqtt_prefs); migrated = true; } else { MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs legacy content failed plausibility checks"); } } + delete old_prefs; break; } default: @@ -742,9 +1086,9 @@ void CommonCLI::loadMQTTPrefs( if (mqtt_rewrite_pending) { legacy_upgrade->requireMqttRewrite(); if (migrated_legacy_mqtt) { - MESH_DEBUG_PRINTLN("MQTT: Migrating headerless /mqtt_prefs to versioned layout"); + MESH_DEBUG_PRINTLN("MQTT: Migrating binary /mqtt_prefs to /mqtt.json"); } else { - MESH_DEBUG_PRINTLN("MQTT: Persisting observer tail into /mqtt_prefs before /com_prefs compaction"); + MESH_DEBUG_PRINTLN("MQTT: Persisting observer tail into /mqtt.json before /com_prefs compaction"); } if (saveMQTTPrefs(fs)) { legacy_upgrade->recordMqttSave(true); @@ -754,7 +1098,7 @@ void CommonCLI::loadMQTTPrefs( // untouched; the next boot can recover the tail and retry the transaction. _mqtt_prefs_hold = true; legacy_upgrade->recordMqttSave(false); - MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs migration save failed; legacy files preserved and held"); + MESH_DEBUG_PRINTLN("MQTT: /mqtt.json migration save failed; legacy files preserved and held"); } } } @@ -763,26 +1107,85 @@ bool CommonCLI::saveMQTTPrefs(FILESYSTEM* fs) { if (_mqtt_prefs_hold) { // Loading deliberately preserved the source file. Do not replace it with this // boot's defaults after an unsupported, corrupt, or temporarily failed read. - MESH_DEBUG_PRINTLN("MQTT: /mqtt_prefs held, not overwriting"); + MESH_DEBUG_PRINTLN("MQTT: observer preference source held, not overwriting /mqtt.json"); return false; } - // Write header and payload sequentially so the transaction needs no second - // full-size (2.8 KiB) staging buffer on constrained targets. The length is - // the shortest that still round-trips this config, so a node with default - // packet filters keeps writing a payload older firmware can read. - const size_t payload_len = MQTTPrefsCodec::payloadLenFor(_mqtt_prefs); - const MQTTPrefsHeader header = MQTTPrefsCodec::makeHeader(payload_len); - MQTTPrefsFileStore store(fs); - const MQTTPrefsAtomicStore::Result result = MQTTPrefsAtomicStore::write( - store, (const uint8_t *)&header, sizeof(header), - (const uint8_t *)&_mqtt_prefs, payload_len); - if (!MQTTPrefsAtomicStore::committed(result)) { - MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt_prefs save failed at %s; source preserved", - mqttPrefsSaveResultName(result)); + MQTTPrefs* repair_defaults = new (std::nothrow) MQTTPrefs; + if (repair_defaults == nullptr) { + MESH_DEBUG_PRINTLN("MQTT: no memory to normalize observer settings before save"); return false; } - return true; + setMQTTPrefsDefaults(repair_defaults); + MQTTPrefsSerializer serializer(&_mqtt_prefs, repair_defaults); + // The serializer hierarchy copies the default values it needs. Release this + // large temporary before file I/O and the independent verification scratch. + delete repair_defaults; + bool repaired = false; + if (!serializer.normalize(&repaired)) return false; + if (repaired) MESH_DEBUG_PRINTLN("MQTT: normalized out-of-range observer settings before save"); + + MQTTPrefsJsonFileStore store(fs); + bool verify_oom = false; + const MQTTPrefsAtomicStore::VerifiedImageResult result = + MQTTPrefsAtomicStore::writeVerifiedImage( + store, + [&]() -> bool { + MQTTPrefsStoreStream stream(&store); + return serializer.saveSerial(stream) && stream.ok(); + }, + [&]() -> bool { + MQTTPrefs* verify = new (std::nothrow) MQTTPrefs; + if (verify == nullptr) { + verify_oom = true; + return false; + } + const JsonPrefsLoadResult verify_result = + loadMqttJsonFile(fs, "/mqtt.json.tmp", verify); + delete verify; + // saveSerial() emits already-normalized values. Needing another + // repair here means save/load is not idempotent. + return verify_result == JsonPrefsLoadResult::Loaded; + }); + + // A recovery file this firmware may still promote is holding the new image, + // so the change cannot be reported as rolled back. No further save can start + // until it is resolved: begin() refuses while either the temp or the backup + // exists, which is exactly the state that gets us here. + if (MQTTPrefsAtomicStore::saveOutcomeUnresolved(result)) _observer_save_indeterminate = true; + + switch (result) { + case MQTTPrefsAtomicStore::VerifiedImageResult::Committed: + return true; + case MQTTPrefsAtomicStore::VerifiedImageResult::BeginFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed at begin; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::WriteFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during write; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::FinishFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during checksum verification; source preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::VerifyFailed: + if (verify_oom) { + MESH_DEBUG_PRINTLN("MQTT: no memory to validate /mqtt.json temp; source preserved"); + } else { + MESH_DEBUG_PRINTLN("MQTT: generated /mqtt.json temp failed schema validation; source preserved"); + } + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::CleanupIndeterminate: + MESH_DEBUG_PRINTLN("MQTT: rejected /mqtt.json temp could not be removed and no primary outranks it; " + "recovery files preserved"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::CommitFailed: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during rename; transaction rolled back"); + break; + case MQTTPrefsAtomicStore::VerifiedImageResult::CommitIndeterminate: + MESH_DEBUG_PRINTLN("MQTT: atomic /mqtt.json save failed during rename and could not be rolled back; " + "recovery files preserved"); + break; + } + return false; } #endif @@ -1419,6 +1822,15 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep strcpy(reply, "Error: delay must be between 0-10000 ms"); } } else if (memcmp(config, "bridge.source ", 14) == 0) { +#ifdef WITH_MQTT_BRIDGE + MQTTPrefs* observer_rollback = new (std::nothrow) MQTTPrefs; + if (observer_rollback == nullptr) { + strcpy(reply, "Error: insufficient memory to update observer setting"); + return; + } + memcpy(observer_rollback, &_mqtt_prefs, sizeof(*observer_rollback)); + const uint8_t old_bridge_pkt_src = _prefs->bridge_pkt_src; +#endif _prefs->bridge_pkt_src = memcmp(&config[14], "rx", 2) == 0; #ifdef WITH_MQTT_BRIDGE if (_prefs->bridge_pkt_src == 1) { @@ -1428,6 +1840,15 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep _mqtt_prefs.mqtt_rx_enabled = 0; _mqtt_prefs.mqtt_tx_enabled = 1; } + _observer_prefs_rollback = observer_rollback; + if (!persistObserverPrefs(reply)) { + _prefs->bridge_pkt_src = old_bridge_pkt_src; + _observer_prefs_rollback = nullptr; + delete observer_rollback; + return; + } + _observer_prefs_rollback = nullptr; + delete observer_rollback; #endif savePrefs(); strcpy(reply, "OK"); diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index e0a0a71b86..42287ffbfd 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -73,7 +73,7 @@ class NodePrefs : public ConfigSerializer { uint8_t extra_sf[4]; // NOTE: observer settings (MQTT/WiFi/timezone/SNMP/alert) are not in NodePrefs. - // They live in MQTTPrefs, persisted separately to /mqtt_prefs, so this struct + // They live in MQTTPrefs, persisted separately to /mqtt.json, so this struct // stays aligned with upstream. See struct MQTTPrefs below. private: @@ -228,6 +228,13 @@ struct LegacyObserverTail { class CommonCLICallbacks { public: virtual void savePrefs() = 0; +#ifdef WITH_MQTT_BRIDGE + virtual bool saveObserverPrefs() = 0; +#else + virtual bool saveObserverPrefs() { + return false; + } +#endif virtual const char* getFirmwareVer() = 0; virtual const char* getBuildDate() = 0; virtual const char* getRole() = 0; @@ -359,10 +366,18 @@ class CommonCLI { char tmp[PRV_KEY_SIZE*2 + 4]; #ifdef WITH_MQTT_BRIDGE MQTTPrefs _mqtt_prefs; + // Points at a per-command snapshot only while an observer setter is running. + // persistObserverPrefs() uses it to undo RAM mutations when flash commit fails. + const MQTTPrefs* _observer_prefs_rollback = nullptr; LegacyObserverTail _legacy_tail; - // /mqtt_prefs is newer, corrupt, or temporarily unreadable. The in-memory prefs + // /mqtt.json is newer, corrupt, or temporarily unreadable. The in-memory prefs // run on defaults and saveMQTTPrefs() must not overwrite the source file. bool _mqtt_prefs_hold = false; + // A failed publish could not be undone, so the next boot may still come up + // with the value that was refused. persistObserverPrefs() must not call that + // a rollback. Latched for the boot: the artifact left behind also makes every + // later transaction fail to begin, so the condition cannot clear itself. + bool _observer_save_indeterminate = false; #endif bool _com_prefs_needs_upgrade = false; // old-format legacy prefs detected; rewrite once after load @@ -384,6 +399,7 @@ class CommonCLI { // false to fall through to the base get/set parsing. bool handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply); bool handleObserverGetCmd(uint32_t sender_timestamp, const char* config, char* reply); + bool persistObserverPrefs(char* reply); // Observer-only top-level commands (ota check/update, tls.bundletest, alert test) // also live in CommonCLI_Observer.cpp; returns true if it handled the command. bool handleObserverCommand(uint32_t sender_timestamp, char* command, char* reply); @@ -393,14 +409,17 @@ class CommonCLI { : _board(&board), _rtc(&rtc), _sensors(&sensors), _region_map(®ion_map), _acl(&acl), _prefs(prefs), _callbacks(callbacks) { } void loadPrefs(FILESYSTEM* _fs); - bool savePrefs(FILESYSTEM* _fs, bool save_mqtt = true); + // Node preferences and observer preferences are separate transactions. + // Callers must explicitly request an observer save when they changed it. + bool savePrefs(FILESYSTEM* _fs, bool save_mqtt = false); void handleCommand(uint32_t sender_timestamp, char* command, char* reply); mesh::MainBoard* getBoard() { return _board; } uint8_t buildAdvertData(uint8_t node_type, uint8_t* app_data); #ifdef WITH_MQTT_BRIDGE - // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt_prefs. + // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // Exposed so the app can hand it to MQTTBridge/AlertReporter, which read these // fields directly (they no longer live in NodePrefs). MQTTPrefs* getObserverPrefs() const { return const_cast(&_mqtt_prefs); } + bool saveObserverPrefs(FILESYSTEM* fs) { return saveMQTTPrefs(fs); } #endif }; diff --git a/src/helpers/CommonCLI_Observer.cpp b/src/helpers/CommonCLI_Observer.cpp index c1b7e74a11..c6fcb60651 100644 --- a/src/helpers/CommonCLI_Observer.cpp +++ b/src/helpers/CommonCLI_Observer.cpp @@ -16,6 +16,7 @@ #include "AlertReporter.h" // for alertReporterBannedChannelMatch[Hex]() #include "MQTTObserverValidation.h" // pure input validators (host-testable) #include +#include #ifdef ESP_PLATFORM #include #include @@ -109,6 +110,46 @@ static bool valueTooLong(const char* val, size_t bufsize, char* reply, const cha return false; } +static bool isObserverPrefsSetCommand(const char* config) { + return strncmp(config, "snmp", 4) == 0 || + strncmp(config, "radio.watchdog", 14) == 0 || + strncmp(config, "mqtt", 4) == 0 || + strncmp(config, "wifi.", 5) == 0 || + strncmp(config, "timezone", 8) == 0 || + strncmp(config, "alert", 5) == 0; +} + +// Keep observer setters atomic from the caller's perspective. The live object +// is restored if its JSON transaction fails, and the snapshot pointer is valid +// only for this synchronous command dispatch. +class ObserverPrefsRollbackScope { +public: + ObserverPrefsRollbackScope(const MQTTPrefs** active_snapshot, + const MQTTPrefs& live, + bool capture) + : _active_snapshot(active_snapshot) { + *_active_snapshot = nullptr; + if (capture) { + _snapshot = new (std::nothrow) MQTTPrefs; + if (_snapshot != nullptr) { + memcpy(_snapshot, &live, sizeof(*_snapshot)); + *_active_snapshot = _snapshot; + } + } + } + + ~ObserverPrefsRollbackScope() { + *_active_snapshot = nullptr; + delete _snapshot; + } + + bool available() const { return _snapshot != nullptr; } + +private: + const MQTTPrefs** _active_snapshot; + MQTTPrefs* _snapshot = nullptr; +}; + static const char* getMQTTPresetNameByIndex(int index) { if (index < MQTT_PRESET_COUNT) return MQTT_PRESETS[index].name; if (index == MQTT_PRESET_COUNT) return MQTT_PRESET_CUSTOM; @@ -161,17 +202,44 @@ static void formatMQTTPresetListReply(char* reply, size_t reply_size, int start) } #endif +bool CommonCLI::persistObserverPrefs(char* reply) { +#ifdef WITH_MQTT_BRIDGE + if (_callbacks->saveObserverPrefs()) return true; + if (_observer_prefs_rollback != nullptr) { + memcpy(&_mqtt_prefs, _observer_prefs_rollback, sizeof(_mqtt_prefs)); + } + // The running node is back on the old value either way, but only claim the + // change is gone when the write really was undone on flash. + if (_observer_save_indeterminate) { + strcpy(reply, "Error: setting not persisted; flash state unresolved, recheck after reboot"); + } else { + strcpy(reply, "Error: setting not persisted; change rolled back"); + } + return false; +#else + (void)reply; + return false; +#endif +} + bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* config, char* reply) { #ifdef WITH_MQTT_BRIDGE + const bool needs_snapshot = isObserverPrefsSetCommand(config); + ObserverPrefsRollbackScope rollback_scope( + &_observer_prefs_rollback, _mqtt_prefs, needs_snapshot); + if (needs_snapshot && !rollback_scope.available()) { + strcpy(reply, "Error: insufficient memory to update observer setting"); + return true; + } bool handled = true; if (memcmp(config, "snmp.community ", 15) == 0) { if (valueTooLong(&config[15], sizeof(_mqtt_prefs.snmp_community), reply, "snmp.community")) return true; StrHelper::strncpy(_mqtt_prefs.snmp_community, &config[15], sizeof(_mqtt_prefs.snmp_community)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - restart to apply"); } else if (memcmp(config, "snmp ", 5) == 0) { _mqtt_prefs.snmp_enabled = memcmp(&config[5], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - restart to apply"); } else if (memcmp(config, "radio.watchdog ", 15) == 0) { const char* val = &config[15]; @@ -189,7 +257,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: radio.watchdog must be 0-120 minutes"); } else { _mqtt_prefs.radio_watchdog_minutes = (uint8_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; if (mins == 0) { strcpy(reply, "OK - radio watchdog disabled"); } else { @@ -200,13 +268,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf #ifdef WITH_MQTT_BRIDGE } else if (strcmp(config, "mqtt.origin") == 0) { _mqtt_prefs.mqtt_origin[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.origin ", 12) == 0) { if (valueTooLong(&config[12], sizeof(_mqtt_prefs.mqtt_origin), reply, "origin")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_origin, &config[12], sizeof(_mqtt_prefs.mqtt_origin)); StrHelper::stripSurroundingQuotes(_mqtt_prefs.mqtt_origin, sizeof(_mqtt_prefs.mqtt_origin)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.iata ", 10) == 0) { const char* iata = &config[10]; @@ -215,7 +283,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // Empty clears the region code (meshcore-topic publishing stays disabled // until one is set). This keeps the pre-existing "clear IATA" capability. _mqtt_prefs.mqtt_iata[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); strcpy(reply, "OK - IATA cleared"); } else { @@ -228,22 +296,22 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf for (int i = 0; _mqtt_prefs.mqtt_iata[i]; i++) { _mqtt_prefs.mqtt_iata[i] = toupper(_mqtt_prefs.mqtt_iata[i]); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); strcpy(reply, "OK"); } } } else if (memcmp(config, "mqtt.status ", 12) == 0) { _mqtt_prefs.mqtt_status_enabled = memcmp(&config[12], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.packets ", 13) == 0) { _mqtt_prefs.mqtt_packets_enabled = memcmp(&config[13], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.raw ", 9) == 0) { _mqtt_prefs.mqtt_raw_enabled = memcmp(&config[9], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.tx ", 8) == 0) { if (memcmp(&config[8], "advert", 6) == 0) { @@ -251,17 +319,17 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { _mqtt_prefs.mqtt_tx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0; } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.rx ", 8) == 0) { _mqtt_prefs.mqtt_rx_enabled = memcmp(&config[8], "on", 2) == 0 ? 1 : 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.interval ", 14) == 0) { uint32_t minutes = _atoi(&config[14]); if (minutes >= 1 && minutes <= 60) { _mqtt_prefs.mqtt_status_interval = minutes * 60000; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridge(); sprintf(reply, "OK - interval set to %u minutes (%lu ms), bridge restarted", minutes, (unsigned long)_mqtt_prefs.mqtt_status_interval); } else { @@ -274,7 +342,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf uint32_t hours = _atoi(&config[24]); if (hours >= MQTT_NEIGHBORS_MIN_INTERVAL_HOURS && hours <= MQTT_NEIGHBORS_MAX_INTERVAL_HOURS) { _mqtt_prefs.mqtt_neighbors_interval = hours * 3600000UL; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - neighbors interval set to %u hours (%lu ms)", (unsigned)hours, (unsigned long)_mqtt_prefs.mqtt_neighbors_interval); } else { @@ -284,7 +352,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // The mesh loop reads this live, so no bridge restart is needed; enabling it // triggers a discovery on the next eligible loop pass. _mqtt_prefs.mqtt_neighbors_enabled = memcmp(&config[15], "on", 2) == 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); #elif defined(WITH_MQTT_BRIDGE) } else if (memcmp(config, "mqtt.neighbors.interval ", 24) == 0 || @@ -303,7 +371,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { StrHelper::strncpy(_mqtt_prefs.mqtt_ntp_server, host, sizeof(_mqtt_prefs.mqtt_ntp_server)); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; #ifdef ESP_PLATFORM // Queue a sync on the MQTT task (Core 0) but do NOT block: this handler // runs on the Arduino loop task, shared with mesh/radio processing and the @@ -325,12 +393,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "wifi.ssid ", 10) == 0) { if (valueTooLong(&config[10], sizeof(_mqtt_prefs.wifi_ssid), reply, "wifi.ssid")) return true; StrHelper::strncpy(_mqtt_prefs.wifi_ssid, &config[10], sizeof(_mqtt_prefs.wifi_ssid)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "wifi.pwd ", 9) == 0) { if (valueTooLong(&config[9], sizeof(_mqtt_prefs.wifi_password), reply, "wifi.pwd")) return true; StrHelper::strncpy(_mqtt_prefs.wifi_password, &config[9], sizeof(_mqtt_prefs.wifi_password)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "wifi.powersave ", 15) == 0) { const char* value = &config[15]; @@ -350,7 +418,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: must be none, min, or max"); } else { _mqtt_prefs.wifi_power_save = ps_value; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; #ifdef ESP_PLATFORM if (WiFi.status() == WL_CONNECTED) { wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE : @@ -374,13 +442,13 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "timezone ", 9) == 0) { if (valueTooLong(&config[9], sizeof(_mqtt_prefs.timezone_string), reply, "timezone")) return true; StrHelper::strncpy(_mqtt_prefs.timezone_string, &config[9], sizeof(_mqtt_prefs.timezone_string)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else if (memcmp(config, "timezone.offset ", 16) == 0) { int8_t offset = _atoi(&config[16]); if (offset >= -12 && offset <= 14) { _mqtt_prefs.timezone_offset = offset; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else { strcpy(reply, "Error: timezone offset must be between -12 and +14"); @@ -397,20 +465,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcmp(preset_name, MQTT_PRESET_CUSTOM) == 0 || strcmp(preset_name, MQTT_PRESET_NONE) == 0) { // Reject duplicate presets (except "none" and "custom") - int dup_slot = -1; - if (findMQTTPreset(preset_name) != nullptr) { - for (int s = 0; s < MAX_MQTT_SLOTS; s++) { - if (s != slot && strcmp(_mqtt_prefs.mqtt_slot_preset[s], preset_name) == 0) { - dup_slot = s; - break; - } - } - } + const int dup_slot = findMQTTPreset(preset_name) == nullptr ? -1 : + mqttAssignedPresetSlot(_mqtt_prefs.mqtt_slot_preset, + preset_name, slot); if (dup_slot >= 0) { sprintf(reply, "Error: preset '%s' is already assigned to slot %d", preset_name, dup_slot + 1); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], preset_name, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); // Check if the slot has everything it needs to connect const MQTTPresetDef* p = findMQTTPreset(preset_name); @@ -469,7 +531,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "server ", 7) == 0) { if (valueTooLong(&subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot]), reply, "server")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_host[slot], &subcmd[7], sizeof(_mqtt_prefs.mqtt_slot_host[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; // Reconfigure the slot so the new host reaches the live connection (other // custom-slot setters do the same; without it the change only applies on // the next reboot/bridge restart). @@ -479,7 +541,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf int port = atoi(&subcmd[5]); if (port > 0 && port <= 65535) { _mqtt_prefs.mqtt_slot_port[slot] = port; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else { @@ -488,19 +550,19 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "username ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot]), reply, "username")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_username[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_username[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(subcmd, "password ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot]), reply, "password")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_password[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_password[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(subcmd, "token ", 6) == 0) { if (valueTooLong(&subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot]), reply, "token")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_token[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_token[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d token set", slot + 1); } else if (memcmp(subcmd, "topic ", 6) == 0) { @@ -510,14 +572,14 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf return true; } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_topic[slot], &subcmd[6], sizeof(_mqtt_prefs.mqtt_slot_topic[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d topic: %s", slot + 1, _mqtt_prefs.mqtt_slot_topic[slot]); } } else if (memcmp(subcmd, "audience ", 9) == 0) { if (valueTooLong(&subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot]), reply, "audience")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_slot_audience[slot], &subcmd[9], sizeof(_mqtt_prefs.mqtt_slot_audience[slot])); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); if (_mqtt_prefs.mqtt_slot_audience[slot][0] != '\0') { sprintf(reply, "OK - slot %d JWT audience: %s", slot + 1, _mqtt_prefs.mqtt_slot_audience[slot]); @@ -527,7 +589,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(subcmd, "audience", 8) == 0 && subcmd[8] == '\0') { // "set mqttN.audience" with no value — clear the audience _mqtt_prefs.mqtt_slot_audience[slot][0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); sprintf(reply, "OK - slot %d JWT audience cleared (using username/password auth)", slot + 1); } else if (strcmp(subcmd, "filter") == 0 || @@ -539,20 +601,18 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: filter must be all, none, or a CSV of types 0-15 / names (advert,txt_msg,...)"); } else { _mqtt_prefs.mqtt_slot_packet_filter[slot] = filter_mask; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; char filter_text[MQTTPacketFilter::kFilterTextSize]; MQTTPacketFilter::format(filter_mask, filter_text, sizeof(filter_text)); snprintf(reply, 160, "OK - slot %d packet types: %s", slot + 1, filter_text); - // A non-default filter extends /mqtt_prefs past what pre-filter - // firmware can read (see MQTTPrefsCodec::payloadLenFor), so say when - // that cost buys nothing: slots beyond the runtime array are never - // published to on this board, the same warning `preset` gives. + // Slots beyond the runtime array are never published to on this board, + // so retain the same inactive-hardware warning `preset` gives. if (slot >= RUNTIME_MQTT_SLOTS && filter_mask != MQTTPacketFilter::kAllPacketTypes) { size_t used = strlen(reply); if (used < 158) { snprintf(reply + used, 160 - used, - " (slot inactive on this hardware; blocks firmware rollback)"); + " (slot inactive on this hardware)"); } } } @@ -562,21 +622,35 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "mqtt.analyzer.us ", 17) == 0) { const int slot = 0; if (memcmp(&config[17], "on", 2) == 0) { + const int dup_slot = mqttAssignedPresetSlot( + _mqtt_prefs.mqtt_slot_preset, "analyzer-us", slot); + if (dup_slot >= 0) { + sprintf(reply, "Error: preset 'analyzer-us' is already assigned to slot %d", + dup_slot + 1); + return true; + } StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-us", sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.analyzer.eu ", 17) == 0) { const int slot = 1; if (memcmp(&config[17], "on", 2) == 0) { + const int dup_slot = mqttAssignedPresetSlot( + _mqtt_prefs.mqtt_slot_preset, "analyzer-eu", slot); + if (dup_slot >= 0) { + sprintf(reply, "Error: preset 'analyzer-eu' is already assigned to slot %d", + dup_slot + 1); + return true; + } StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], "analyzer-eu", sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } else { StrHelper::strncpy(_mqtt_prefs.mqtt_slot_preset[slot], MQTT_PRESET_NONE, sizeof(_mqtt_prefs.mqtt_slot_preset[slot])); } - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->restartBridgeSlot(slot); strcpy(reply, "OK"); } else if (memcmp(config, "mqtt.owner ", 11) == 0) { @@ -585,11 +659,11 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // Owner key is optional — empty clears it (previously this errored, so a // set key could never be removed via the portal/CLI). _mqtt_prefs.mqtt_owner_public_key[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK - owner key cleared"); } else if (mqttOwnerKeyValid(owner_key)) { StrHelper::strncpy(_mqtt_prefs.mqtt_owner_public_key, owner_key, sizeof(_mqtt_prefs.mqtt_owner_public_key)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); } else { strcpy(reply, "Error: public key must be 64 hex characters (32 bytes)"); @@ -597,7 +671,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else if (memcmp(config, "mqtt.email ", 11) == 0) { if (valueTooLong(&config[11], sizeof(_mqtt_prefs.mqtt_email), reply, "email")) return true; StrHelper::strncpy(_mqtt_prefs.mqtt_email, &config[11], sizeof(_mqtt_prefs.mqtt_email)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; strcpy(reply, "OK"); #endif } else if (memcmp(config, "alert ", 6) == 0) { @@ -605,12 +679,12 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf const char* val = &config[6]; if (memcmp(val, "on", 2) == 0 && (val[2] == 0 || val[2] == ' ')) { _mqtt_prefs.alert_enabled = 1; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alerts on"); } else if (memcmp(val, "off", 3) == 0 && (val[3] == 0 || val[3] == ' ')) { _mqtt_prefs.alert_enabled = 0; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alerts off"); } else { @@ -625,7 +699,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf if (len == 0) { _mqtt_prefs.alert_psk_hex[0] = '\0'; _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.psk cleared (alerts disabled until configured)"); } else if (val[0] == '#') { @@ -659,7 +733,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf // The new PSK is operator-supplied, so any previously-derived // hashtag name is no longer accurate provenance — drop it. _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.psk updated"); } @@ -672,7 +746,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf if (in_len == 0) { _mqtt_prefs.alert_psk_hex[0] = '\0'; _mqtt_prefs.alert_hashtag[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.hashtag cleared (alerts disabled until configured)"); } else { @@ -708,7 +782,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf mesh::Utils::toHex(hex, digest, 16); StrHelper::strncpy(_mqtt_prefs.alert_hashtag, hashtag, sizeof(_mqtt_prefs.alert_hashtag)); StrHelper::strncpy(_mqtt_prefs.alert_psk_hex, hex, sizeof(_mqtt_prefs.alert_psk_hex)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); sprintf(reply, "OK - alert.hashtag: %s", _mqtt_prefs.alert_hashtag); } @@ -726,7 +800,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf size_t len = strlen(val); if (len == 0) { _mqtt_prefs.alert_region[0] = '\0'; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); strcpy(reply, "OK - alert.region cleared (using default scope)"); } else if (len >= sizeof(_mqtt_prefs.alert_region)) { @@ -734,7 +808,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf } else { StrHelper::strncpy(_mqtt_prefs.alert_region, val, sizeof(_mqtt_prefs.alert_region)); StrHelper::stripSurroundingQuotes(_mqtt_prefs.alert_region, sizeof(_mqtt_prefs.alert_region)); - savePrefs(); + if (!persistObserverPrefs(reply)) return true; _callbacks->onAlertConfigChanged(); sprintf(reply, "OK - alert.region: %s", _mqtt_prefs.alert_region); } @@ -744,7 +818,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.wifi must be 0-1440 minutes (0=off)"); } else { _mqtt_prefs.alert_wifi_minutes = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.wifi %d min%s", mins, mins == 0 ? " (disabled)" : ""); } } else if (memcmp(config, "alert.mqtt ", 11) == 0) { @@ -753,7 +827,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.mqtt must be 0-10080 minutes (0=off)"); } else { _mqtt_prefs.alert_mqtt_minutes = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.mqtt %d min%s", mins, mins == 0 ? " (disabled)" : ""); } } else if (memcmp(config, "alert.interval ", 15) == 0) { @@ -764,7 +838,7 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf strcpy(reply, "Error: alert.interval must be 60-10080 minutes"); } else { _mqtt_prefs.alert_min_interval_min = (uint16_t)mins; - savePrefs(); + if (!persistObserverPrefs(reply)) return true; sprintf(reply, "OK - alert.interval %d min", mins); } } else { diff --git a/src/helpers/ConfigSerializer.cpp b/src/helpers/ConfigSerializer.cpp index adff147f47..b8788bf5f9 100644 --- a/src/helpers/ConfigSerializer.cpp +++ b/src/helpers/ConfigSerializer.cpp @@ -1,5 +1,9 @@ #include "ConfigSerializer.h" +#include +#include +#include + bool ConfigSerializer::saveSerial(Stream& s) { Context context(&s, OP::WRITE); _context = &context; // set the context for structure() call @@ -22,9 +26,12 @@ bool ConfigSerializer::saveSerial(Stream& s) { static bool is_whitespace(char c) { return c == ' ' || c == '\t' || c == '\r' || c == '\n'; } -static bool is_key_char(char c) { +static bool is_key_start_char(char c) { return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_'; } +static bool is_key_char(char c) { + return is_key_start_char(c) || (c >= '0' && c <= '9'); +} static bool is_value_char(char c) { return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || c == '-' || c == '.'; } @@ -64,14 +71,26 @@ int ConfigSerializer::Context::readNext() { case EXPECT_KEY: if (rd_len > 0 && c == ':') { rd_buf[rd_len] = 0; rd_len = 0; rd_mode = EXPECT_VAL_OR_OBJ; return TOK_KEY; } if (rd_len == 0 && is_whitespace(c)) return TOK_WHITESPACE; - if (rd_len < CONFIG_MAX_KEYLEN-1 && is_key_char(c)) { rd_buf[rd_len++] = c; return TOK_WHITESPACE; } + if (rd_len < CONFIG_MAX_KEYLEN-1 && + ((rd_len == 0 && is_key_start_char(c)) || (rd_len > 0 && is_key_char(c)))) { + rd_buf[rd_len++] = c; + return TOK_WHITESPACE; + } return TOK_ERROR; case EXPECT_VAL_OR_OBJ: if (rd_len == 0 && is_whitespace(c)) return TOK_WHITESPACE; - if (rd_len == 0 && c == '"') { rd_mode = EXPECT_STRING_VAL; return TOK_WHITESPACE; } + if (rd_len == 0 && c == '"') { + rd_token_quoted = true; + rd_mode = EXPECT_STRING_VAL; + return TOK_WHITESPACE; + } if (rd_len == 0 && c == '{') { rd_mode = EXPECT_KEY; return TOK_START_OBJ; } - if (is_value_char(c) && rd_len < CONFIG_MAX_TOKEN_LEN-1) { rd_buf[rd_len++] = c; return TOK_WHITESPACE; } + if (is_value_char(c) && rd_len < CONFIG_MAX_TOKEN_LEN-1) { + if (rd_len == 0) rd_token_quoted = false; + rd_buf[rd_len++] = c; + return TOK_WHITESPACE; + } if (rd_len > 0 && (c == ',' || c == '}' || is_whitespace(c))) { pending = c; rd_buf[rd_len] = 0; rd_len = 0; rd_mode = EXPECT_COMMA_OR_CLOSE; return TOK_VALUE; } return TOK_ERROR; @@ -107,9 +126,19 @@ bool ConfigSerializer::loadSerial(Stream& s) { if (next_tok == TOK_KEY) { context.setKey(sp, context.getToken()); } else if (next_tok == TOK_VALUE) { + context.setValueEvent(sp, false); _depth = 1; // re-run the structure() hierarchy again (looking for specific key, at specific depth) structure(); } else if (next_tok == TOK_START_OBJ) { + // The root has no key. For every nested object, rerun the schema at the + // parent depth so strict scalar fields can reject an object value and + // object fields can confirm that their value has the expected shape. + if (sp > 0) { + context.setValueEvent(sp, true); + _depth = 1; + structure(); + if (!context.success) break; + } if (sp < CONFIG_MAX_DEPTH - 1) { sp++; } else { @@ -153,6 +182,10 @@ void ConfigSerializer::def(const char* key, void* value, size_t len) { _context->file()->print("\""); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } memset(value, 0, len); mesh::Utils::fromHex((uint8_t *)value, len, _context->getToken()); } @@ -181,6 +214,10 @@ void ConfigSerializer::def(const char* key, char* value, size_t max_len) { _context->file()->print("\""); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } strncpy(value, _context->getToken(), max_len - 1); value[max_len - 1] = 0; } @@ -195,6 +232,10 @@ void ConfigSerializer::def(const char* key, int32_t& value) { _context->file()->print(value); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -208,6 +249,10 @@ void ConfigSerializer::def(const char* key, uint32_t& value) { _context->file()->print(value); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -221,6 +266,10 @@ void ConfigSerializer::def(const char* key, int16_t& value) { _context->file()->print((int32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atol(_context->getToken()); } } @@ -234,6 +283,10 @@ void ConfigSerializer::def(const char* key, uint16_t& value) { _context->file()->print((uint32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -247,6 +300,10 @@ void ConfigSerializer::def(const char* key, uint8_t& value) { _context->file()->print((uint32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -260,6 +317,10 @@ void ConfigSerializer::def(const char* key, int8_t& value) { _context->file()->print((int32_t) value, 10); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atoi(_context->getToken()); } } @@ -273,6 +334,10 @@ void ConfigSerializer::def(const char* key, bool& value) { _context->file()->print(value ? "true" : "false"); } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = strcmp(_context->getToken(), "true") == 0 || atoi(_context->getToken()) != 0; // 'true' or a non-zero number } } @@ -290,6 +355,10 @@ void ConfigSerializer::def(const char* key, double& value) { } } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = atof(_context->getToken()); } } @@ -307,6 +376,10 @@ void ConfigSerializer::def(const char* key, float& value) { } } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() || _context->valueDepth() != _depth) { + _context->success = false; + return; + } value = (float) atof(_context->getToken()); } } @@ -323,9 +396,64 @@ void ConfigSerializer::def(const char* key, ConfigSerializer& sub_obj) { if (_context->file()->print("}") != 1) _context->success = false; // failure detect } else { if (_context->keyMatch(_depth, key)) { + if (_context->objectStart() && _context->valueDepth() == _depth) { + return; // the object itself; descendant values recurse below + } + if (_context->valueDepth() <= _depth) { + _context->success = false; // known object supplied as a scalar + return; + } sub_obj._context = _context; // inherit the Context sub_obj._depth = _depth + 1; sub_obj.structure(); // recurse into sub object } } } + +bool ConfigSerializer::defStrict(const char* key, char* value, size_t max_len, bool& seen) { + if (_context->op() == OP::WRITE) { + def(key, value, max_len); + return true; + } + if (!_context->keyMatch(_depth, key)) return false; + if (seen || max_len == 0 || _context->objectStart() || + _context->valueDepth() != _depth || !_context->tokenQuoted()) { + _context->success = false; + return false; + } + seen = true; + const char* token = _context->getToken(); + const size_t len = strlen(token); + if (len >= max_len) { + _context->success = false; + return false; + } + memcpy(value, token, len + 1); + return true; +} + +bool ConfigSerializer::defStrict(const char* key, int32_t& value, bool& seen) { + if (_context->op() == OP::WRITE) { + def(key, value); + return true; + } + if (!_context->keyMatch(_depth, key)) return false; + if (seen || _context->objectStart() || _context->valueDepth() != _depth || + _context->tokenQuoted()) { + _context->success = false; + return false; + } + seen = true; + + const char* token = _context->getToken(); + char* end = nullptr; + errno = 0; + const long long parsed = strtoll(token, &end, 10); + if (token[0] == '\0' || end == token || *end != '\0' || errno == ERANGE || + parsed < INT32_MIN || parsed > INT32_MAX) { + _context->success = false; + return false; + } + value = static_cast(parsed); + return true; +} diff --git a/src/helpers/ConfigSerializer.h b/src/helpers/ConfigSerializer.h index 7e6d6f2a69..e889e6379d 100644 --- a/src/helpers/ConfigSerializer.h +++ b/src/helpers/ConfigSerializer.h @@ -25,17 +25,35 @@ class ConfigSerializer { OP _op; uint8_t rd_len; uint8_t rd_mode; + uint8_t rd_value_depth; char pending; + bool rd_token_quoted; + bool rd_object_start; char rd_buf[CONFIG_MAX_TOKEN_LEN]; char _keys[CONFIG_MAX_DEPTH][CONFIG_MAX_KEYLEN]; public: bool success = true; - Context(Stream* f, OP op) : _f(f), _op(op) { rd_buf[rd_len = 0] = 0; rd_mode = 0; pending = 0; } + Context(Stream* f, OP op) : _f(f), _op(op) { + rd_buf[rd_len = 0] = 0; + rd_mode = 0; + rd_value_depth = 0; + pending = 0; + rd_token_quoted = false; + rd_object_start = false; + memset(_keys, 0, sizeof(_keys)); + } OP op() const { return _op; } Stream* file() const { return _f; } int readNext(); const char* getToken() const { return rd_buf; } + bool tokenQuoted() const { return rd_token_quoted; } + uint8_t valueDepth() const { return rd_value_depth; } + bool objectStart() const { return rd_object_start; } + void setValueEvent(uint8_t depth, bool object_start) { + rd_value_depth = depth; + rd_object_start = object_start; + } bool keyMatch(int8_t depth, const char* key) { return strcmp(key, _keys[depth]) == 0; } void setKey(uint8_t depth, const char* key) { strcpy(_keys[depth], key); } }; @@ -60,6 +78,67 @@ class ConfigSerializer { void def(const char* key, bool& value); void def(const char* key, ConfigSerializer& sub_obj); + // Strict read helpers for schemas whose values may be edited outside the + // firmware. Unlike the legacy def() overloads, these reject duplicate keys, + // truncated strings, malformed integers, and integer overflow. `seen` must + // be a field owned by the schema object and initialized false before load. + bool defStrict(const char* key, char* value, size_t max_len, bool& seen); + bool defStrict(const char* key, int32_t& value, bool& seen); + + // Literal keys are checked where a schema defines them, so adding a field + // that an older ConfigSerializer cannot tokenize is a build failure rather + // than a silent downgrade trap. Dynamic keys retain the pointer overloads. + template static void checkKey(const char (&)[N]) { + static_assert(N <= CONFIG_MAX_KEYLEN, + "ConfigSerializer key exceeds the visible-key limit"); + } + template void def(const char (&key)[N], char* value, size_t max_len) { + checkKey(key); def(static_cast(key), value, max_len); + } + template void def(const char (&key)[N], void* value, size_t len) { + checkKey(key); def(static_cast(key), value, len); + } + template void def(const char (&key)[N], int32_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], int16_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], int8_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint32_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint16_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], uint8_t& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], float& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], double& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], bool& value) { + checkKey(key); def(static_cast(key), value); + } + template void def(const char (&key)[N], ConfigSerializer& sub_obj) { + checkKey(key); def(static_cast(key), sub_obj); + } + template + bool defStrict(const char (&key)[N], char* value, size_t max_len, bool& seen) { + checkKey(key); + return defStrict(static_cast(key), value, max_len, seen); + } + template + bool defStrict(const char (&key)[N], int32_t& value, bool& seen) { + checkKey(key); + return defStrict(static_cast(key), value, seen); + } + virtual void structure() = 0; public: diff --git a/src/helpers/MQTTDefaults.h b/src/helpers/MQTTDefaults.h index 5072e4229f..21c53bc46b 100644 --- a/src/helpers/MQTTDefaults.h +++ b/src/helpers/MQTTDefaults.h @@ -8,7 +8,7 @@ #include "MQTTPacketFilter.h" #include "MQTTPresets.h" -// Compile-time defaults for fresh /mqtt_prefs (override via platformio build_flags). +// Compile-time defaults for fresh /mqtt.json (override via platformio build_flags). // Example: // -D MQTT_DEFAULT_SLOT1_PRESET='"meshcore-ca-1"' // -D MQTT_DEFAULT_IATA='"YYZ"' diff --git a/src/helpers/MQTTObserverValidation.h b/src/helpers/MQTTObserverValidation.h index 2042f2267c..a8d465a77a 100644 --- a/src/helpers/MQTTObserverValidation.h +++ b/src/helpers/MQTTObserverValidation.h @@ -58,3 +58,20 @@ static inline bool mqttNtpHostnameValid(const char* host) { static inline bool mqttValueFits(const char* s, size_t bufsize) { return s != NULL && bufsize > 0 && strlen(s) < bufsize; } + +// Find a preset already assigned to another slot. The caller decides which +// names are singleton presets ("none" and "custom" intentionally are not). +template +static inline int mqttAssignedPresetSlot( + const char (&presets)[SlotCount][PresetSize], + const char* preset_name, + int ignored_slot) { + if (preset_name == NULL) return -1; + for (size_t slot = 0; slot < SlotCount; ++slot) { + if (static_cast(slot) != ignored_slot && + strcmp(presets[slot], preset_name) == 0) { + return static_cast(slot); + } + } + return -1; +} diff --git a/src/helpers/MQTTPacketFilter.h b/src/helpers/MQTTPacketFilter.h index df81182827..47fbfe5c21 100644 --- a/src/helpers/MQTTPacketFilter.h +++ b/src/helpers/MQTTPacketFilter.h @@ -71,8 +71,8 @@ inline bool resolveToken(const char* begin, size_t len, uint8_t* type_out) { } // Parse an allowlist value. Empty input means "all" so WebConfig can clear a -// field and retain the same backwards-compatible default as an older -// /mqtt_prefs file. Keywords and type names are deliberately lowercase; "all" +// field and retain the same all-packet-types default as an older preference +// file. Keywords and type names are deliberately lowercase; "all" // and "none" cannot be mixed into a list. Entries may be decimal (0..15) or // named, may carry surrounding ASCII whitespace, and may be repeated. inline bool parse(const char* input, uint16_t* mask_out) { @@ -186,8 +186,8 @@ inline uint16_t enabledUnion(const uint16_t* masks, const bool* enabled, size_t return combined; } -// True when every slot still carries the default all-types mask, i.e. nothing -// depends on the packet-filter tail of /mqtt_prefs being written. +// True when every slot still carries the default all-types mask. The legacy +// binary encoder uses this while producing migration fixtures. inline bool allMasksDefault(const uint16_t* masks, size_t count) { if (masks == nullptr) return true; for (size_t i = 0; i < count; ++i) { diff --git a/src/helpers/MQTTPrefsAtomicStore.h b/src/helpers/MQTTPrefsAtomicStore.h index be77bf99e1..373dfa3b58 100644 --- a/src/helpers/MQTTPrefsAtomicStore.h +++ b/src/helpers/MQTTPrefsAtomicStore.h @@ -3,12 +3,82 @@ #include #include -// Transactional writer for /mqtt_prefs. The Store interface is intentionally -// narrow so host tests can exercise every failure boundary without an Arduino -// filesystem: begin(), write(), finish(), commit(), and abort(). The caller -// supplies header and payload separately, avoiding a second full-size buffer. +// Transactional writer policy for preferences. The Store interface is +// intentionally narrow so host tests can exercise every failure boundary +// without an Arduino filesystem. namespace MQTTPrefsAtomicStore { +// Production /mqtt.json flow. finish() verifies the exact bytes written, +// verify_image reparses the finished temp through the schema, and only then is +// commit() allowed to publish it. A schema-verification failure discards the +// finished temp; a commit failure is undone by rollbackFailedCommit(). +// +// A failed save that is really gone and one that may still surface at the next +// boot are distinct outcomes, not shades of the same one. Publishing moves the +// old primary aside before the verified temp takes its name, so a half-done +// commit leaves an image that boot recovery would promote; CommitFailed means +// that was undone, CommitIndeterminate means it could not be. The same applies +// before the commit: a rejected temp that cleanup could not delete is still an +// image recovery may promote when no primary outranks it, which is +// CleanupIndeterminate. Callers must not report these as a rollback. +enum class VerifiedImageResult : uint8_t { + Committed, + BeginFailed, + WriteFailed, + FinishFailed, + VerifyFailed, + CleanupIndeterminate, + CommitFailed, + CommitIndeterminate, +}; + +// The change may still be on flash at the next boot, so the caller must not +// tell the operator it was rolled back. +inline bool saveOutcomeUnresolved(VerifiedImageResult result) { + return result == VerifiedImageResult::CleanupIndeterminate || + result == VerifiedImageResult::CommitIndeterminate; +} + +template +inline VerifiedImageResult writeVerifiedImage(Store& store, + ImageWriter write_image, + ImageVerifier verify_image) { + if (!store.begin()) { + store.abort(); + return VerifiedImageResult::BeginFailed; + } + if (!write_image()) { + // A short write leaves a structurally incomplete image that recovery + // classifies as invalid, so a temp that survives cleanup here cannot be + // promoted and the change really is gone. + store.abort(); + return VerifiedImageResult::WriteFailed; + } + if (!store.finish()) { + // Unlike a short write, this does not prove the bytes on disk are + // unusable: finish() also fails when a complete temp cannot be read back. + return store.abort() ? VerifiedImageResult::FinishFailed + : VerifiedImageResult::CleanupIndeterminate; + } + if (!verify_image()) { + // The temp is complete and byte-verified, only rejected by the schema. If + // it cannot be deleted and no primary outranks it, boot recovery promotes + // the very image this call is about to report as not saved. + return store.discardFinishedTemp() ? VerifiedImageResult::VerifyFailed + : VerifiedImageResult::CleanupIndeterminate; + } + if (!store.commit()) { + // Undo the partial publish before answering. Only the store knows whether + // the pre-transaction state was actually restored, so take its word for + // which failure this was. + const bool rolled_back = store.rollbackFailedCommit(); + store.abort(); + return rolled_back ? VerifiedImageResult::CommitFailed + : VerifiedImageResult::CommitIndeterminate; + } + return VerifiedImageResult::Committed; +} + enum class Result : uint8_t { Committed, BeginFailed, @@ -59,7 +129,7 @@ inline ImageResult writeImage(Store& store, ImageWriter write_image) { } // Coordinates a two-file legacy upgrade. /com_prefs must not be compacted -// until the observer tail it carries has been published into /mqtt_prefs. +// until the observer tail it carries has been published into /mqtt.json. // Keeping this state in a tiny pure helper lets host tests cover power-cut // boundaries without an Arduino filesystem. class LegacyUpgradeGate { diff --git a/src/helpers/MQTTPrefsCodec.h b/src/helpers/MQTTPrefsCodec.h index 7bbfe30aeb..5ac073c083 100644 --- a/src/helpers/MQTTPrefsCodec.h +++ b/src/helpers/MQTTPrefsCodec.h @@ -53,6 +53,9 @@ static const size_t kEncodedSize = sizeof(MQTTPrefsHeader) + kV1BaselinePayloadS // refuses to overwrite the file, so the node cannot be recovered over the air. // Touching any filter opts that node into the longer payload — a deliberate, // operator-initiated trade rather than a side effect of upgrading. +// The encoder below is retained for host migration fixtures and downgrade +// compatibility tests. Production firmware reads legacy binary files but only +// writes the JSON schema. inline size_t payloadLenFor(const MQTTPrefs& prefs) { return MQTTPacketFilter::allMasksDefault(prefs.mqtt_slot_packet_filter, MQTT_PREFS_SLOT_COUNT) @@ -68,13 +71,60 @@ inline MQTTPrefsHeader makeHeader(size_t payload_len) { return header; } +inline void freezeV1(const MQTTPrefs& prefs, LegacyV1MQTTPrefs* frozen) { + if (frozen == nullptr) return; + memset(frozen, 0, sizeof(*frozen)); + memcpy(frozen->mqtt_origin, prefs.mqtt_origin, sizeof(frozen->mqtt_origin)); + memcpy(frozen->mqtt_iata, prefs.mqtt_iata, sizeof(frozen->mqtt_iata)); + frozen->mqtt_status_enabled = prefs.mqtt_status_enabled; + frozen->mqtt_packets_enabled = prefs.mqtt_packets_enabled; + frozen->mqtt_raw_enabled = prefs.mqtt_raw_enabled; + frozen->mqtt_tx_enabled = prefs.mqtt_tx_enabled; + frozen->mqtt_status_interval = prefs.mqtt_status_interval; + memcpy(frozen->wifi_ssid, prefs.wifi_ssid, sizeof(frozen->wifi_ssid)); + memcpy(frozen->wifi_password, prefs.wifi_password, sizeof(frozen->wifi_password)); + frozen->wifi_power_save = prefs.wifi_power_save; + memcpy(frozen->timezone_string, prefs.timezone_string, sizeof(frozen->timezone_string)); + frozen->timezone_offset = prefs.timezone_offset; + memcpy(frozen->mqtt_slot_preset, prefs.mqtt_slot_preset, sizeof(frozen->mqtt_slot_preset)); + memcpy(frozen->mqtt_slot_host, prefs.mqtt_slot_host, sizeof(frozen->mqtt_slot_host)); + memcpy(frozen->mqtt_slot_port, prefs.mqtt_slot_port, sizeof(frozen->mqtt_slot_port)); + memcpy(frozen->mqtt_slot_username, prefs.mqtt_slot_username, sizeof(frozen->mqtt_slot_username)); + memcpy(frozen->mqtt_slot_password, prefs.mqtt_slot_password, sizeof(frozen->mqtt_slot_password)); + memcpy(frozen->mqtt_owner_public_key, prefs.mqtt_owner_public_key, + sizeof(frozen->mqtt_owner_public_key)); + memcpy(frozen->mqtt_email, prefs.mqtt_email, sizeof(frozen->mqtt_email)); + memcpy(frozen->mqtt_slot_token, prefs.mqtt_slot_token, sizeof(frozen->mqtt_slot_token)); + memcpy(frozen->mqtt_slot_topic, prefs.mqtt_slot_topic, sizeof(frozen->mqtt_slot_topic)); + memcpy(frozen->mqtt_slot_audience, prefs.mqtt_slot_audience, + sizeof(frozen->mqtt_slot_audience)); + frozen->mqtt_rx_enabled = prefs.mqtt_rx_enabled; + memcpy(frozen->mqtt_ntp_server, prefs.mqtt_ntp_server, sizeof(frozen->mqtt_ntp_server)); + frozen->snmp_enabled = prefs.snmp_enabled; + memcpy(frozen->snmp_community, prefs.snmp_community, sizeof(frozen->snmp_community)); + frozen->radio_watchdog_minutes = prefs.radio_watchdog_minutes; + frozen->alert_enabled = prefs.alert_enabled; + memcpy(frozen->alert_psk_hex, prefs.alert_psk_hex, sizeof(frozen->alert_psk_hex)); + frozen->alert_wifi_minutes = prefs.alert_wifi_minutes; + frozen->alert_mqtt_minutes = prefs.alert_mqtt_minutes; + frozen->alert_min_interval_min = prefs.alert_min_interval_min; + memcpy(frozen->alert_hashtag, prefs.alert_hashtag, sizeof(frozen->alert_hashtag)); + memcpy(frozen->alert_region, prefs.alert_region, sizeof(frozen->alert_region)); + frozen->mqtt_neighbors_enabled = prefs.mqtt_neighbors_enabled; + frozen->mqtt_neighbors_interval = prefs.mqtt_neighbors_interval; + memcpy(frozen->mqtt_slot_packet_filter, prefs.mqtt_slot_packet_filter, + sizeof(frozen->mqtt_slot_packet_filter)); +} + inline size_t encode(const MQTTPrefs& prefs, uint8_t* output, size_t output_size) { const size_t payload_len = payloadLenFor(prefs); const size_t encoded_size = sizeof(MQTTPrefsHeader) + payload_len; if (output == nullptr || output_size < encoded_size) return 0; const MQTTPrefsHeader header = makeHeader(payload_len); + LegacyV1MQTTPrefs frozen; + freezeV1(prefs, &frozen); memcpy(output, &header, sizeof(header)); - memcpy(output + sizeof(header), &prefs, payload_len); + memcpy(output + sizeof(header), &frozen, payload_len); return encoded_size; } @@ -346,6 +396,120 @@ inline bool isPlausibleLegacy(Source source, const uint8_t* input, size_t size) } } +// A valid v1 header proves the intended layout, not that a torn or corrupted +// payload still contains bounded C strings. Validate every field present at a +// shipped v1 boundary before copying it into the runtime object; otherwise the +// later semantic validators and JSON writer could scan beyond a fixed array. +inline bool isPlausibleV1(const LegacyV1MQTTPrefs& prefs, size_t payload_len) { + if (payload_len != kV1PreObserverPayloadSize && + payload_len != kV1PreNeighborsPayloadSize && + payload_len != kV1PreFilterPayloadSize && + payload_len != kV1BaselinePayloadSize) { + return false; + } + const uint8_t* input = reinterpret_cast(&prefs); + // A v1 header already identifies the layout. At this stage only reject + // fields that could make later C-string validation/serialization read past a + // fixed array; numeric values are safely repaired by MQTTPrefsSerializer. + if (!hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_origin), sizeof(prefs.mqtt_origin)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_iata), sizeof(prefs.mqtt_iata)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, wifi_ssid), sizeof(prefs.wifi_ssid)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, wifi_password), sizeof(prefs.wifi_password)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, timezone_string), sizeof(prefs.timezone_string)) || + !hasPlausibleSharedAuth(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_owner_public_key), + offsetof(LegacyV1MQTTPrefs, mqtt_email)) || + !hasPlausibleSlotText(input, payload_len, MQTT_PREFS_SLOT_COUNT, + offsetof(LegacyV1MQTTPrefs, mqtt_slot_preset), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_host), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_username), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_password), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_token), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_topic), + offsetof(LegacyV1MQTTPrefs, mqtt_slot_audience)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, mqtt_ntp_server), + sizeof(prefs.mqtt_ntp_server))) { + return false; + } + if (payload_len >= kV1PreNeighborsPayloadSize) { + if (!hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, snmp_community), + sizeof(prefs.snmp_community)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_psk_hex), + sizeof(prefs.alert_psk_hex)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_hashtag), + sizeof(prefs.alert_hashtag)) || + !hasTerminatedText(input, payload_len, + offsetof(LegacyV1MQTTPrefs, alert_region), + sizeof(prefs.alert_region))) { + return false; + } + } + return true; +} + +inline void migrateV1(const LegacyV1MQTTPrefs& old_prefs, size_t payload_len, + MQTTPrefs* prefs) { + if (prefs == nullptr || payload_len < kV1PreObserverPayloadSize) return; + memcpy(prefs->mqtt_origin, old_prefs.mqtt_origin, sizeof(prefs->mqtt_origin)); + memcpy(prefs->mqtt_iata, old_prefs.mqtt_iata, sizeof(prefs->mqtt_iata)); + prefs->mqtt_status_enabled = old_prefs.mqtt_status_enabled; + prefs->mqtt_packets_enabled = old_prefs.mqtt_packets_enabled; + prefs->mqtt_raw_enabled = old_prefs.mqtt_raw_enabled; + prefs->mqtt_tx_enabled = old_prefs.mqtt_tx_enabled; + prefs->mqtt_status_interval = old_prefs.mqtt_status_interval; + memcpy(prefs->wifi_ssid, old_prefs.wifi_ssid, sizeof(prefs->wifi_ssid)); + memcpy(prefs->wifi_password, old_prefs.wifi_password, sizeof(prefs->wifi_password)); + prefs->wifi_power_save = old_prefs.wifi_power_save; + memcpy(prefs->timezone_string, old_prefs.timezone_string, sizeof(prefs->timezone_string)); + prefs->timezone_offset = old_prefs.timezone_offset; + memcpy(prefs->mqtt_slot_preset, old_prefs.mqtt_slot_preset, sizeof(prefs->mqtt_slot_preset)); + memcpy(prefs->mqtt_slot_host, old_prefs.mqtt_slot_host, sizeof(prefs->mqtt_slot_host)); + memcpy(prefs->mqtt_slot_port, old_prefs.mqtt_slot_port, sizeof(prefs->mqtt_slot_port)); + memcpy(prefs->mqtt_slot_username, old_prefs.mqtt_slot_username, sizeof(prefs->mqtt_slot_username)); + memcpy(prefs->mqtt_slot_password, old_prefs.mqtt_slot_password, sizeof(prefs->mqtt_slot_password)); + memcpy(prefs->mqtt_owner_public_key, old_prefs.mqtt_owner_public_key, + sizeof(prefs->mqtt_owner_public_key)); + memcpy(prefs->mqtt_email, old_prefs.mqtt_email, sizeof(prefs->mqtt_email)); + memcpy(prefs->mqtt_slot_token, old_prefs.mqtt_slot_token, sizeof(prefs->mqtt_slot_token)); + memcpy(prefs->mqtt_slot_topic, old_prefs.mqtt_slot_topic, sizeof(prefs->mqtt_slot_topic)); + memcpy(prefs->mqtt_slot_audience, old_prefs.mqtt_slot_audience, + sizeof(prefs->mqtt_slot_audience)); + prefs->mqtt_rx_enabled = old_prefs.mqtt_rx_enabled; + memcpy(prefs->mqtt_ntp_server, old_prefs.mqtt_ntp_server, sizeof(prefs->mqtt_ntp_server)); + + if (payload_len >= kV1PreNeighborsPayloadSize) { + prefs->snmp_enabled = old_prefs.snmp_enabled; + memcpy(prefs->snmp_community, old_prefs.snmp_community, sizeof(prefs->snmp_community)); + prefs->radio_watchdog_minutes = old_prefs.radio_watchdog_minutes; + prefs->alert_enabled = old_prefs.alert_enabled; + memcpy(prefs->alert_psk_hex, old_prefs.alert_psk_hex, sizeof(prefs->alert_psk_hex)); + prefs->alert_wifi_minutes = old_prefs.alert_wifi_minutes; + prefs->alert_mqtt_minutes = old_prefs.alert_mqtt_minutes; + prefs->alert_min_interval_min = old_prefs.alert_min_interval_min; + memcpy(prefs->alert_hashtag, old_prefs.alert_hashtag, sizeof(prefs->alert_hashtag)); + memcpy(prefs->alert_region, old_prefs.alert_region, sizeof(prefs->alert_region)); + // The pre-neighbors payload includes the old zero padding byte at this + // offset; copying it preserves the deployed format's disabled default. + prefs->mqtt_neighbors_enabled = old_prefs.mqtt_neighbors_enabled; + } + if (payload_len >= kV1PreFilterPayloadSize) { + prefs->mqtt_neighbors_interval = old_prefs.mqtt_neighbors_interval; + } + if (payload_len >= kV1BaselinePayloadSize) { + memcpy(prefs->mqtt_slot_packet_filter, old_prefs.mqtt_slot_packet_filter, + sizeof(prefs->mqtt_slot_packet_filter)); + } +} + inline void migratePreSlot(const OldMQTTPrefs& old_prefs, MQTTPrefs* prefs) { memcpy(prefs->mqtt_origin, old_prefs.mqtt_origin, sizeof(prefs->mqtt_origin)); memcpy(prefs->mqtt_iata, old_prefs.mqtt_iata, sizeof(prefs->mqtt_iata)); diff --git a/src/helpers/MQTTPrefsRecovery.h b/src/helpers/MQTTPrefsRecovery.h index 7534678eb3..15eb014e4a 100644 --- a/src/helpers/MQTTPrefsRecovery.h +++ b/src/helpers/MQTTPrefsRecovery.h @@ -2,27 +2,53 @@ #include -// Pure recovery policy for the three MQTT preference transaction files. The +// Pure recovery policy for the three MQTT preference transaction files. The // writer first moves the old primary to .bak, then moves the verified .tmp to -// the primary name. On a reset, the loader uses this policy before decoding -// /mqtt_prefs. "Preserve" is deliberately distinct from "Usable": it covers -// an unsupported newer layout, corruption, or an unreadable file and must -// never be replaced by an older image. +// the primary name. On a reset, the loader uses this policy before decoding +// the primary. FutureUsable is syntactically valid but belongs to newer +// firmware. FutureClaimed and Indeterminate cannot be safely classified by +// this firmware, so recovery may run a known-good backup but must retain the +// uncertain image and hold further writes. Preserve is definitively invalid or +// unsupported and may be discarded only where the transaction policy permits. +// +// The filenames are the transaction state. A temp that exists while the primary +// name is empty says the commit had already passed the backup rename, and that +// is the only record of it — so an uncertain temp is answered with +// UseBackupHeld, which renames nothing. Publishing the backup instead would +// make the candidate look like an ordinary stale artifact to the next boot, +// which would delete it exactly when it finally became readable. When neither +// file can run this boot, RunDefaultsHeld keeps both names untouched for the +// same reason. namespace MQTTPrefsRecovery { enum class FileState : uint8_t { Missing, Usable, + FutureUsable, + FutureClaimed, + Indeterminate, Preserve, }; enum class Action : uint8_t { None, KeepPrimary, + DiscardTemp, PromoteTemp, PromoteBackup, + // Run the backup where it lies, changing nothing on disk, and hold writes. + // Every later boot re-runs this policy against the same three names until one + // of them can classify the candidate. + UseBackupHeld, + // Neither the candidate nor the backup can run this boot. Change nothing, + // come up on defaults, and hold writes until a boot that can classify them. + RunDefaultsHeld, }; +inline bool uncertain(FileState state) { + return state == FileState::FutureClaimed || state == FileState::Indeterminate; +} + inline Action select(FileState primary, FileState temp, FileState backup) { // A primary of any kind owns the name. In particular, do not roll a newer // or corrupt primary back to an older backup just because it cannot be read @@ -30,14 +56,36 @@ inline Action select(FileState primary, FileState temp, FileState backup) { if (primary != FileState::Missing) return Action::KeepPrimary; // A completed temp is the new image and wins over the old backup. - if (temp == FileState::Usable) return Action::PromoteTemp; + if (temp == FileState::Usable || temp == FileState::FutureUsable) { + return Action::PromoteTemp; + } - // If temp is opaque but a known-good backup exists, boot from the backup. - // The caller may discard the opaque temp once that usable backup has become - // primary. Otherwise, rename the opaque temp into the empty primary name so - // the normal loader can hold it. + // Preserve is definitively invalid, so it was never a committed image. If + // no backup exists, discard the interrupted temp and leave the primary name + // absent; this is what lets a first JSON migration retry from /mqtt_prefs. + // If a backup exists, it is the prior committed image and wins regardless + // of whether this firmware understands its schema. if (temp == FileState::Preserve) { - return backup == FileState::Usable ? Action::PromoteBackup : Action::PromoteTemp; + return backup == FileState::Missing ? Action::DiscardTemp : Action::PromoteBackup; + } + + // FutureClaimed and Indeterminate may be a completed image this firmware + // cannot classify. A known-good backup can run this boot, but it must run + // from its own name: promoting it would spend the empty primary name that + // marks the candidate as mid-commit. + // + // A backup this firmware cannot run is still the previous committed image, so + // the same argument applies to it: promoting the candidate would give it the + // authoritative name, and the "any primary owns the name" rule above would + // then keep it even on a boot that proves it corrupt. Spend the transaction + // state recorded in the filenames only once the backup is definitively no + // longer a usable fallback. + if (uncertain(temp)) { + if (backup == FileState::Usable) return Action::UseBackupHeld; + if (backup == FileState::FutureUsable || uncertain(backup)) { + return Action::RunDefaultsHeld; + } + return Action::PromoteTemp; } // No temp survived. The backup is the only recoverable image, even when it diff --git a/src/helpers/MQTTPrefsSerializer.h b/src/helpers/MQTTPrefsSerializer.h new file mode 100644 index 0000000000..4d182c442a --- /dev/null +++ b/src/helpers/MQTTPrefsSerializer.h @@ -0,0 +1,421 @@ +#pragma once + +#include +#include +#include +#include + +#ifdef WITH_MQTT_BRIDGE + +static const int32_t MQTT_PREFS_JSON_FORMAT_VERSION = 1; + +// Format invariant, binding on every future schema version: `version` is the +// FIRST property of the root object. The probe below reads it with this +// firmware's grammar and stops at the first construct that grammar cannot +// tokenize, so a newer file that introduces unknown syntax ahead of its version +// field is indistinguishable from a corrupt one here. That costs it the +// opaque-future preservation guarantee: as a temp left by an interrupted commit +// it would be classified as discardable rather than retained. Keep the version +// first when adding fields or bumping the version; a host test pins it. +// +// Reads only the mandatory root version while ignoring every other schema +// field. Recovery uses this before the v1 decoder so a syntactically valid +// future file remains opaque even when that schema changes a v1 field's type. +class MQTTPrefsVersionProbe : public ConfigSerializer { + int32_t _version = 0; + bool _seen_version = false; +protected: + void structure() override { defStrict("version", _version, _seen_version); } +public: + bool hasFutureVersion() const { + return _seen_version && _version > MQTT_PREFS_JSON_FORMAT_VERSION; + } +}; + +// ConfigSerializer adapter for the observer preference POD. Keeping the +// serializer separate avoids making the runtime object layout part of the JSON +// format and adds no permanent per-slot serializer state to CommonCLI. +class MQTTPrefsSerializer : public ConfigSerializer { + + class WifiPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _power_save; + bool _seen_ssid = false, _seen_password = false, _seen_power_save = false; + protected: + void structure() override { + defStrict("ssid", _prefs->wifi_ssid, sizeof(_prefs->wifi_ssid), _seen_ssid); + defStrict("password", _prefs->wifi_password, sizeof(_prefs->wifi_password), _seen_password); + defStrict("power_save", _power_save, _seen_power_save); + } + public: + explicit WifiPrefs(MQTTPrefs* prefs) : _prefs(prefs), _power_save(prefs->wifi_power_save) {} + bool apply(bool* repaired) { + if (_power_save < 0 || _power_save > 2) { + _power_save = 1; + *repaired = true; + } + _prefs->wifi_power_save = static_cast(_power_save); + return true; + } + }; + + class TimePrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _utc_offset, _default_utc_offset; + char _default_ntp_server[sizeof(MQTTPrefs::mqtt_ntp_server)]; + bool _seen_timezone = false, _seen_utc_offset = false, _seen_ntp_server = false; + protected: + void structure() override { + defStrict("timezone", _prefs->timezone_string, sizeof(_prefs->timezone_string), _seen_timezone); + defStrict("utc_offset", _utc_offset, _seen_utc_offset); + defStrict("ntp_server", _prefs->mqtt_ntp_server, sizeof(_prefs->mqtt_ntp_server), _seen_ntp_server); + } + public: + TimePrefs(MQTTPrefs* prefs, const MQTTPrefs* defaults) + : _prefs(prefs), _utc_offset(prefs->timezone_offset), + _default_utc_offset(defaults->timezone_offset) { + if (_default_utc_offset < -12 || _default_utc_offset > 14) { + _default_utc_offset = 0; + } + memcpy(_default_ntp_server, defaults->mqtt_ntp_server, sizeof(_default_ntp_server)); + if (_default_ntp_server[0] != '\0' && + !mqttNtpHostnameValid(_default_ntp_server)) { + _default_ntp_server[0] = '\0'; + } + } + bool apply(bool* repaired) { + if (_utc_offset < -12 || _utc_offset > 14) { + _utc_offset = _default_utc_offset; + *repaired = true; + } + if (_prefs->mqtt_ntp_server[0] != '\0' && + !mqttNtpHostnameValid(_prefs->mqtt_ntp_server)) { + memcpy(_prefs->mqtt_ntp_server, _default_ntp_server, sizeof(_default_ntp_server)); + *repaired = true; + } + _prefs->timezone_offset = static_cast(_utc_offset); + return true; + } + }; + + class StatusPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _interval_ms; + bool _seen_enabled = false, _seen_interval = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("interval_ms", _interval_ms, _seen_interval); + } + public: + explicit StatusPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->mqtt_status_enabled), + _interval_ms(static_cast(prefs->mqtt_status_interval)) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 1; *repaired = true; } + if (_interval_ms < 60000 || _interval_ms > 3600000) { + _interval_ms = 300000; + *repaired = true; + } + _prefs->mqtt_status_enabled = static_cast(_enabled); + _prefs->mqtt_status_interval = static_cast(_interval_ms); + } + }; + + class NeighborPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _interval_ms; + bool _seen_enabled = false, _seen_interval = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("interval_ms", _interval_ms, _seen_interval); + } + public: + explicit NeighborPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->mqtt_neighbors_enabled), + _interval_ms(static_cast(prefs->mqtt_neighbors_interval)) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + if (_interval_ms < static_cast(MQTT_NEIGHBORS_MIN_INTERVAL_MS) || + _interval_ms > static_cast(MQTT_NEIGHBORS_MAX_INTERVAL_MS)) { + _interval_ms = static_cast(MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS); + *repaired = true; + } + _prefs->mqtt_neighbors_enabled = static_cast(_enabled); + _prefs->mqtt_neighbors_interval = static_cast(_interval_ms); + } + }; + + class OwnerPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + bool _seen_public_key = false, _seen_email = false; + protected: + void structure() override { + defStrict("public_key", _prefs->mqtt_owner_public_key, + sizeof(_prefs->mqtt_owner_public_key), _seen_public_key); + defStrict("email", _prefs->mqtt_email, sizeof(_prefs->mqtt_email), _seen_email); + } + public: + explicit OwnerPrefs(MQTTPrefs* prefs) : _prefs(prefs) {} + void apply(bool* repaired) { + if (_prefs->mqtt_owner_public_key[0] != '\0' && + !mqttOwnerKeyValid(_prefs->mqtt_owner_public_key)) { + _prefs->mqtt_owner_public_key[0] = '\0'; + *repaired = true; + } + } + }; + + class SlotPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int _index; + int32_t _port, _filter; + bool _seen_preset = false, _seen_host = false, _seen_port = false; + bool _seen_username = false, _seen_password = false, _seen_token = false; + bool _seen_topic = false, _seen_audience = false, _seen_filter = false; + protected: + void structure() override { + defStrict("preset", _prefs->mqtt_slot_preset[_index], + sizeof(_prefs->mqtt_slot_preset[_index]), _seen_preset); + defStrict("host", _prefs->mqtt_slot_host[_index], + sizeof(_prefs->mqtt_slot_host[_index]), _seen_host); + defStrict("port", _port, _seen_port); + defStrict("username", _prefs->mqtt_slot_username[_index], + sizeof(_prefs->mqtt_slot_username[_index]), _seen_username); + defStrict("password", _prefs->mqtt_slot_password[_index], + sizeof(_prefs->mqtt_slot_password[_index]), _seen_password); + defStrict("token", _prefs->mqtt_slot_token[_index], + sizeof(_prefs->mqtt_slot_token[_index]), _seen_token); + defStrict("topic", _prefs->mqtt_slot_topic[_index], + sizeof(_prefs->mqtt_slot_topic[_index]), _seen_topic); + defStrict("audience", _prefs->mqtt_slot_audience[_index], + sizeof(_prefs->mqtt_slot_audience[_index]), _seen_audience); + defStrict("packet_filter", _filter, _seen_filter); + } + public: + SlotPrefs(MQTTPrefs* prefs, int index) + : _prefs(prefs), _index(index), _port(prefs->mqtt_slot_port[index]), + _filter(prefs->mqtt_slot_packet_filter[index]) {} + void apply(bool* repaired) { + if (_port < 0 || _port > 65535) { _port = 0; *repaired = true; } + if (_filter < 0 || _filter > 65535) { _filter = 0xffff; *repaired = true; } + const char* preset = _prefs->mqtt_slot_preset[_index]; + if (strcmp(preset, MQTT_PRESET_NONE) != 0 && + strcmp(preset, MQTT_PRESET_CUSTOM) != 0 && findMQTTPreset(preset) == nullptr) { + strcpy(_prefs->mqtt_slot_preset[_index], MQTT_PRESET_NONE); + *repaired = true; + } + _prefs->mqtt_slot_port[_index] = static_cast(_port); + _prefs->mqtt_slot_packet_filter[_index] = static_cast(_filter); + } + }; + + class MqttPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _packets_enabled, _raw_enabled, _tx_enabled, _rx_enabled; + char _default_iata[sizeof(MQTTPrefs::mqtt_iata)]; + bool _seen_origin = false, _seen_iata = false, _seen_packets = false; + bool _seen_raw = false, _seen_tx = false, _seen_rx = false; + StatusPrefs _status; + NeighborPrefs _neighbors; + OwnerPrefs _owner; + static_assert(MQTT_PREFS_SLOT_COUNT == 6, + "MQTTPrefsSerializer slot members and keys must be updated"); + SlotPrefs _slot1, _slot2, _slot3, _slot4, _slot5, _slot6; + protected: + void structure() override { + defStrict("origin", _prefs->mqtt_origin, sizeof(_prefs->mqtt_origin), _seen_origin); + defStrict("iata", _prefs->mqtt_iata, sizeof(_prefs->mqtt_iata), _seen_iata); + defStrict("packets_enabled", _packets_enabled, _seen_packets); + defStrict("raw_enabled", _raw_enabled, _seen_raw); + defStrict("tx_enabled", _tx_enabled, _seen_tx); + defStrict("rx_enabled", _rx_enabled, _seen_rx); + def("status", _status); + def("neighbors", _neighbors); + def("owner", _owner); + def("slot1", _slot1); + def("slot2", _slot2); + def("slot3", _slot3); + def("slot4", _slot4); + def("slot5", _slot5); + def("slot6", _slot6); + } + public: + MqttPrefs(MQTTPrefs* prefs, const MQTTPrefs* defaults) + : _prefs(prefs), _packets_enabled(prefs->mqtt_packets_enabled), + _raw_enabled(prefs->mqtt_raw_enabled), _tx_enabled(prefs->mqtt_tx_enabled), + _rx_enabled(prefs->mqtt_rx_enabled), _status(prefs), _neighbors(prefs), + _owner(prefs), _slot1(prefs, 0), _slot2(prefs, 1), + _slot3(prefs, 2), _slot4(prefs, 3), + _slot5(prefs, 4), _slot6(prefs, 5) { + memcpy(_default_iata, defaults->mqtt_iata, sizeof(_default_iata)); + if (_default_iata[0] != '\0' && !mqttIataValid(_default_iata)) { + _default_iata[0] = '\0'; + } + for (char* p = _default_iata; *p; ++p) { + if (*p >= 'a' && *p <= 'z') *p = static_cast(*p - ('a' - 'A')); + } + } + void apply(bool* repaired) { + if (_packets_enabled < 0 || _packets_enabled > 1) { _packets_enabled = 1; *repaired = true; } + if (_raw_enabled < 0 || _raw_enabled > 1) { _raw_enabled = 0; *repaired = true; } + if (_tx_enabled < 0 || _tx_enabled > 2) { _tx_enabled = 2; *repaired = true; } + if (_rx_enabled < 0 || _rx_enabled > 1) { _rx_enabled = 1; *repaired = true; } + _prefs->mqtt_packets_enabled = static_cast(_packets_enabled); + _prefs->mqtt_raw_enabled = static_cast(_raw_enabled); + _prefs->mqtt_tx_enabled = static_cast(_tx_enabled); + _prefs->mqtt_rx_enabled = static_cast(_rx_enabled); + if (_prefs->mqtt_iata[0] != '\0') { + if (!mqttIataValid(_prefs->mqtt_iata)) { + memcpy(_prefs->mqtt_iata, _default_iata, sizeof(_default_iata)); + *repaired = true; + } else { + for (char* p = _prefs->mqtt_iata; *p; ++p) { + if (*p >= 'a' && *p <= 'z') { + *p = static_cast(*p - ('a' - 'A')); + *repaired = true; + } + } + } + } + _status.apply(repaired); + _neighbors.apply(repaired); + _owner.apply(repaired); + _slot1.apply(repaired); _slot2.apply(repaired); _slot3.apply(repaired); + _slot4.apply(repaired); _slot5.apply(repaired); _slot6.apply(repaired); + } + }; + + class SnmpPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled; + bool _seen_enabled = false, _seen_community = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("community", _prefs->snmp_community, + sizeof(_prefs->snmp_community), _seen_community); + } + public: + explicit SnmpPrefs(MQTTPrefs* prefs) : _prefs(prefs), _enabled(prefs->snmp_enabled) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + _prefs->snmp_enabled = static_cast(_enabled); + } + }; + + class RadioPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _watchdog_min; + bool _seen_watchdog = false; + protected: + void structure() override { defStrict("watchdog_min", _watchdog_min, _seen_watchdog); } + public: + explicit RadioPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _watchdog_min(prefs->radio_watchdog_minutes) {} + void apply(bool* repaired) { + if (_watchdog_min < 0 || _watchdog_min > 120) { _watchdog_min = 5; *repaired = true; } + _prefs->radio_watchdog_minutes = static_cast(_watchdog_min); + } + }; + + class AlertPrefs : public ConfigSerializer { + MQTTPrefs* _prefs; + int32_t _enabled, _wifi_minutes, _mqtt_minutes, _rate_limit_min; + bool _seen_enabled = false, _seen_psk = false, _seen_wifi = false; + bool _seen_mqtt = false, _seen_rate = false, _seen_hashtag = false, _seen_region = false; + protected: + void structure() override { + defStrict("enabled", _enabled, _seen_enabled); + defStrict("psk_hex", _prefs->alert_psk_hex, sizeof(_prefs->alert_psk_hex), _seen_psk); + defStrict("wifi_minutes", _wifi_minutes, _seen_wifi); + defStrict("mqtt_minutes", _mqtt_minutes, _seen_mqtt); + defStrict("rate_limit_min", _rate_limit_min, _seen_rate); + defStrict("hashtag", _prefs->alert_hashtag, sizeof(_prefs->alert_hashtag), _seen_hashtag); + defStrict("region", _prefs->alert_region, sizeof(_prefs->alert_region), _seen_region); + } + public: + explicit AlertPrefs(MQTTPrefs* prefs) + : _prefs(prefs), _enabled(prefs->alert_enabled), + _wifi_minutes(prefs->alert_wifi_minutes), _mqtt_minutes(prefs->alert_mqtt_minutes), + _rate_limit_min(prefs->alert_min_interval_min) {} + void apply(bool* repaired) { + if (_enabled < 0 || _enabled > 1) { _enabled = 0; *repaired = true; } + if (_wifi_minutes < 0 || _wifi_minutes > 1440) { _wifi_minutes = 30; *repaired = true; } + if (_mqtt_minutes < 0 || _mqtt_minutes > 10080) { _mqtt_minutes = 240; *repaired = true; } + if (_rate_limit_min < 60 || _rate_limit_min > 10080) { _rate_limit_min = 60; *repaired = true; } + if (_prefs->alert_psk_hex[0] != '\0') { + const size_t len = strlen(_prefs->alert_psk_hex); + bool valid_hex = len == 32; + for (size_t i = 0; valid_hex && i < len; ++i) { + const char c = _prefs->alert_psk_hex[i]; + valid_hex = (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || + (c >= 'a' && c <= 'f'); + } + if (!valid_hex) { + _prefs->alert_psk_hex[0] = '\0'; + _prefs->alert_hashtag[0] = '\0'; + *repaired = true; + } + } + _prefs->alert_enabled = static_cast(_enabled); + _prefs->alert_wifi_minutes = static_cast(_wifi_minutes); + _prefs->alert_mqtt_minutes = static_cast(_mqtt_minutes); + _prefs->alert_min_interval_min = static_cast(_rate_limit_min); + } + }; + + MQTTPrefs* _prefs; + int32_t _version = MQTT_PREFS_JSON_FORMAT_VERSION; + bool _seen_version = false; + WifiPrefs _wifi; + TimePrefs _time; + MqttPrefs _mqtt; + SnmpPrefs _snmp; + RadioPrefs _radio; + AlertPrefs _alert; + +protected: + void structure() override { + defStrict("version", _version, _seen_version); // must stay first; see above + def("wifi", _wifi); + def("time", _time); + def("mqtt", _mqtt); + def("snmp", _snmp); + def("radio", _radio); + def("alert", _alert); + } + +public: + explicit MQTTPrefsSerializer(MQTTPrefs* prefs, const MQTTPrefs* repair_defaults = nullptr) + : _prefs(prefs), _wifi(prefs), + _time(prefs, repair_defaults ? repair_defaults : prefs), + _mqtt(prefs, repair_defaults ? repair_defaults : prefs), _snmp(prefs), + _radio(prefs), _alert(prefs) {} + + bool hasSupportedVersion() const { + return _seen_version && _version == MQTT_PREFS_JSON_FORMAT_VERSION; + } + bool hasFutureVersion() const { + return _seen_version && _version > MQTT_PREFS_JSON_FORMAT_VERSION; + } + + bool normalize(bool* repaired) { + if (repaired == nullptr) return false; + *repaired = false; + _wifi.apply(repaired); + _time.apply(repaired); + _mqtt.apply(repaired); + _snmp.apply(repaired); + _radio.apply(repaired); + _alert.apply(repaired); + return true; + } + + bool apply(bool* repaired) { + return hasSupportedVersion() && normalize(repaired); + } +}; + +#endif // WITH_MQTT_BRIDGE diff --git a/src/helpers/MQTTPrefsStorage.h b/src/helpers/MQTTPrefsStorage.h index e3c6a098be..0bc0867ee2 100644 --- a/src/helpers/MQTTPrefsStorage.h +++ b/src/helpers/MQTTPrefsStorage.h @@ -62,10 +62,9 @@ struct PreWifiPowerOldMQTTPrefs { char mqtt_email[64]; }; -// MQTT preferences stored separately from NodePrefs to avoid upstream layout -// conflicts. The full layout is the frozen v1 payload baseline. The prefix -// before observer settings is also an explicitly supported v1 payload: it was -// used before the observer fields were appended. +// Runtime MQTT preferences, stored separately from NodePrefs to avoid upstream +// layout conflicts. JSON persistence is field-based; this type is deliberately +// free to evolve independently of the frozen binary migration layouts below. struct MQTTPrefs { char mqtt_origin[32]; char mqtt_iata[8]; @@ -110,17 +109,57 @@ struct MQTTPrefs { char alert_region[31]; // Neighbors publishing (PSRAM boards only). Appended at the end of the - // observer tail so a shorter (pre-neighbors) /mqtt_prefs payload from earlier - // firmware still loads with these defaulting off/24h; keeps the format at - // VERSION 1. Field order and sizes are kept byte-identical to the flex - // neighbors build so a /mqtt_prefs written by either firmware is - // interchangeable (see the offsetof static_asserts below). + // observer tail in the former binary format. Binary compatibility is now + // represented only by LegacyV1MQTTPrefs below. uint8_t mqtt_neighbors_enabled; uint32_t mqtt_neighbors_interval; // Per-slot payload-type allow masks. Bit N controls MeshCore packet type N - // for both packets and raw MQTT topics. Appended so older v1 payloads load - // with the default all-types masks intact. + // for both packets and raw MQTT topics. + uint16_t mqtt_slot_packet_filter[MQTT_PREFS_SLOT_COUNT]; +}; + +// Frozen payload written by the version-1 binary format. Keep this distinct +// from the runtime MQTTPrefs type: JSON persistence must not make the runtime +// object's padding or member order an on-flash ABI again. Legacy decoding reads +// into this type and field-copies into a defaulted runtime object. +struct LegacyV1MQTTPrefs { + char mqtt_origin[32]; + char mqtt_iata[8]; + uint8_t mqtt_status_enabled; + uint8_t mqtt_packets_enabled; + uint8_t mqtt_raw_enabled; + uint8_t mqtt_tx_enabled; + uint32_t mqtt_status_interval; + char wifi_ssid[32]; + char wifi_password[64]; + uint8_t wifi_power_save; + char timezone_string[32]; + int8_t timezone_offset; + char mqtt_slot_preset[MQTT_PREFS_SLOT_COUNT][24]; + char mqtt_slot_host[MQTT_PREFS_SLOT_COUNT][64]; + uint16_t mqtt_slot_port[MQTT_PREFS_SLOT_COUNT]; + char mqtt_slot_username[MQTT_PREFS_SLOT_COUNT][32]; + char mqtt_slot_password[MQTT_PREFS_SLOT_COUNT][64]; + char mqtt_owner_public_key[65]; + char mqtt_email[64]; + char mqtt_slot_token[MQTT_PREFS_SLOT_COUNT][48]; + char mqtt_slot_topic[MQTT_PREFS_SLOT_COUNT][96]; + char mqtt_slot_audience[MQTT_PREFS_SLOT_COUNT][64]; + uint8_t mqtt_rx_enabled; + char mqtt_ntp_server[64]; + uint8_t snmp_enabled; + char snmp_community[24]; + uint8_t radio_watchdog_minutes; + uint8_t alert_enabled; + char alert_psk_hex[33]; + uint16_t alert_wifi_minutes; + uint16_t alert_mqtt_minutes; + uint16_t alert_min_interval_min; + char alert_hashtag[24]; + char alert_region[31]; + uint8_t mqtt_neighbors_enabled; + uint32_t mqtt_neighbors_interval; uint16_t mqtt_slot_packet_filter[MQTT_PREFS_SLOT_COUNT]; }; @@ -272,20 +311,20 @@ static const size_t LEGACY6_AUDIENCE_RX_SIZE = 2840; // Frozen on-flash layouts; every firmware and native fixture build checks them. static_assert(sizeof(MQTTPrefsHeader) == 8, "versioned /mqtt_prefs header must stay 8 bytes"); -static_assert(offsetof(MQTTPrefs, snmp_enabled) == MQTT_PREFS_V1_PRE_OBSERVER_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, snmp_enabled) == MQTT_PREFS_V1_PRE_OBSERVER_PAYLOAD_SIZE, "v1 pre-observer /mqtt_prefs boundary changed"); -static_assert(sizeof(MQTTPrefs) == MQTT_PREFS_V1_FULL_PAYLOAD_SIZE, +static_assert(sizeof(LegacyV1MQTTPrefs) == MQTT_PREFS_V1_FULL_PAYLOAD_SIZE, "v1 /mqtt_prefs payload layout changed"); // Lock the neighbors tail to the flex neighbors build's layout so a /mqtt_prefs // written by either firmware is byte-for-byte interchangeable. The enable flag // lands in the old struct's zeroed trailing padding (offset 2857), and the // interval begins exactly at the pre-neighbors payload size (2860) so a // pre-neighbors read stops right before it and the interval keeps its default. -static_assert(offsetof(MQTTPrefs, mqtt_neighbors_enabled) == 2857, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_neighbors_enabled) == 2857, "neighbors enable flag must sit at the flex-compatible offset"); -static_assert(offsetof(MQTTPrefs, mqtt_neighbors_interval) == MQTT_PREFS_V1_PRE_NEIGHBORS_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_neighbors_interval) == MQTT_PREFS_V1_PRE_NEIGHBORS_PAYLOAD_SIZE, "neighbors interval offset must equal the pre-neighbors payload size"); -static_assert(offsetof(MQTTPrefs, mqtt_slot_packet_filter) == MQTT_PREFS_V1_PRE_FILTER_PAYLOAD_SIZE, +static_assert(offsetof(LegacyV1MQTTPrefs, mqtt_slot_packet_filter) == MQTT_PREFS_V1_PRE_FILTER_PAYLOAD_SIZE, "packet filters must begin at the pre-filter payload boundary"); static_assert(sizeof(OldMQTTPrefs) == 472, "frozen pre-slot /mqtt_prefs layout changed"); static_assert(sizeof(PreWifiPowerOldMQTTPrefs) == 472, "frozen pre-WiFi-power /mqtt_prefs layout changed"); diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index bda752a6fe..0ad539e936 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -529,7 +529,7 @@ class MQTTBridge : public BridgeBase { LifecycleOps _lifecycle_ops; MQTTLifecycle::Coordinator _lifecycle; - // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt_prefs. + // Observer config (MQTT/WiFi/timezone/SNMP/alert), persisted to /mqtt.json. // _prefs (held by BridgeBase) still provides upstream fields (freq/sf/node_name…). MQTTPrefs* _obs = nullptr; diff --git a/test/README.md b/test/README.md index 711e648406..801825105d 100644 --- a/test/README.md +++ b/test/README.md @@ -33,7 +33,8 @@ does not reflect the GoogleTest count — run the built binary directly | `test_mqtt_packet_filter` | `src/helpers/MQTTPacketFilter.h` | per-slot 0-15 allowlist parsing/formatting, numeric and named spellings; exact bounds; membership; candidate/eligible split and retry-completion policy; pre-queue union gate; default-mask detection | | `test_mqtt_runtime_buffer_lifecycle` | `src/helpers/MQTTRuntimeBufferLifecycle.h` | idempotent allocation/release; partial-allocation degradation; retry of only missing buffers | | `test_mqtt_prefs_codec` | `src/helpers/MQTTPrefsStorage.h`, `src/helpers/MQTTPrefsCodec.h` | binary pre-slot/3-slot/6-slot migration fixtures; v1 header integrity; downgrade preservation; shortest-payload write policy (default filters stay downgrade-readable) | -| `test_mqtt_prefs_atomic_store` | `src/helpers/MQTTPrefsAtomicStore.h` | transactional MQTT writes and legacy `/node_prefs` handoff; exact short-write detection; begin/finish/rename failure cleanup; original-file preservation | +| `test_mqtt_prefs_serializer` | `src/helpers/MQTTPrefsSerializer.h`, `src/helpers/ConfigSerializer.*` | semantic nested `/mqtt.json` round trips; numeric slot keys; required/future version handling; strict length/overflow/duplicate rejection; safe semantic repair; scratch-before-live loading | +| `test_mqtt_prefs_atomic_store` | `src/helpers/MQTTPrefsAtomicStore.h`, `src/helpers/MQTTPrefsRecovery.h` | production JSON begin/write/checksum-finish/schema-verify/commit orchestration; first-migration and rename-boundary recovery; legacy `/node_prefs` handoff; failure cleanup and original-file preservation | | `test_mqtt_payload_builder` | `src/helpers/MQTTPayloadBuilder.cpp` | status/packet/raw JSON contracts; optional fields; escaping; RX metrics and path; score handling; exact buffer bounds; maximum representative payloads | | `test_utils` | `src/Utils.cpp` | `Utils::toHex` (upstream) | diff --git a/test/test_config_serializer/test_config_serializer.cpp b/test/test_config_serializer/test_config_serializer.cpp index 27c3c8119e..ae63f68248 100644 --- a/test/test_config_serializer/test_config_serializer.cpp +++ b/test/test_config_serializer/test_config_serializer.cpp @@ -214,6 +214,118 @@ TEST(NodePrefs, FemGainSettingsRoundTrip) { EXPECT_EQ(1, loaded.radio_fem_txgain); } +TEST(ConfigSerializer, LoadSerial_KeyDigitsAfterFirstCharacter) { + MockInputStream s("{age:1,flags:2,name:\"ok\",slot1:7}"); + TestStruct data; + data.age = data.flags = 0; + strcpy(data.name, "before"); + EXPECT_TRUE(data.loadSerial(s)); + EXPECT_EQ(1, data.age); + EXPECT_EQ(2, data.flags); + EXPECT_STREQ("ok", data.name); +} + +TEST(ConfigSerializer, LoadSerial_RejectsLeadingDigitKey) { + MockInputStream s("{1slot:7}"); + TestStruct data; + EXPECT_FALSE(data.loadSerial(s)); +} + +// ── /prefs.json compatibility under the strict shape checks ───────────────── +// +// The scalar-vs-object rejection added for /mqtt.json also runs for the plain +// def() overloads that NodePrefs uses, and loadPrefsInt() applies /prefs.json +// straight onto the live object without consulting the return value. These +// pin what that combination does to files already on deployed devices. + +// A settings file in the exact shape the firmware writes, transcribed rather +// than produced by saveSerial() so a change to the writer cannot quietly move +// the fixture with it. +static const char* DEPLOYED_PREFS_JSON = + "{name:\"Repeater-1\",pass:\"hunter2\",guest:\"\",owner:\"ops@example.com\"," + "adv_int:4,f_adv_int:12,lat:47.612345,lon:-122.334567,disc_mod:1719791234," + "radio:{freq:910.5250,bw:250.0000,sf:10,cr:5,cad:0,int_thr:0,rxgain:1," + "fem_rxgain:0,fem_txgain:1,tx:22,af:1.0000,rxdelay:1000.0000," + "f_txdelay:0.5000,d_txdelay:0.2000,agc_int:0,hash_mode:0,multi_ack:0}," + "bridge:{en:0,delay:500,src:1,baud:115200,ch:1,secret:\"\"}," + "gps:{en:0,int:60,adv_loc:0}," + "repeat:{disable:0,f_max:64,f_max_uns:32,f_max_adv:16,loop:1}," + "room:{rd_only:0},power:{adc_mult:1.0000,pwr_sav_en:0}}"; + +TEST(NodePrefs, DeployedPrefsJsonStillLoadsCompletely) { + MockInputStream s(DEPLOYED_PREFS_JSON); + NodePrefs prefs; + + ASSERT_TRUE(prefs.loadSerial(s)); + EXPECT_STREQ("Repeater-1", prefs.node_name); + EXPECT_STREQ("ops@example.com", prefs.owner_info); + EXPECT_EQ(4, prefs.advert_interval); + EXPECT_DOUBLE_EQ(47.612345, prefs.node_lat); + EXPECT_DOUBLE_EQ(-122.334567, prefs.node_lon); + EXPECT_EQ(1719791234u, prefs.discovery_mod_timestamp); + EXPECT_FLOAT_EQ(910.525f, prefs.freq); + EXPECT_EQ(10, prefs.sf); + EXPECT_EQ(22, prefs.tx_power_dbm); + EXPECT_EQ(1, prefs.radio_fem_txgain); + EXPECT_EQ(1, prefs.bridge_pkt_src); + EXPECT_EQ(115200u, prefs.bridge_baud); + EXPECT_EQ(60u, prefs.gps_interval); + EXPECT_EQ(64, prefs.flood_max); + EXPECT_EQ(1, prefs.loop_detect); +} + +TEST(NodePrefs, UnknownNestedObjectCannotBeMistakenForARootScalar) { + // Depth is what keeps an unknown group's inner keys from matching a root + // field of the same name, so an unrecognized section must stay ignorable. + MockInputStream s("{name:\"Repeater-1\",mqtt:{name:\"other\",owner:\"nobody\"},adv_int:4}"); + NodePrefs prefs; + strcpy(prefs.owner_info, "ops@example.com"); + + EXPECT_TRUE(prefs.loadSerial(s)); + EXPECT_STREQ("Repeater-1", prefs.node_name); + EXPECT_STREQ("ops@example.com", prefs.owner_info); + EXPECT_EQ(4, prefs.advert_interval); +} + +TEST(NodePrefs, TornPrefsJsonAppliesOnlyTheFieldsBeforeTheTear) { + // A power cut during the old non-transactional /prefs.json write leaves a + // file like this. loadPrefsInt() ignores the failed return and keeps the + // partial result, which is the pre-existing behavior; the strict checks + // must not turn it into something worse than a partial load. + MockInputStream s("{name:\"Repeater-1\",adv_int:4,radio:{freq:910.5250,sf:1"); + NodePrefs prefs; + prefs.sf = 9; + + EXPECT_FALSE(prefs.loadSerial(s)); + EXPECT_STREQ("Repeater-1", prefs.node_name); + EXPECT_EQ(4, prefs.advert_interval); + EXPECT_FLOAT_EQ(910.525f, prefs.freq); + EXPECT_EQ(9, prefs.sf); // the torn value never completed a token +} + +TEST(NodePrefs, ShapeMismatchIsRejectedAndStopsFurtherApplication) { + // Hand-edited or corrupted files are the regression surface for the strict + // checks: a known scalar holding an object now fails the load and stops + // parsing, so nothing after the mismatch reaches the live object. + MockInputStream scalar_as_object("{name:{x:1},adv_int:4}"); + NodePrefs prefs; + prefs.advert_interval = 7; + EXPECT_FALSE(prefs.loadSerial(scalar_as_object)); + EXPECT_EQ(7, prefs.advert_interval); + + // The reverse mismatch is rejected too: a known group given a scalar. + MockInputStream object_as_scalar("{name:\"Repeater-1\",radio:1}"); + NodePrefs scalar_group; + EXPECT_FALSE(scalar_group.loadSerial(object_as_scalar)); + + // And a known scalar nested one level deeper than the schema places it. + MockInputStream over_nested("{radio:{sf:{value:10}}}"); + NodePrefs nested; + nested.sf = 9; + EXPECT_FALSE(nested.loadSerial(over_nested)); + EXPECT_EQ(9, nested.sf); +} + // ── main ─────────────────────────────────────────────────────── diff --git a/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp b/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp index 67880a8244..1979483965 100644 --- a/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp +++ b/test/test_mqtt_prefs_atomic_store/test_mqtt_prefs_atomic_store.cpp @@ -20,6 +20,7 @@ enum class FailurePoint { PayloadWrite, ImageWrite, Finish, + Verify, Commit, }; @@ -44,7 +45,8 @@ class InMemoryStore { ++write_calls; if (!_open) return 0; const bool should_fail = (write_calls == 1 && _failure == FailurePoint::HeaderWrite) || - (write_calls == 2 && _failure == FailurePoint::PayloadWrite); + (write_calls == 2 && _failure == FailurePoint::PayloadWrite) || + _failure == FailurePoint::ImageWrite; const size_t written = should_fail && size > 0 ? size - 1 : size; _staging.insert(_staging.end(), bytes, bytes + written); return written; @@ -113,6 +115,186 @@ AtomicStore::Result runWithObserverTail(InMemoryStore* store) { return AtomicStore::write(*store, header, sizeof(header), payload, sizeof(payload)); } +// Models MQTTPrefsJsonFileStore under the SPIFFS rule it was written for: +// rename() refuses an existing destination, so publishing must move the old +// primary to .bak before the verified temp can take its name. Both halves of +// that publish, and the rollback that undoes a half-done one, are injectable. +class InMemoryJsonStore { +public: + struct Options { + FailurePoint failure = FailurePoint::None; + bool rollback_rename_fails = false; + bool rollback_remove_fails = false; + bool has_primary = true; + bool discard_remove_fails = false; + bool abort_remove_fails = false; + }; + + explicit InMemoryJsonStore(FailurePoint failure) : _opts{failure, false, false, true} { + _files["/mqtt.json"] = oldImage(); + } + explicit InMemoryJsonStore(Options opts) : _opts(opts) { + if (_opts.has_primary) _files["/mqtt.json"] = oldImage(); + } + + bool begin() { + ++begin_calls; + // Production refuses to open a new transaction while recovery still owns + // an artifact, so a rollback that left one behind blocks the retry. + if (has("/mqtt.json.tmp") || has("/mqtt.json.bak")) return false; + _staging.clear(); + _open = _opts.failure != FailurePoint::Begin; + return _open; + } + + size_t write(const uint8_t* bytes, size_t size) { + ++write_calls; + if (!_open) return 0; + const size_t written = + _opts.failure == FailurePoint::ImageWrite && size > 0 ? size - 1 : size; + _staging.insert(_staging.end(), bytes, bytes + written); + return written; + } + + bool finish() { + ++finish_calls; + _open = false; + // Production writes straight to /mqtt.json.tmp, so the bytes are on flash + // before finish() reads them back. A finish() failure can therefore be a + // failed read-back of an otherwise complete image, not a torn one. + _files["/mqtt.json.tmp"] = _staging; + if (_opts.failure == FailurePoint::Finish) return false; + _finished = true; + return true; + } + + bool verify() { + ++verify_calls; + return _opts.failure != FailurePoint::Verify; + } + + bool commit() { + ++commit_calls; + if (!_finished) return false; + if (has("/mqtt.json.bak")) return false; + if (has("/mqtt.json") && !rename("/mqtt.json", "/mqtt.json.bak")) return false; + // Fail the publish rename, the boundary that leaves the old primary parked + // in .bak with the verified new image still sitting in the temp. + if (_opts.failure == FailurePoint::Commit) return false; + if (!rename("/mqtt.json.tmp", "/mqtt.json")) return false; + _files.erase("/mqtt.json.bak"); + _finished = false; + return true; + } + + bool rollbackFailedCommit() { + ++rollback_calls; + if (!has("/mqtt.json") && has("/mqtt.json.bak")) { + if (_opts.rollback_rename_fails || !rename("/mqtt.json.bak", "/mqtt.json")) { + return false; + } + } + if (has("/mqtt.json.tmp")) { + if (_opts.rollback_remove_fails) return has("/mqtt.json"); + _files.erase("/mqtt.json.tmp"); + } + return true; + } + + // Both cleanups report the same disposition as production: false means a temp + // survived with no primary to outrank it, so boot recovery may still promote + // the image the caller is about to report as not saved. + bool discardFinishedTemp() { + ++discard_calls; + const bool removed = !_opts.discard_remove_fails; + if (removed) _files.erase("/mqtt.json.tmp"); + _finished = false; + return removed || has("/mqtt.json"); + } + + bool abort() { + ++abort_calls; + _open = false; + _staging.clear(); + bool removed = true; + if (!_finished && has("/mqtt.json.tmp")) { + removed = !_opts.abort_remove_fails; + if (removed) _files.erase("/mqtt.json.tmp"); + } + _finished = false; + return removed || has("/mqtt.json"); + } + + // Apply the boot-recovery policy to whatever the transaction left behind and + // report the image the node would come up on. Every file present in these + // scenarios is a complete image, so each maps to Usable. + std::vector imageAfterReboot() { + const auto state = [this](const char* path) { + return has(path) ? Recovery::FileState::Usable : Recovery::FileState::Missing; + }; + switch (Recovery::select(state("/mqtt.json"), state("/mqtt.json.tmp"), + state("/mqtt.json.bak"))) { + case Recovery::Action::PromoteTemp: + rename("/mqtt.json.tmp", "/mqtt.json"); + break; + case Recovery::Action::PromoteBackup: + rename("/mqtt.json.bak", "/mqtt.json"); + break; + case Recovery::Action::DiscardTemp: + _files.erase("/mqtt.json.tmp"); + break; + default: + break; + } + return has("/mqtt.json") ? _files.at("/mqtt.json") : std::vector(); + } + + bool has(const char* path) const { return _files.count(path) != 0; } + bool canStartSave() const { return !has("/mqtt.json.tmp") && !has("/mqtt.json.bak"); } + const std::vector& source() const { return _files.at("/mqtt.json"); } + static std::vector oldImage() { + const char* text = "{version:1,wifi:{ssid:\"old\"}}"; + return std::vector(text, text + strlen(text)); + } + static std::vector newImage() { + const char* text = "{version:1,wifi:{ssid:\"mesh\"}}"; + return std::vector(text, text + strlen(text)); + } + + int begin_calls = 0; + int write_calls = 0; + int finish_calls = 0; + int commit_calls = 0; + int abort_calls = 0; + int verify_calls = 0; + int discard_calls = 0; + int rollback_calls = 0; + +private: + bool rename(const char* from, const char* to) { + if (_files.count(from) == 0 || _files.count(to) != 0) return false; + _files[to] = _files[from]; + _files.erase(from); + return true; + } + + Options _opts; + bool _open = false; + bool _finished = false; + std::vector _staging; + std::map> _files; +}; + +AtomicStore::VerifiedImageResult runVerifiedJson(InMemoryJsonStore* store) { + const std::vector json = InMemoryJsonStore::newImage(); + return AtomicStore::writeVerifiedImage( + *store, + [store, &json]() { + return store->write(json.data(), json.size()) == json.size(); + }, + [store]() { return store->verify(); }); +} + class LegacyComPrefs { public: LegacyComPrefs() : bytes({'l', 'e', 'g', 'a', 'c', 'y', '-', 'c', 'o', 'm'}) {} @@ -258,7 +440,8 @@ class SpiffsMqttTransaction { } // Inject ordinary operation failures (as distinct from a power cut). A - // failed temp rename leaves both the verified temp and old backup intact. + // failed temp rename leaves both the verified temp and old backup intact, + // which is the state rollbackFailedPublish() then undoes. bool publish(bool fail_backup_rename, bool fail_temp_rename, bool fail_cleanup) { writeVerifiedTemp(); if (fail_backup_rename || !rename("/mqtt_prefs", "/mqtt_prefs.bak")) return false; @@ -267,6 +450,15 @@ class SpiffsMqttTransaction { return true; // backup cleanup is intentionally non-fatal after publish } + bool rollbackFailedPublish() { + if (!has("/mqtt_prefs") && has("/mqtt_prefs.bak") && + !rename("/mqtt_prefs.bak", "/mqtt_prefs")) { + return false; + } + _files.erase("/mqtt_prefs.tmp"); + return true; + } + void recover(Recovery::FileState primary = Recovery::FileState::Usable, Recovery::FileState temp = Recovery::FileState::Usable, Recovery::FileState backup = Recovery::FileState::Usable) { @@ -286,9 +478,18 @@ class SpiffsMqttTransaction { } return; } + if (action == Recovery::Action::DiscardTemp) { + _files.erase("/mqtt_prefs.tmp"); + return; + } + if (action == Recovery::Action::UseBackupHeld || + action == Recovery::Action::RunDefaultsHeld) { + return; // production renames nothing, so the transaction state survives + } if (action == Recovery::Action::PromoteBackup) { rename("/mqtt_prefs.bak", "/mqtt_prefs"); - if (backup == Recovery::FileState::Usable && temp != Recovery::FileState::Missing) { + if (backup == Recovery::FileState::Usable && !Recovery::uncertain(temp) && + temp != Recovery::FileState::Missing) { _files.erase("/mqtt_prefs.tmp"); } return; @@ -298,12 +499,13 @@ class SpiffsMqttTransaction { // incomplete transaction artifact. It only preserves artifacts when the // primary itself is opaque. if (had_primary && primary == Recovery::FileState::Usable) { - _files.erase("/mqtt_prefs.tmp"); - _files.erase("/mqtt_prefs.bak"); + if (!Recovery::uncertain(temp)) _files.erase("/mqtt_prefs.tmp"); + if (!Recovery::uncertain(backup)) _files.erase("/mqtt_prefs.bak"); } } bool has(const char* path) const { return _files.count(path) != 0; } + void removePrimary() { _files.erase("/mqtt_prefs"); } bool canStartSave() const { return !has("/mqtt_prefs.tmp") && !has("/mqtt_prefs.bak"); } const std::vector& primary() const { return _files.at("/mqtt_prefs"); } static std::vector oldImage() { return {'o', 'l', 'd'}; } @@ -337,6 +539,144 @@ TEST(MQTTPrefsAtomicStore, CommitPublishesExactHeaderThenPayload) { EXPECT_EQ(0, store.abort_calls); } +TEST(MQTTPrefsAtomicStore, ProductionJsonPolicyCoversEveryVerificationBoundary) { + const struct { + FailurePoint point; + AtomicStore::VerifiedImageResult expected; + int writes; + int finishes; + int verifies; + int commits; + int aborts; + int discards; + int rollbacks; + } cases[] = { + {FailurePoint::None, AtomicStore::VerifiedImageResult::Committed, + 1, 1, 1, 1, 0, 0, 0}, + {FailurePoint::Begin, AtomicStore::VerifiedImageResult::BeginFailed, + 0, 0, 0, 0, 1, 0, 0}, + {FailurePoint::ImageWrite, AtomicStore::VerifiedImageResult::WriteFailed, + 1, 0, 0, 0, 1, 0, 0}, + {FailurePoint::Finish, AtomicStore::VerifiedImageResult::FinishFailed, + 1, 1, 0, 0, 1, 0, 0}, + {FailurePoint::Verify, AtomicStore::VerifiedImageResult::VerifyFailed, + 1, 1, 1, 0, 0, 1, 0}, + {FailurePoint::Commit, AtomicStore::VerifiedImageResult::CommitFailed, + 1, 1, 1, 1, 1, 0, 1}, + }; + + for (const auto& test_case : cases) { + InMemoryJsonStore store(test_case.point); + EXPECT_EQ(test_case.expected, runVerifiedJson(&store)); + EXPECT_EQ(test_case.writes, store.write_calls); + EXPECT_EQ(test_case.finishes, store.finish_calls); + EXPECT_EQ(test_case.verifies, store.verify_calls); + EXPECT_EQ(test_case.commits, store.commit_calls); + EXPECT_EQ(test_case.aborts, store.abort_calls); + EXPECT_EQ(test_case.discards, store.discard_calls); + EXPECT_EQ(test_case.rollbacks, store.rollback_calls); + // Every failure leaves the previous image published and no artifact behind, + // so the next save can start immediately. + if (test_case.point != FailurePoint::None) { + EXPECT_EQ(InMemoryJsonStore::oldImage(), store.source()); + EXPECT_TRUE(store.canStartSave()); + } + } +} + +TEST(MQTTPrefsAtomicStore, FailedPublishIsUndoneSoTheRefusedValueCannotReturnAtBoot) { + InMemoryJsonStore store(FailurePoint::Commit); + + // The publish moved the old primary to .bak before failing to rename the + // verified temp into place. Reporting the change as rolled back is only + // truthful because that half-done transaction is undone here. + ASSERT_EQ(AtomicStore::VerifiedImageResult::CommitFailed, runVerifiedJson(&store)); + EXPECT_EQ(InMemoryJsonStore::oldImage(), store.source()); + EXPECT_FALSE(store.has("/mqtt.json.tmp")); + EXPECT_FALSE(store.has("/mqtt.json.bak")); + EXPECT_EQ(InMemoryJsonStore::oldImage(), store.imageAfterReboot()); +} + +TEST(MQTTPrefsAtomicStore, UnrestorablePublishFailureIsReportedAsIndeterminate) { + // Rollback cannot republish the backup: the primary name stays empty and the + // verified temp still wins recovery, so the refused value does come back at + // the next boot. The caller must say so rather than claim a rollback. + InMemoryJsonStore backup_stuck(InMemoryJsonStore::Options{ + FailurePoint::Commit, /*rollback_rename_fails=*/true, false, true}); + ASSERT_EQ(AtomicStore::VerifiedImageResult::CommitIndeterminate, + runVerifiedJson(&backup_stuck)); + EXPECT_TRUE(backup_stuck.has("/mqtt.json.tmp")); + EXPECT_TRUE(backup_stuck.has("/mqtt.json.bak")); + EXPECT_FALSE(backup_stuck.canStartSave()); // retries fail until this resolves + EXPECT_EQ(InMemoryJsonStore::newImage(), backup_stuck.imageAfterReboot()); + + // Same verdict for the first-ever save, where there is no backup to restore + // and the undeletable temp is the only image the next boot can find. + InMemoryJsonStore first_save(InMemoryJsonStore::Options{ + FailurePoint::Commit, false, /*rollback_remove_fails=*/true, + /*has_primary=*/false}); + ASSERT_EQ(AtomicStore::VerifiedImageResult::CommitIndeterminate, + runVerifiedJson(&first_save)); + EXPECT_FALSE(first_save.has("/mqtt.json")); + EXPECT_EQ(InMemoryJsonStore::newImage(), first_save.imageAfterReboot()); +} + +TEST(MQTTPrefsAtomicStore, RejectedTempThatCannotBeDeletedIsReportedAsIndeterminate) { + // The temp is a complete, byte-verified, perfectly valid image: only the + // scratch allocation that reparses it failed. With no primary on a fresh + // install, a temp that cleanup cannot delete is exactly what boot recovery + // promotes, so this must not be answered as a rollback. + InMemoryJsonStore first_save(InMemoryJsonStore::Options{ + FailurePoint::Verify, false, false, /*has_primary=*/false, + /*discard_remove_fails=*/true}); + ASSERT_EQ(AtomicStore::VerifiedImageResult::CleanupIndeterminate, + runVerifiedJson(&first_save)); + EXPECT_TRUE(AtomicStore::saveOutcomeUnresolved( + AtomicStore::VerifiedImageResult::CleanupIndeterminate)); + EXPECT_TRUE(first_save.has("/mqtt.json.tmp")); + EXPECT_FALSE(first_save.canStartSave()); // retries fail until this resolves + EXPECT_EQ(InMemoryJsonStore::newImage(), first_save.imageAfterReboot()); + + // With a primary published, the same stuck temp cannot win recovery, so the + // change really is gone and the ordinary rejection verdict still holds. + InMemoryJsonStore with_primary(InMemoryJsonStore::Options{ + FailurePoint::Verify, false, false, /*has_primary=*/true, + /*discard_remove_fails=*/true}); + ASSERT_EQ(AtomicStore::VerifiedImageResult::VerifyFailed, + runVerifiedJson(&with_primary)); + EXPECT_FALSE(AtomicStore::saveOutcomeUnresolved( + AtomicStore::VerifiedImageResult::VerifyFailed)); + EXPECT_EQ(InMemoryJsonStore::oldImage(), with_primary.imageAfterReboot()); +} + +TEST(MQTTPrefsAtomicStore, UnverifiedTempThatCannotBeDeletedIsReportedAsIndeterminate) { + // finish() failing does not prove the bytes are torn — a complete image whose + // read-back failed still parses at the next boot. If abort() cannot remove it + // and no primary outranks it, the outcome is unresolved for the same reason. + InMemoryJsonStore store(InMemoryJsonStore::Options{ + FailurePoint::Finish, false, false, /*has_primary=*/false, + /*discard_remove_fails=*/false, /*abort_remove_fails=*/true}); + + ASSERT_EQ(AtomicStore::VerifiedImageResult::CleanupIndeterminate, + runVerifiedJson(&store)); + EXPECT_TRUE(store.has("/mqtt.json.tmp")); + EXPECT_FALSE(store.canStartSave()); + EXPECT_EQ(InMemoryJsonStore::newImage(), store.imageAfterReboot()); +} + +TEST(MQTTPrefsAtomicStore, FirstSavePublishFailureLeavesNoImageToPromote) { + InMemoryJsonStore store(InMemoryJsonStore::Options{ + FailurePoint::Commit, false, false, /*has_primary=*/false}); + + // No prior /mqtt.json exists, so rollback only has to discard the temp. The + // next boot re-runs migration from the legacy source instead of adopting the + // value the CLI just refused. + ASSERT_EQ(AtomicStore::VerifiedImageResult::CommitFailed, runVerifiedJson(&store)); + EXPECT_FALSE(store.has("/mqtt.json.tmp")); + EXPECT_TRUE(store.imageAfterReboot().empty()); + EXPECT_TRUE(store.canStartSave()); +} + TEST(MQTTPrefsAtomicStore, AnyFailureAbortsAndPreservesExistingSource) { const std::vector source = {'o', 'l', 'd', '-', 'p', 'r', 'e', 'f', 's'}; const struct { @@ -546,6 +886,23 @@ TEST(MQTTPrefsAtomicStore, PowerCutDuringTempWriteKeepsPrimaryAndAllowsNextSave) EXPECT_TRUE(store.canStartSave()); } +TEST(MQTTPrefsAtomicStore, TornFirstMigrationTempIsDiscardedSoLegacyCanRetry) { + SpiffsMqttTransaction store; + store.removePrimary(); + store.cutDuringTempWrite(); + + EXPECT_EQ(Recovery::Action::DiscardTemp, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::Preserve, + Recovery::FileState::Missing)); + store.recover(Recovery::FileState::Missing, + Recovery::FileState::Preserve, + Recovery::FileState::Missing); + EXPECT_FALSE(store.has("/mqtt_prefs")); + EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.canStartSave()); +} + TEST(MQTTPrefsAtomicStore, RecoveredUsablePrimaryClearsOpaqueTransactionArtifacts) { { SpiffsMqttTransaction store; @@ -584,8 +941,11 @@ TEST(MQTTPrefsAtomicStore, SpiffsRenameAndCleanupFailuresRemainRecoverable) { EXPECT_FALSE(store.has("/mqtt_prefs")); EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); EXPECT_TRUE(store.has("/mqtt_prefs.bak")); + // Unlike a power cut at this boundary, a returned failure is answered to + // the caller, so the transaction is undone before recovery ever sees it. + EXPECT_TRUE(store.rollbackFailedPublish()); store.recover(); - EXPECT_EQ(SpiffsMqttTransaction::newImage(), store.primary()); + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); EXPECT_FALSE(store.has("/mqtt_prefs.bak")); } @@ -605,8 +965,14 @@ TEST(MQTTPrefsAtomicStore, RecoveryNeverOverwritesOpaqueNewerLayout) { EXPECT_EQ(Recovery::Action::KeepPrimary, Recovery::select(Recovery::FileState::Preserve, Recovery::FileState::Usable, Recovery::FileState::Usable)); - // If there is no primary, a usable backup wins over an opaque temp. Once - // promoted, production treats the backup as authoritative and clears temp. + // A syntactically valid future temp may already have passed verification and + // reached the rename phase. It wins over the stale supported backup and is + // promoted into the authoritative name, where older firmware will hold it. + EXPECT_EQ(Recovery::Action::PromoteTemp, + Recovery::select(Recovery::FileState::Missing, Recovery::FileState::FutureUsable, + Recovery::FileState::Usable)); + // A corrupt or incomplete temp is different: the supported backup remains + // the last known committed image. EXPECT_EQ(Recovery::Action::PromoteBackup, Recovery::select(Recovery::FileState::Missing, Recovery::FileState::Preserve, Recovery::FileState::Usable)); @@ -617,6 +983,145 @@ TEST(MQTTPrefsAtomicStore, RecoveryNeverOverwritesOpaqueNewerLayout) { Recovery::FileState::Preserve)); } +TEST(MQTTPrefsAtomicStore, AmbiguousFutureOrOomTempIsNeverDeleted) { + for (const Recovery::FileState uncertain : { + Recovery::FileState::FutureClaimed, + Recovery::FileState::Indeterminate}) { + SpiffsMqttTransaction store; + store.cutAt(SpiffsMqttTransaction::Boundary::AfterBackupRename); + store.recover(Recovery::FileState::Missing, uncertain, + Recovery::FileState::Usable); + + // The supported backup runs this boot from its own name. Nothing is + // renamed, so the empty primary name still records that the candidate had + // reached the publish phase. Its presence also blocks a new transaction. + EXPECT_FALSE(store.has("/mqtt_prefs")); + EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.has("/mqtt_prefs.bak")); + EXPECT_FALSE(store.canStartSave()); + } +} + +TEST(MQTTPrefsAtomicStore, PreservedCandidateIsPromotedByTheBootThatCanReadIt) { + // The whole point of keeping an uncertain temp is that a later boot can act + // on it. Publishing the backup on the first boot would defeat that: the + // candidate would then look like a stale artifact next to a usable primary, + // and get deleted exactly when it finally became readable. + const struct { + Recovery::FileState first_boot; + Recovery::FileState second_boot; + // A promoted current-format image ends the transaction, so its backup goes + // too. A future-format one keeps the last readable image and stays held. + bool clears_backup; + } cases[] = { + // Classification scratch could not be allocated, then heap recovered. + {Recovery::FileState::Indeterminate, Recovery::FileState::Usable, true}, + // Downgraded firmware could not parse it, then the node rolled forward. + {Recovery::FileState::FutureClaimed, Recovery::FileState::Usable, true}, + // Still a newer schema on the second boot, but now classifiable. + {Recovery::FileState::FutureClaimed, Recovery::FileState::FutureUsable, false}, + }; + + for (const auto& test_case : cases) { + SpiffsMqttTransaction store; + store.cutAt(SpiffsMqttTransaction::Boundary::AfterBackupRename); + + store.recover(Recovery::FileState::Missing, test_case.first_boot, + Recovery::FileState::Usable); + ASSERT_TRUE(store.has("/mqtt_prefs.tmp")); + ASSERT_FALSE(store.has("/mqtt_prefs")); + + store.recover(Recovery::FileState::Missing, test_case.second_boot, + Recovery::FileState::Usable); + EXPECT_EQ(SpiffsMqttTransaction::newImage(), store.primary()); + EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); + EXPECT_EQ(test_case.clears_backup, !store.has("/mqtt_prefs.bak")); + EXPECT_EQ(test_case.clears_backup, store.canStartSave()); + } +} + +TEST(MQTTPrefsAtomicStore, CandidateThatProvesInvalidYieldsToTheHeldBackup) { + SpiffsMqttTransaction store; + store.cutAt(SpiffsMqttTransaction::Boundary::AfterBackupRename); + store.recover(Recovery::FileState::Missing, Recovery::FileState::Indeterminate, + Recovery::FileState::Usable); + + // The second boot can classify it and finds it definitively corrupt, so the + // last committed image is published and the candidate is dropped. The node + // leaves the held state without ever having discarded an unread candidate. + store.recover(Recovery::FileState::Missing, Recovery::FileState::Preserve, + Recovery::FileState::Usable); + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); + EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.canStartSave()); +} + +TEST(MQTTPrefsAtomicStore, UncertainTempWithNoUsableBackupStillOwnsThePrimaryName) { + // With no image that can run this boot, the candidate is the only thing left + // to protect, so it takes the authoritative name and CommonCLI holds it. + EXPECT_EQ(Recovery::Action::PromoteTemp, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::Indeterminate, + Recovery::FileState::Missing)); + EXPECT_EQ(Recovery::Action::PromoteTemp, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::FutureClaimed, + Recovery::FileState::Preserve)); +} + +TEST(MQTTPrefsAtomicStore, UncertainTempDoesNotSpendAnOpaqueBackup) { + // Newer firmware was replacing a future-version primary when power was lost. + // This boot can prove the backup is a syntactically valid future image but + // cannot run it, and cannot classify the candidate at all. + EXPECT_EQ(Recovery::Action::RunDefaultsHeld, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::FutureClaimed, + Recovery::FileState::FutureUsable)); + EXPECT_EQ(Recovery::Action::RunDefaultsHeld, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::Indeterminate, + Recovery::FileState::Indeterminate)); + // A backup that is definitively invalid has ceased to be a fallback, so the + // candidate may take the authoritative name. + EXPECT_EQ(Recovery::Action::PromoteTemp, + Recovery::select(Recovery::FileState::Missing, + Recovery::FileState::Indeterminate, + Recovery::FileState::Preserve)); +} + +TEST(MQTTPrefsAtomicStore, HeldOpaqueBackupSurvivesACandidateThatProvesInvalid) { + SpiffsMqttTransaction store; + store.cutAt(SpiffsMqttTransaction::Boundary::AfterBackupRename); + + // Nothing is renamed, so the empty primary name still records the interrupted + // commit and the previous committed image keeps its backup name. + store.recover(Recovery::FileState::Missing, Recovery::FileState::FutureClaimed, + Recovery::FileState::FutureUsable); + EXPECT_FALSE(store.has("/mqtt_prefs")); + EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.has("/mqtt_prefs.bak")); + EXPECT_FALSE(store.canStartSave()); + + // A later boot understands both and finds the candidate corrupt. Because the + // first boot did not spend the primary name on it, the last committed image + // is still there to publish instead of being stranded behind a bad primary. + store.recover(Recovery::FileState::Missing, Recovery::FileState::Preserve, + Recovery::FileState::Usable); + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); + EXPECT_FALSE(store.has("/mqtt_prefs.tmp")); + EXPECT_TRUE(store.canStartSave()); +} + +TEST(MQTTPrefsAtomicStore, UsablePrimaryDoesNotCleanIndeterminateArtifact) { + SpiffsMqttTransaction store; + store.cutDuringTempWrite(); + store.recover(Recovery::FileState::Usable, + Recovery::FileState::Indeterminate); + EXPECT_EQ(SpiffsMqttTransaction::oldImage(), store.primary()); + EXPECT_TRUE(store.has("/mqtt_prefs.tmp")); + EXPECT_FALSE(store.canStartSave()); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp b/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp index 7bf49746df..73ec5ec84c 100644 --- a/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp +++ b/test/test_mqtt_prefs_codec/test_mqtt_prefs_codec.cpp @@ -28,6 +28,7 @@ MQTTPrefs defaults() { prefs.alert_wifi_minutes = 30; prefs.alert_mqtt_minutes = 240; prefs.alert_min_interval_min = 60; + prefs.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; return prefs; } @@ -59,6 +60,21 @@ Codec::DecodePlan classify(const std::vector& bytes) { return Codec::classify(bytes.data(), prefix_size, bytes.size()); } +void writeV1Payload(std::vector* bytes, const MQTTPrefs& prefs, size_t payload_len) { + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(prefs, &frozen); + ASSERT_GE(bytes->size(), sizeof(MQTTPrefsHeader) + payload_len); + memcpy(bytes->data() + sizeof(MQTTPrefsHeader), &frozen, payload_len); +} + +MQTTPrefs decodeV1(const std::vector& bytes, const Codec::DecodePlan& plan) { + LegacyV1MQTTPrefs frozen = {}; + memcpy(&frozen, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = defaults(); + Codec::migrateV1(frozen, plan.payload_len, &loaded); + return loaded; +} + void fillHighEntropy(std::vector* bytes) { uint32_t state = 0x89abcdef; for (size_t i = 0; i < bytes->size(); ++i) { @@ -244,8 +260,7 @@ TEST(MQTTPrefsCodec, CurrentVersionedPayloadRoundTripsExactly) { ASSERT_EQ(Codec::Source::Current, plan.source); ASSERT_FALSE(plan.preserve_file); ASSERT_TRUE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -277,8 +292,7 @@ TEST(MQTTPrefsCodec, DefaultFiltersKeepTheDowngradeReadablePayloadLength) { ASSERT_EQ(Codec::kV1PreFilterPayloadSize, plan.payload_len); ASSERT_FALSE(plan.preserve_file); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -299,8 +313,7 @@ TEST(MQTTPrefsCodec, AnyNonDefaultFilterOptsIntoTheLongerPayload) { const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::kV1BaselinePayloadSize, plan.payload_len); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_EQ(mask, loaded.mqtt_slot_packet_filter[slot]) << slot; EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); } @@ -338,7 +351,7 @@ TEST(MQTTPrefsCodec, PreFilterV1PayloadDefaultsEverySlotToAllTypes) { std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreFilterPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreFilterPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreFilterPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreFilterPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -346,8 +359,7 @@ TEST(MQTTPrefsCodec, PreFilterV1PayloadDefaultsEverySlotToAllTypes) { ASSERT_TRUE(plan.observer_fields_present); ASSERT_FALSE(plan.preserve_file); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("pre-filter-node", loaded.mqtt_origin); EXPECT_EQ(1u, loaded.mqtt_neighbors_enabled); EXPECT_EQ(MQTT_NEIGHBORS_MAX_INTERVAL_MS, loaded.mqtt_neighbors_interval); @@ -368,7 +380,7 @@ TEST(MQTTPrefsCodec, CompatibleShortV1PayloadPreservesDefaultsBeyondObserverBoun std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreObserverPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreObserverPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreObserverPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreObserverPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -376,8 +388,7 @@ TEST(MQTTPrefsCodec, CompatibleShortV1PayloadPreservesDefaultsBeyondObserverBoun ASSERT_FALSE(plan.preserve_file); ASSERT_FALSE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("short-v1-node", loaded.mqtt_origin); EXPECT_STREQ("ntp.short.example", loaded.mqtt_ntp_server); EXPECT_EQ(0, loaded.snmp_enabled); @@ -400,7 +411,7 @@ TEST(MQTTPrefsCodec, PreNeighborsV1PayloadLoadsObserverFieldsAndDefaultsNeighbor std::vector bytes(sizeof(MQTTPrefsHeader) + Codec::kV1PreNeighborsPayloadSize, 0); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(Codec::kV1PreNeighborsPayloadSize)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, Codec::kV1PreNeighborsPayloadSize); + writeV1Payload(&bytes, source, Codec::kV1PreNeighborsPayloadSize); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -408,10 +419,7 @@ TEST(MQTTPrefsCodec, PreNeighborsV1PayloadLoadsObserverFieldsAndDefaultsNeighbor ASSERT_FALSE(plan.preserve_file); ASSERT_TRUE(plan.observer_fields_present); - MQTTPrefs loaded = defaults(); - loaded.mqtt_neighbors_enabled = 1; // pretend stale - loaded.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; // caller's defaulted tail - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + MQTTPrefs loaded = decodeV1(bytes, plan); EXPECT_STREQ("pre-neighbors-node", loaded.mqtt_origin); EXPECT_STREQ("PNW", loaded.alert_region); @@ -475,6 +483,34 @@ TEST(MQTTPrefsCodec, CorruptOrShortVersionedInputsArePreserved) { EXPECT_TRUE(plan.preserve_file); } +TEST(MQTTPrefsCodec, HeaderValidV1StillRequiresBoundedPayloadStrings) { + MQTTPrefs source = defaults(); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + ASSERT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); + + memset(frozen.mqtt_slot_topic[5], 'x', sizeof(frozen.mqtt_slot_topic[5])); + EXPECT_FALSE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); +} + +TEST(MQTTPrefsCodec, HeaderValidV1AllowsNumericValuesForSerializerRepair) { + MQTTPrefs source = defaults(); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + + // A valid v1 header fixes the layout. Numeric bytes cannot trigger an + // out-of-bounds read and are normalized after migration, so plausibility + // must not reject deployed files merely because these values need repair. + frozen.timezone_offset = 99; + frozen.mqtt_status_enabled = 0xff; + frozen.mqtt_rx_enabled = 0xff; + frozen.snmp_enabled = 0xff; + frozen.alert_enabled = 0xff; + frozen.mqtt_neighbors_enabled = 0xa5; + EXPECT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1BaselinePayloadSize)); + EXPECT_TRUE(Codec::isPlausibleV1(frozen, Codec::kV1PreNeighborsPayloadSize)); +} + TEST(MQTTPrefsCodec, LegacyPlausibilityRejectsHighEntropyBytesAtEveryWhitelistedSize) { // A headerless raw struct has no checksum, so this only reduces false // migrations; it cannot prove that a plausible-looking file is authentic. @@ -531,7 +567,9 @@ TEST(MQTTPrefsCodec, LongerSameVersionPayloadLoadsTheBaselineAndIgnoresTheTail) const size_t payload_len = Codec::kV1BaselinePayloadSize + kTail; std::vector bytes(sizeof(MQTTPrefsHeader) + payload_len, 0xA5); writeHeader(&bytes, MQTT_PREFS_VERSION, static_cast(payload_len)); - memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &source, sizeof(source)); + LegacyV1MQTTPrefs frozen; + Codec::freezeV1(source, &frozen); + memcpy(bytes.data() + sizeof(MQTTPrefsHeader), &frozen, sizeof(frozen)); const Codec::DecodePlan plan = classify(bytes); ASSERT_EQ(Codec::Source::Current, plan.source); @@ -542,9 +580,11 @@ TEST(MQTTPrefsCodec, LongerSameVersionPayloadLoadsTheBaselineAndIgnoresTheTail) // Reading plan.payload_len bytes recovers this build's whole struct exactly, // and cannot run past it into the unknown tail. + LegacyV1MQTTPrefs loaded_frozen = {}; + memcpy(&loaded_frozen, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); + ASSERT_TRUE(Codec::isPlausibleV1(loaded_frozen, plan.payload_len)); MQTTPrefs loaded = defaults(); - memcpy(&loaded, bytes.data() + sizeof(MQTTPrefsHeader), plan.payload_len); - EXPECT_EQ(0, memcmp(&source, &loaded, sizeof(source))); + Codec::migrateV1(loaded_frozen, plan.payload_len, &loaded); EXPECT_STREQ("future-node", loaded.mqtt_origin); EXPECT_STREQ("field-ssid", loaded.wifi_ssid); EXPECT_STREQ("field-secret", loaded.wifi_password); diff --git a/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp new file mode 100644 index 0000000000..ce91b0c1a9 --- /dev/null +++ b/test/test_mqtt_prefs_serializer/test_mqtt_prefs_serializer.cpp @@ -0,0 +1,381 @@ +#include + +#include + +#define WITH_MQTT_BRIDGE 1 +#define PROGMEM +#include "helpers/MQTTPrefsSerializer.h" + +class InputStream : public Stream { +public: + explicit InputStream(const std::string& text) : _text(text) {} + int available() override { return static_cast(_text.size() - _pos); } + int read() override { return _pos < _text.size() ? _text[_pos++] : -1; } + int peek() override { return _pos < _text.size() ? _text[_pos] : -1; } +private: + std::string _text; + size_t _pos = 0; +}; + +class OutputStream : public Stream { +public: + size_t write(uint8_t byte) override { _text.push_back(static_cast(byte)); return 1; } + size_t print(int value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned int value, int = DEC) override { return appendNumber(value); } + size_t print(long value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned long value, int = DEC) override { return appendNumber(value); } + size_t print(long long value, int = DEC) override { return appendNumber(value); } + size_t print(unsigned long long value, int = DEC) override { return appendNumber(value); } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + const std::string& text() const { return _text; } +private: + template size_t appendNumber(T value) { + const std::string number = std::to_string(value); + _text += number; + return number.size(); + } + std::string _text; +}; + +class StickyFailingStream : public Stream { +public: + explicit StickyFailingStream(size_t limit) : _limit(limit) {} + size_t write(uint8_t) override { + if (_failed || _written >= _limit) { + _failed = true; + return 0; + } + ++_written; + return 1; + } + size_t print(int value, int = DEC) override { + const std::string number = std::to_string(value); + return Print::print(number.c_str()); + } + int available() override { return 0; } + int read() override { return -1; } + int peek() override { return -1; } + bool failed() const { return _failed; } +private: + size_t _limit; + size_t _written = 0; + bool _failed = false; +}; + +static MQTTPrefs defaults() { + MQTTPrefs prefs = {}; + prefs.mqtt_status_enabled = 1; + prefs.mqtt_packets_enabled = 1; + prefs.mqtt_tx_enabled = 2; + prefs.mqtt_rx_enabled = 1; + prefs.mqtt_status_interval = 300000; + prefs.wifi_power_save = 1; + prefs.timezone_offset = -7; + prefs.radio_watchdog_minutes = 5; + prefs.alert_wifi_minutes = 30; + prefs.alert_mqtt_minutes = 240; + prefs.alert_min_interval_min = 60; + prefs.mqtt_neighbors_interval = MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS; + strcpy(prefs.snmp_community, "public"); + for (int i = 0; i < MQTT_PREFS_SLOT_COUNT; ++i) { + strcpy(prefs.mqtt_slot_preset[i], "none"); + prefs.mqtt_slot_packet_filter[i] = 0xffff; + } + return prefs; +} + +TEST(MQTTPrefsSerializer, RoundTripsEveryGroupAndNumericSlotKeys) { + MQTTPrefs source = defaults(); + strcpy(source.wifi_ssid, "mesh-net"); + strcpy(source.wifi_password, "p\\\"ass\nword"); + strcpy(source.timezone_string, "MST7MDT,M3.2.0"); + strcpy(source.mqtt_ntp_server, "time.example"); + strcpy(source.mqtt_origin, "observer-one"); + strcpy(source.mqtt_iata, "SEA"); + source.mqtt_neighbors_enabled = 1; + source.mqtt_neighbors_interval = MQTT_NEIGHBORS_MAX_INTERVAL_MS; + strcpy(source.mqtt_owner_public_key, + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"); + strcpy(source.mqtt_email, "owner@example.com"); + strcpy(source.mqtt_slot_preset[5], "custom"); + strcpy(source.mqtt_slot_host[5], "broker.example"); + source.mqtt_slot_port[5] = 65535; + strcpy(source.mqtt_slot_username[5], "user-six"); + strcpy(source.mqtt_slot_password[5], "secret-six"); + strcpy(source.mqtt_slot_token[5], "token-six"); + strcpy(source.mqtt_slot_topic[5], "mesh/{iata}/{type}"); + strcpy(source.mqtt_slot_audience[5], "audience-six"); + source.mqtt_slot_packet_filter[5] = 0x8001; + source.snmp_enabled = 1; + source.radio_watchdog_minutes = 120; + source.alert_enabled = 1; + strcpy(source.alert_psk_hex, "0123456789abcdef0123456789abcdef"); + strcpy(source.alert_hashtag, "#ops"); + strcpy(source.alert_region, "PNW"); + + OutputStream output; + MQTTPrefsSerializer writer(&source); + ASSERT_TRUE(writer.saveSerial(output)); + EXPECT_NE(std::string::npos, output.text().find("slot6:{")) << output.text(); + EXPECT_NE(std::string::npos, output.text().find("packet_filter:32769")) << output.text(); + + MQTTPrefs loaded = defaults(); + InputStream input(output.text()); + MQTTPrefsSerializer reader(&loaded); + ASSERT_TRUE(reader.loadSerial(input)) << output.text(); + bool repaired = true; + ASSERT_TRUE(reader.apply(&repaired)); + EXPECT_FALSE(repaired); + EXPECT_STREQ(source.wifi_password, loaded.wifi_password); + EXPECT_STREQ(source.mqtt_owner_public_key, loaded.mqtt_owner_public_key); + EXPECT_STREQ(source.mqtt_slot_host[5], loaded.mqtt_slot_host[5]); + EXPECT_EQ(65535, loaded.mqtt_slot_port[5]); + EXPECT_EQ(0x8001, loaded.mqtt_slot_packet_filter[5]); + EXPECT_EQ(MQTT_NEIGHBORS_MAX_INTERVAL_MS, loaded.mqtt_neighbors_interval); + EXPECT_STREQ("PNW", loaded.alert_region); +} + +TEST(MQTTPrefsSerializer, MissingOptionalKeysKeepDefaults) { + MQTTPrefs prefs = defaults(); + InputStream input("{version:1,mqtt:{origin:\"changed\"}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = true; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_FALSE(repaired); + EXPECT_STREQ("changed", prefs.mqtt_origin); + EXPECT_EQ(1, prefs.mqtt_packets_enabled); + EXPECT_EQ(300000u, prefs.mqtt_status_interval); + EXPECT_EQ(0xffff, prefs.mqtt_slot_packet_filter[5]); +} + +TEST(MQTTPrefsSerializer, RequiresSupportedVersion) { + MQTTPrefs missing = defaults(); + InputStream no_version("{wifi:{ssid:\"x\"}}"); + MQTTPrefsSerializer missing_serializer(&missing); + ASSERT_TRUE(missing_serializer.loadSerial(no_version)); + bool repaired = false; + EXPECT_FALSE(missing_serializer.apply(&repaired)); + + MQTTPrefs future = defaults(); + InputStream future_input("{version:2,wifi:{ssid:\"x\"}}"); + MQTTPrefsSerializer future_serializer(&future); + ASSERT_TRUE(future_serializer.loadSerial(future_input)); + EXPECT_TRUE(future_serializer.hasFutureVersion()); + EXPECT_FALSE(future_serializer.apply(&repaired)); +} + +TEST(MQTTPrefsSerializer, FutureVersionProbeIgnoresV1FieldTypeChanges) { + InputStream probe_input("{version:2,wifi:{power_save:\"max\"}}"); + MQTTPrefsVersionProbe probe; + ASSERT_TRUE(probe.loadSerial(probe_input)); + EXPECT_TRUE(probe.hasFutureVersion()); + + // The same image is not valid under v1, demonstrating why recovery must + // probe the version before invoking the current schema. + MQTTPrefs prefs = defaults(); + InputStream v1_input("{version:2,wifi:{power_save:\"max\"}}"); + MQTTPrefsSerializer v1(&prefs); + EXPECT_FALSE(v1.loadSerial(v1_input)); + + for (const char* future_text : { + "{version:2,this_key_is_too_long:1}", + "{version:2,x:[1]}", + "{version:2,wifi:{ssid:\"torn\"}"}) { + InputStream future_input(future_text); + MQTTPrefsVersionProbe future_probe; + EXPECT_FALSE(future_probe.loadSerial(future_input)) << future_text; + EXPECT_TRUE(future_probe.hasFutureVersion()) << future_text; + } +} + +TEST(MQTTPrefsSerializer, VersionIsWrittenAsTheFirstRootProperty) { + MQTTPrefs prefs = defaults(); + MQTTPrefsSerializer writer(&prefs); + OutputStream output; + ASSERT_TRUE(writer.saveSerial(output)); + EXPECT_EQ(0u, output.text().find("{version:1,")) << output.text(); +} + +TEST(MQTTPrefsSerializer, FutureGrammarAheadOfVersionCannotBeRecognizedAsFuture) { + // Why the version-first invariant is part of the format rather than a style + // preference. These two files differ only in key order, and only the + // compliant one keeps its preservation guarantee on this firmware. + InputStream compliant("{version:2,x:[1]}"); + MQTTPrefsVersionProbe compliant_probe; + EXPECT_FALSE(compliant_probe.loadSerial(compliant)); + EXPECT_TRUE(compliant_probe.hasFutureVersion()); + + InputStream violating("{x:[1],version:2}"); + MQTTPrefsVersionProbe violating_probe; + EXPECT_FALSE(violating_probe.loadSerial(violating)); + EXPECT_FALSE(violating_probe.hasFutureVersion()); +} + +TEST(MQTTPrefsSerializer, RejectsDuplicateKnownKey) { + MQTTPrefs prefs = defaults(); + InputStream input("{version:1,mqtt:{origin:\"one\",origin:\"two\"}}"); + MQTTPrefsSerializer serializer(&prefs); + EXPECT_FALSE(serializer.loadSerial(input)); +} + +TEST(MQTTPrefsSerializer, RejectsOverlongStringAndIntegerOverflow) { + MQTTPrefs prefs = defaults(); + InputStream long_string( + "{version:1,wifi:{ssid:\"12345678901234567890123456789012\"}}"); + MQTTPrefsSerializer string_serializer(&prefs); + EXPECT_FALSE(string_serializer.loadSerial(long_string)); + + prefs = defaults(); + InputStream overflow("{version:1,mqtt:{slot1:{port:999999999999}}}"); + MQTTPrefsSerializer number_serializer(&prefs); + EXPECT_FALSE(number_serializer.loadSerial(overflow)); + + prefs = defaults(); + InputStream quoted_number("{version:\"1\"}"); + MQTTPrefsSerializer quoted_number_serializer(&prefs); + EXPECT_FALSE(quoted_number_serializer.loadSerial(quoted_number)); + + prefs = defaults(); + InputStream bare_string("{version:1,wifi:{ssid:meshnet}}"); + MQTTPrefsSerializer bare_string_serializer(&prefs); + EXPECT_FALSE(bare_string_serializer.loadSerial(bare_string)); +} + +TEST(MQTTPrefsSerializer, RejectsScalarObjectShapeMismatches) { + MQTTPrefs prefs = defaults(); + InputStream object_version("{version:{x:1}}"); + MQTTPrefsSerializer object_version_serializer(&prefs); + EXPECT_FALSE(object_version_serializer.loadSerial(object_version)); + + prefs = defaults(); + InputStream object_port("{version:1,mqtt:{slot1:{port:{x:1883}}}}"); + MQTTPrefsSerializer object_port_serializer(&prefs); + EXPECT_FALSE(object_port_serializer.loadSerial(object_port)); + + prefs = defaults(); + InputStream scalar_mqtt("{version:1,mqtt:1}"); + MQTTPrefsSerializer scalar_mqtt_serializer(&prefs); + EXPECT_FALSE(scalar_mqtt_serializer.loadSerial(scalar_mqtt)); + + prefs = defaults(); + InputStream scalar_slot("{version:1,mqtt:{slot1:1}}"); + MQTTPrefsSerializer scalar_slot_serializer(&prefs); + EXPECT_FALSE(scalar_slot_serializer.loadSerial(scalar_slot)); +} + +TEST(MQTTPrefsSerializer, RepairsSemanticRanges) { + MQTTPrefs prefs = defaults(); + InputStream input( + "{version:1,wifi:{power_save:9},time:{utc_offset:99}," + "mqtt:{tx_enabled:7,status:{enabled:3,interval_ms:10}," + "neighbors:{enabled:2,interval_ms:100},slot1:{port:-1,packet_filter:-2}}," + "radio:{watchdog_min:121},alert:{rate_limit_min:1}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = false; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_EQ(1, prefs.wifi_power_save); + EXPECT_EQ(-7, prefs.timezone_offset); + EXPECT_EQ(2, prefs.mqtt_tx_enabled); + EXPECT_EQ(300000u, prefs.mqtt_status_interval); + EXPECT_EQ(MQTT_NEIGHBORS_DEFAULT_INTERVAL_MS, prefs.mqtt_neighbors_interval); + EXPECT_EQ(0, prefs.mqtt_slot_port[0]); + EXPECT_EQ(0xffff, prefs.mqtt_slot_packet_filter[0]); + EXPECT_EQ(5, prefs.radio_watchdog_minutes); + EXPECT_EQ(60, prefs.alert_min_interval_min); +} + +TEST(MQTTPrefsSerializer, RepairsTextValuesToSafeDefaults) { + MQTTPrefs prefs = defaults(); + InputStream input( + "{version:1,time:{ntp_server:\"bad/host\"},mqtt:{iata:\"sea\"," + "owner:{public_key:\"not-a-key\"},slot1:{preset:\"not-a-preset\"}," + "slot2:{preset:\"analyzer-us\"},slot3:{preset:\"analyzer-us\"}}," + "alert:{psk_hex:\"not-hex\",hashtag:\"#stale\"}}"); + MQTTPrefsSerializer serializer(&prefs); + ASSERT_TRUE(serializer.loadSerial(input)); + bool repaired = false; + ASSERT_TRUE(serializer.apply(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_STREQ("SEA", prefs.mqtt_iata); + EXPECT_STREQ("", prefs.mqtt_ntp_server); + EXPECT_STREQ("", prefs.mqtt_owner_public_key); + EXPECT_STREQ("none", prefs.mqtt_slot_preset[0]); + EXPECT_STREQ("analyzer-us", prefs.mqtt_slot_preset[1]); + // Historical firmware allowed duplicate aliases. Preserve them on load; + // current setters prevent creating new duplicates without silently changing + // a deployed configuration during migration. + EXPECT_STREQ("analyzer-us", prefs.mqtt_slot_preset[2]); + EXPECT_STREQ("", prefs.alert_psk_hex); + EXPECT_STREQ("", prefs.alert_hashtag); +} + +TEST(MQTTPrefsSerializer, LateParseOrVersionFailureCannotMutateLivePrefs) { + MQTTPrefs live = defaults(); + strcpy(live.wifi_ssid, "live-network"); + + MQTTPrefs scratch = defaults(); + InputStream truncated("{wifi:{ssid:\"uncommitted\"},version:1"); + MQTTPrefsSerializer truncated_serializer(&scratch); + EXPECT_FALSE(truncated_serializer.loadSerial(truncated)); + EXPECT_STREQ("live-network", live.wifi_ssid); + + scratch = defaults(); + InputStream future("{wifi:{ssid:\"future-network\"},version:2}"); + MQTTPrefsSerializer future_serializer(&scratch); + ASSERT_TRUE(future_serializer.loadSerial(future)); + EXPECT_TRUE(future_serializer.hasFutureVersion()); + bool repaired = false; + EXPECT_FALSE(future_serializer.apply(&repaired)); + EXPECT_STREQ("live-network", live.wifi_ssid); +} + +TEST(MQTTPrefsSerializer, StickyShortWriteFailsTheCompleteSave) { + MQTTPrefs prefs = defaults(); + MQTTPrefsSerializer serializer(&prefs); + StickyFailingStream output(20); + EXPECT_FALSE(serializer.saveSerial(output)); + EXPECT_TRUE(output.failed()); +} + +TEST(MQTTPrefsSerializer, SaveNormalizationIsIdempotentAgainstKnownDefaults) { + MQTTPrefs prefs = defaults(); + prefs.timezone_offset = 99; + strcpy(prefs.mqtt_ntp_server, "bad/host"); + strcpy(prefs.mqtt_iata, "not-iata"); + strcpy(prefs.mqtt_slot_preset[0], "not-a-preset"); + + MQTTPrefs repair_defaults = defaults(); + repair_defaults.timezone_offset = -7; + strcpy(repair_defaults.mqtt_iata, "sea"); + strcpy(repair_defaults.mqtt_slot_preset[0], "analyzer-us"); + + MQTTPrefsSerializer writer(&prefs, &repair_defaults); + bool repaired = false; + ASSERT_TRUE(writer.normalize(&repaired)); + EXPECT_TRUE(repaired); + EXPECT_EQ(-7, prefs.timezone_offset); + EXPECT_STREQ("", prefs.mqtt_ntp_server); + EXPECT_STREQ("SEA", prefs.mqtt_iata); + EXPECT_STREQ("none", prefs.mqtt_slot_preset[0]); + + OutputStream output; + ASSERT_TRUE(writer.saveSerial(output)); + + MQTTPrefs loaded = defaults(); + InputStream input(output.text()); + MQTTPrefsSerializer reader(&loaded); + ASSERT_TRUE(reader.loadSerial(input)); + repaired = true; + ASSERT_TRUE(reader.apply(&repaired)); + EXPECT_FALSE(repaired) << output.text(); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/test_observer_validation/test_observer_validation.cpp b/test/test_observer_validation/test_observer_validation.cpp index 3045caed6a..8e9913ca86 100644 --- a/test/test_observer_validation/test_observer_validation.cpp +++ b/test/test_observer_validation/test_observer_validation.cpp @@ -129,6 +129,18 @@ TEST(ValueFits, RejectsNullOrZeroBuffer) { EXPECT_FALSE(mqttValueFits("abc", 0)); } +TEST(AssignedPresetSlot, FindsDuplicatesOutsideTheTargetSlot) { + const char presets[4][24] = { + "analyzer-us", "none", "analyzer-eu", "analyzer-us" + }; + + EXPECT_EQ(3, mqttAssignedPresetSlot(presets, "analyzer-us", 0)); + EXPECT_EQ(0, mqttAssignedPresetSlot(presets, "analyzer-us", 3)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, "analyzer-eu", 2)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, "missing", 0)); + EXPECT_EQ(-1, mqttAssignedPresetSlot(presets, nullptr, 0)); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_webconfig_batch/test_webconfig_batch.cpp b/test/test_webconfig_batch/test_webconfig_batch.cpp index 813412c57c..670c6ead14 100644 --- a/test/test_webconfig_batch/test_webconfig_batch.cpp +++ b/test/test_webconfig_batch/test_webconfig_batch.cpp @@ -230,6 +230,7 @@ TEST(WebConfigBatch, CliFailureRepliesAreRecognisedInEveryShapeCommonCLIEmits) { "Err - bad params", // MyMesh setperm "ERR: bad pubkey", // neighbor.remove "Error: IATA code must be exactly 3 letters",// observer setters + "Error: setting not persisted; change rolled back", // observer storage failure "(ERR: clock cannot go backwards)", // clock sync, parenthesised "Unknown command", // top-level fallthrough "unknown config: mqtt.nope", // set fallthrough