diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 64fed99b34..6a801be7d6 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -27,6 +27,49 @@ static bool isValidName(const char *n) { return true; } +// The binary loader has sanitised these values since before /prefs.json existed. +// Both loaders call it now, so a file that was edited by hand or written by +// another tool cannot leave the node on values its own CLI would refuse. +// Non-finite floats never reach this point: ConfigSerializer keeps the default +// in place for those, and constrain() would pass a NaN through unchanged. +void CommonCLI::sanitisePrefs() { + // sanitise bad pref values + _prefs->rx_delay_base = constrain(_prefs->rx_delay_base, 0, 20.0f); + _prefs->tx_delay_factor = constrain(_prefs->tx_delay_factor, 0, 2.0f); + _prefs->direct_tx_delay_factor = constrain(_prefs->direct_tx_delay_factor, 0, 2.0f); + // 99 is what the CLI can produce, not the 9 of "set af": "set dutycycle 1" + // stores 100/1 - 1. The old limit of 9 predates that command by a year + // (01d84d5d 2025-03-05 against 0aa0ec1f 2026-03-07) and would throw away a + // duty cycle below 10 percent at the next boot. + _prefs->airtime_factor = constrain(_prefs->airtime_factor, 0, 99.0f); + _prefs->freq = constrain(_prefs->freq, 150.0f, 2500.0f); + _prefs->bw = constrain(_prefs->bw, 7.8f, 500.0f); + _prefs->sf = constrain(_prefs->sf, 5, 12); + _prefs->cr = constrain(_prefs->cr, 5, 8); + _prefs->tx_power_dbm = constrain(_prefs->tx_power_dbm, -9, 30); + _prefs->multi_acks = constrain(_prefs->multi_acks, 0, 1); + _prefs->adc_multiplier = constrain(_prefs->adc_multiplier, 0.0f, 10.0f); + _prefs->path_hash_mode = constrain(_prefs->path_hash_mode, 0, 2); // NOTE: mode 3 reserved for future + + // sanitise bad bridge pref values + _prefs->bridge_enabled = constrain(_prefs->bridge_enabled, 0, 1); + _prefs->bridge_delay = constrain(_prefs->bridge_delay, 0, 10000); + _prefs->bridge_pkt_src = constrain(_prefs->bridge_pkt_src, 0, 1); + _prefs->bridge_baud = constrain(_prefs->bridge_baud, 9600, BRIDGE_MAX_BAUD); + _prefs->bridge_channel = constrain(_prefs->bridge_channel, 0, 14); + + _prefs->powersaving_enabled = constrain(_prefs->powersaving_enabled, 0, 1); + + _prefs->gps_enabled = constrain(_prefs->gps_enabled, 0, 1); + _prefs->advert_loc_policy = constrain(_prefs->advert_loc_policy, 0, 2); + + // sanitise settings + _prefs->rx_boosted_gain = constrain(_prefs->rx_boosted_gain, 0, 1); // boolean + _prefs->radio_fem_rxgain = constrain(_prefs->radio_fem_rxgain, 0, 1); // boolean + _prefs->radio_fem_txgain = constrain(_prefs->radio_fem_txgain, 0, 1); // boolean + _prefs->cad_enabled = constrain(_prefs->cad_enabled, 0, 1); // boolean +} + void CommonCLI::loadPrefs(FILESYSTEM* fs) { if (fs->exists("/prefs.json")) { #if defined(RP2040_PLATFORM) @@ -35,8 +78,11 @@ void CommonCLI::loadPrefs(FILESYSTEM* fs) { File file = fs->open("/prefs.json"); #endif if (file) { - _prefs->loadSerial(file); // new Serial prefs + if (!_prefs->loadSerial(file)) { + MESH_DEBUG_PRINTLN("loadPrefs: /prefs.json did not parse cleanly"); + } file.close(); + sanitisePrefs(); } } else if (fs->exists("/com_prefs")) { loadPrefsInt(fs, "/com_prefs"); @@ -104,39 +150,9 @@ void CommonCLI::loadPrefsInt(FILESYSTEM* fs, const char* filename) { // Legacy file.read((uint8_t *)&_prefs->cad_enabled, sizeof(_prefs->cad_enabled)); // 294 // next: 295 - // sanitise bad pref values - _prefs->rx_delay_base = constrain(_prefs->rx_delay_base, 0, 20.0f); - _prefs->tx_delay_factor = constrain(_prefs->tx_delay_factor, 0, 2.0f); - _prefs->direct_tx_delay_factor = constrain(_prefs->direct_tx_delay_factor, 0, 2.0f); - _prefs->airtime_factor = constrain(_prefs->airtime_factor, 0, 9.0f); - _prefs->freq = constrain(_prefs->freq, 150.0f, 2500.0f); - _prefs->bw = constrain(_prefs->bw, 7.8f, 500.0f); - _prefs->sf = constrain(_prefs->sf, 5, 12); - _prefs->cr = constrain(_prefs->cr, 5, 8); - _prefs->tx_power_dbm = constrain(_prefs->tx_power_dbm, -9, 30); - _prefs->multi_acks = constrain(_prefs->multi_acks, 0, 1); - _prefs->adc_multiplier = constrain(_prefs->adc_multiplier, 0.0f, 10.0f); - _prefs->path_hash_mode = constrain(_prefs->path_hash_mode, 0, 2); // NOTE: mode 3 reserved for future - - // sanitise bad bridge pref values - _prefs->bridge_enabled = constrain(_prefs->bridge_enabled, 0, 1); - _prefs->bridge_delay = constrain(_prefs->bridge_delay, 0, 10000); - _prefs->bridge_pkt_src = constrain(_prefs->bridge_pkt_src, 0, 1); - _prefs->bridge_baud = constrain(_prefs->bridge_baud, 9600, BRIDGE_MAX_BAUD); - _prefs->bridge_channel = constrain(_prefs->bridge_channel, 0, 14); - - _prefs->powersaving_enabled = constrain(_prefs->powersaving_enabled, 0, 1); - - _prefs->gps_enabled = constrain(_prefs->gps_enabled, 0, 1); - _prefs->advert_loc_policy = constrain(_prefs->advert_loc_policy, 0, 2); - - // sanitise settings - _prefs->rx_boosted_gain = constrain(_prefs->rx_boosted_gain, 0, 1); // boolean - _prefs->radio_fem_rxgain = constrain(_prefs->radio_fem_rxgain, 0, 1); // boolean - _prefs->radio_fem_txgain = constrain(_prefs->radio_fem_txgain, 0, 1); // boolean - _prefs->cad_enabled = constrain(_prefs->cad_enabled, 0, 1); // boolean file.close(); + sanitisePrefs(); } } diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index cb85f6f993..b8cdea2e74 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -338,6 +338,7 @@ class CommonCLI { mesh::RTCClock* getRTCClock() { return _rtc; } void savePrefs(); void loadPrefsInt(FILESYSTEM* _fs, const char* filename); + void sanitisePrefs(); void handleRegionCmd(char* command, char* reply); void handleGetCmd(uint32_t sender_timestamp, char* command, char* reply); diff --git a/src/helpers/ConfigSerializer.cpp b/src/helpers/ConfigSerializer.cpp index daf834b2b1..4e1913b469 100644 --- a/src/helpers/ConfigSerializer.cpp +++ b/src/helpers/ConfigSerializer.cpp @@ -1,5 +1,27 @@ #include "ConfigSerializer.h" #include // atoi/atol/atof (Arduino.h pulls this in on-device, native builds do not) +#include // memcpy, for the bit check below +#include + +namespace { + +// True if the value is a finite number. NaN and infinity are recognised by +// their bits, not by a comparison: nRF52 builds compile with -Ofast, whose +// -ffast-math lets float comparisons assume that NaN and infinity never +// occur, so a comparison alone may let them through. +bool isFinite(float v) { + uint32_t bits; + memcpy(&bits, &v, sizeof(bits)); + return (bits & 0x7F800000u) != 0x7F800000u; +} + +bool isFinite(double v) { + uint64_t bits; + memcpy(&bits, &v, sizeof(bits)); + return (bits & 0x7FF0000000000000ull) != 0x7FF0000000000000ull; +} + +} // namespace bool ConfigSerializer::saveSerial(Stream& s) { Context context(&s, OP::WRITE); @@ -294,7 +316,11 @@ void ConfigSerializer::def(const char* key, double& value) { } } else { if (_context->keyMatch(_depth, key)) { - value = atof(_context->getToken()); + // A token that is not a finite number is no value: leave what is already + // in the field, which is the default the firmware set up before reading + // the file. + double parsed = atof(_context->getToken()); + if (isFinite(parsed)) value = parsed; } } } @@ -311,7 +337,10 @@ void ConfigSerializer::def(const char* key, float& value) { } } else { if (_context->keyMatch(_depth, key)) { - value = (float) atof(_context->getToken()); + // Checked after the narrowing to float, since a value that is finite as + // a double can overflow to infinity as a float. + float parsed = (float) atof(_context->getToken()); + if (isFinite(parsed)) value = parsed; } } }