diff --git a/protobufs b/protobufs index 7916a0ce8..485ede742 160000 --- a/protobufs +++ b/protobufs @@ -1 +1 @@ -Subproject commit 7916a0ce81ec4639a361a72c9c4d2e9b8aad63f2 +Subproject commit 485ede7422e6db2c819e42913167ab15d9f557b7 diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index b047a14a6..ccc9a00ef 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -365,6 +365,15 @@ extern void getMacAddr(uint8_t *dmac); * we use !macaddr (no colons). */ meshtastic_User &owner = devicestate.owner; + +// The slim NodeInfoLite header defines the local long_name cap; the wire-facing +// meshtastic_User stays wider so names from senders built against the older +// 39-byte limit still decode (nanopb halts on string overflow). +static_assert(MAX_LONG_NAME_BYTES + 1 == sizeof(meshtastic_NodeInfoLite::long_name), + "MAX_LONG_NAME_BYTES must match the NodeInfoLite storage width"); +static_assert(sizeof(meshtastic_User::long_name) > MAX_LONG_NAME_BYTES, + "wire User.long_name must be wider than the local cap so clampLongName stays in bounds"); + meshtastic_Position localPosition = meshtastic_Position_init_default; meshtastic_CriticalErrorCode error_code = meshtastic_CriticalErrorCode_NONE; // For the error code, only show values from this boot (discard value from flash) @@ -1514,6 +1523,7 @@ void NodeDB::installDefaultDeviceState() #else snprintf(owner.long_name, sizeof(owner.long_name), "Meshtastic %04x", getNodeNum() & 0x0ffff); #endif + clampLongName(owner.long_name); // vendor userprefs may exceed the local cap #ifdef USERPREFS_CONFIG_OWNER_SHORT_NAME snprintf(owner.short_name, sizeof(owner.short_name), (const char *)USERPREFS_CONFIG_OWNER_SHORT_NAME); #else @@ -1727,8 +1737,9 @@ void NodeDB::loadFromDisk() if (nodeInfoLiteHasUser(us)) { LOG_WARN("Restoring owner fields (long_name/short_name/is_licensed/is_unmessagable) from NodeDB for our node 0x%08x", us->num); - memcpy(owner.long_name, us->long_name, sizeof(owner.long_name)); - owner.long_name[sizeof(owner.long_name) - 1] = '\0'; + // owner.long_name (40) is wider than the lite source (25); bound by the source + memcpy(owner.long_name, us->long_name, sizeof(us->long_name)); + owner.long_name[sizeof(us->long_name) - 1] = '\0'; memcpy(owner.short_name, us->short_name, sizeof(owner.short_name)); owner.short_name[sizeof(owner.short_name) - 1] = '\0'; owner.is_licensed = nodeInfoLiteIsLicensed(us); @@ -1742,6 +1753,10 @@ void NodeDB::loadFromDisk() LOG_INFO("Loaded saved devicestate version %d", devicestate.version); } + // Devicestate saved by firmware that allowed 39-byte names gets clamped on + // first load; from here on owner never carries more than the local cap. + clampLongName(owner.long_name); + state = loadProto(configFileName, meshtastic_LocalConfig_size, sizeof(meshtastic_LocalConfig), &meshtastic_LocalConfig_msg, &config); if (state != LoadFileResult::LOAD_SUCCESS) { diff --git a/src/mesh/generated/meshtastic/clientonly.pb.h b/src/mesh/generated/meshtastic/clientonly.pb.h index 5109e20b2..9375d9f46 100644 --- a/src/mesh/generated/meshtastic/clientonly.pb.h +++ b/src/mesh/generated/meshtastic/clientonly.pb.h @@ -17,7 +17,7 @@ typedef struct _meshtastic_DeviceProfile { /* Long name for the node */ bool has_long_name; - char long_name[40]; + char long_name[25]; /* Short name of the node */ bool has_short_name; char short_name[5]; diff --git a/src/mesh/generated/meshtastic/mesh.pb.h b/src/mesh/generated/meshtastic/mesh.pb.h index 7c2eef04f..4436fd16e 100644 --- a/src/mesh/generated/meshtastic/mesh.pb.h +++ b/src/mesh/generated/meshtastic/mesh.pb.h @@ -774,7 +774,10 @@ typedef struct _meshtastic_User { Note: app developers are encouraged to also use the following standard node IDs "^all" (for broadcast), "^local" (for the locally connected node) */ char id[16]; - /* A full name for this user, i.e. "Kevin Hester" */ + /* A full name for this user, i.e. "Kevin Hester" + Limited to 24 bytes of UTF-8: longer names are accepted from senders + built against the older 39-byte limit, but devices truncate them before + storing or rebroadcasting. Clients should enforce 24 bytes in their UI. */ char long_name[40]; /* A VERY short name, ideally two characters. Suitable for a tiny OLED screen */ diff --git a/src/mesh/generated/meshtastic/mqtt.pb.h b/src/mesh/generated/meshtastic/mqtt.pb.h index c5b10f1f5..5249c098b 100644 --- a/src/mesh/generated/meshtastic/mqtt.pb.h +++ b/src/mesh/generated/meshtastic/mqtt.pb.h @@ -27,7 +27,7 @@ typedef struct _meshtastic_ServiceEnvelope { /* Information about a node intended to be reported unencrypted to a map using MQTT. */ typedef struct _meshtastic_MapReport { /* A full name for this user, i.e. "Kevin Hester" */ - char long_name[40]; + char long_name[25]; /* A VERY short name, ideally two characters. Suitable for a tiny OLED screen */ char short_name[5]; @@ -126,7 +126,7 @@ extern const pb_msgdesc_t meshtastic_MapReport_msg; /* Maximum encoded size of messages (where known) */ /* meshtastic_ServiceEnvelope_size depends on runtime parameters */ #define MESHTASTIC_MESHTASTIC_MQTT_PB_H_MAX_SIZE meshtastic_MapReport_size -#define meshtastic_MapReport_size 110 +#define meshtastic_MapReport_size 95 #ifdef __cplusplus } /* extern "C" */ diff --git a/src/meshUtils.cpp b/src/meshUtils.cpp index f2ee20589..697446c16 100644 --- a/src/meshUtils.cpp +++ b/src/meshUtils.cpp @@ -206,4 +206,10 @@ bool sanitizeUtf8(char *buf, size_t bufSize) } return replaced; -} \ No newline at end of file +} + +void clampLongName(char *longName) +{ + longName[MAX_LONG_NAME_BYTES] = '\0'; + sanitizeUtf8(longName, MAX_LONG_NAME_BYTES + 1); +} diff --git a/src/meshUtils.h b/src/meshUtils.h index dd591f333..0ce01c522 100644 --- a/src/meshUtils.h +++ b/src/meshUtils.h @@ -60,6 +60,17 @@ size_t pb_string_length(const char *str, size_t max_len); // Ensures the result is null-terminated within bufSize. Returns true if any bytes were replaced. bool sanitizeUtf8(char *buf, size_t bufSize); +// Longest User.long_name content (bytes, excluding NUL) we store or transmit. +// The wire decode buffer stays at 40 so names from senders built against the +// older 39-byte limit still parse; everything we keep or send is clamped to +// this, matching the slim NodeInfoLite storage width in deviceonly.proto. +#define MAX_LONG_NAME_BYTES 24 + +// Clamp a long_name buffer (at least MAX_LONG_NAME_BYTES + 1 bytes) in-place +// to MAX_LONG_NAME_BYTES bytes of content, fixing any partial UTF-8 sequence +// left at the cut. +void clampLongName(char *longName); + /// Calculate 2^n without calling pow() - used for spreading factor and other calculations inline uint32_t pow_of_2(uint32_t n) { diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 85e66fe96..4f0e7c3ca 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -620,10 +620,14 @@ void AdminModule::handleGetModuleConfigResponse(const meshtastic_MeshPacket &mp, * Setter methods */ -void AdminModule::handleSetOwner(const meshtastic_User &o) +void AdminModule::handleSetOwner(meshtastic_User &o) { int changed = 0; + // Apps built against the older 39-byte limit may send longer names; clamp + // before the changed-compare so re-sending the same long name is a no-op. + clampLongName(o.long_name); + if (*o.long_name) { changed |= strcmp(owner.long_name, o.long_name); strncpy(owner.long_name, o.long_name, sizeof(owner.long_name)); diff --git a/src/modules/AdminModule.h b/src/modules/AdminModule.h index aeb41471c..81796d913 100644 --- a/src/modules/AdminModule.h +++ b/src/modules/AdminModule.h @@ -58,7 +58,7 @@ class AdminModule : public ProtobufModule, public Obser /** * Setters */ - void handleSetOwner(const meshtastic_User &o); + void handleSetOwner(meshtastic_User &o); void handleSetChannel(const meshtastic_Channel &cc); protected: diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 3884b543f..c1e8da1a4 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -726,7 +726,9 @@ void MQTT::perhapsReportToMap() // Fill MapReport message meshtastic_MapReport mapReport = meshtastic_MapReport_init_default; - memcpy(mapReport.long_name, owner.long_name, sizeof(owner.long_name)); + // owner.long_name (40) is wider than mapReport.long_name (25); bound by the destination + strncpy(mapReport.long_name, owner.long_name, sizeof(mapReport.long_name)); + mapReport.long_name[sizeof(mapReport.long_name) - 1] = '\0'; memcpy(mapReport.short_name, owner.short_name, sizeof(owner.short_name)); mapReport.role = config.device.role; mapReport.hw_model = owner.hw_model; diff --git a/test/test_type_conversions/test_main.cpp b/test/test_type_conversions/test_main.cpp index a02d9bdf8..18063d5ea 100644 --- a/test/test_type_conversions/test_main.cpp +++ b/test/test_type_conversions/test_main.cpp @@ -1,6 +1,8 @@ // Tests for src/mesh/TypeConversions.cpp covering: // - bitfield bit collapse on store + extraction round-trip -// - long_name / short_name truncation at the new max_size:25 / 5 boundaries +// - long_name / short_name truncation at the storage boundaries (wire User +// stays 40 wide for decoding legacy senders; NodeInfoLite stores 25 / 5) +// - wire-level decode acceptance of legacy 39-byte long_names // - public_key / hw_model / role pass-through // - thin vs bundled NodeInfo emission // @@ -10,6 +12,8 @@ #include "NodeDB.h" #include "TestUtil.h" #include "TypeConversions.h" +#include "mesh-pb-constants.h" +#include "meshUtils.h" #include #include #include @@ -128,6 +132,46 @@ void test_long_name_truncated_utf8_boundary_sanitized(void) TEST_ASSERT_EQUAL_INT('?', lite.long_name[23]); } +// ---------- wire decode width (decode-liberal, store-narrow) ------------------ + +// Hand-built wire-format User payload: field 2 (long_name), wire type 2. +static size_t makeUserPayload(uint8_t *buf, size_t nameLen) +{ + size_t i = 0; + buf[i++] = 0x12; // tag: field 2, length-delimited + buf[i++] = (uint8_t)nameLen; + for (size_t j = 0; j < nameLen; j++) + buf[i++] = (uint8_t)('A' + (j % 26)); + return i; +} + +void test_wire_decode_accepts_legacy_39_byte_long_name(void) +{ + // The longest name a sender built against the old max_size:40 can emit. + // nanopb halts on string overflow rather than truncating, so this only + // passes while the wire-facing meshtastic_User stays 40 wide. + uint8_t buf[64]; + size_t len = makeUserPayload(buf, 39); + meshtastic_User u = meshtastic_User_init_zero; + TEST_ASSERT_TRUE(pb_decode_from_bytes(buf, len, &meshtastic_User_msg, &u)); + TEST_ASSERT_EQUAL_INT(39, (int)strlen(u.long_name)); + + // ...and the store boundary clamps it to the local cap. + meshtastic_NodeInfoLite lite = meshtastic_NodeInfoLite_init_default; + TypeConversions::CopyUserToNodeInfoLite(&lite, u); + TEST_ASSERT_EQUAL_INT(MAX_LONG_NAME_BYTES, (int)strlen(lite.long_name)); +} + +void test_wire_decode_rejects_name_beyond_wire_limit(void) +{ + // 45 bytes exceeds even the 40-byte wire buffer; the whole message is + // rejected (documents the hard outer bound). + uint8_t buf[64]; + size_t len = makeUserPayload(buf, 45); + meshtastic_User u = meshtastic_User_init_zero; + TEST_ASSERT_FALSE(pb_decode_from_bytes(buf, len, &meshtastic_User_msg, &u)); +} + // ---------- short_name truncation -------------------------------------------- void test_short_name_passes_through(void) @@ -383,6 +427,8 @@ void setup() RUN_TEST(test_long_name_truncates_when_too_long); RUN_TEST(test_long_name_round_trip_to_wire); RUN_TEST(test_long_name_truncated_utf8_boundary_sanitized); + RUN_TEST(test_wire_decode_accepts_legacy_39_byte_long_name); + RUN_TEST(test_wire_decode_rejects_name_beyond_wire_limit); RUN_TEST(test_short_name_passes_through); RUN_TEST(test_short_name_truncates_when_too_long); RUN_TEST(test_bitfield_is_licensed_round_trip); diff --git a/test/test_utf8/test_main.cpp b/test/test_utf8/test_main.cpp index 5a074e96e..5dd65f24a 100644 --- a/test/test_utf8/test_main.cpp +++ b/test/test_utf8/test_main.cpp @@ -163,6 +163,47 @@ void test_above_max_codepoint() TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf))); } +// --- clampLongName: local 24-byte cap over wider wire buffers --- + +void test_clamp_long_name_short_unchanged() +{ + char buf[40] = "Kevin Hester"; + clampLongName(buf); + TEST_ASSERT_EQUAL_STRING("Kevin Hester", buf); +} + +void test_clamp_long_name_exact_cap_unchanged() +{ + char buf[40] = "abcdefghijklmnopqrstuvwx"; // exactly 24 bytes + clampLongName(buf); + TEST_ASSERT_EQUAL_STRING("abcdefghijklmnopqrstuvwx", buf); +} + +void test_clamp_long_name_truncates_39_bytes() +{ + char buf[40]; + memset(buf, 'a', 39); + buf[39] = '\0'; + clampLongName(buf); + TEST_ASSERT_EQUAL_INT(MAX_LONG_NAME_BYTES, (int)strlen(buf)); +} + +void test_clamp_long_name_fixes_partial_rune_at_cut() +{ + // 22 ASCII then a 4-byte emoji straddling the 24-byte boundary + char buf[40]; + memset(buf, 'a', 22); + buf[22] = '\xF0'; + buf[23] = '\x9F'; + buf[24] = '\x8C'; + buf[25] = '\x99'; + buf[26] = '\0'; + clampLongName(buf); + TEST_ASSERT_EQUAL_INT(24, (int)strlen(buf)); + TEST_ASSERT_EQUAL_INT('?', buf[22]); + TEST_ASSERT_EQUAL_INT('?', buf[23]); +} + void setup() { UNITY_BEGIN(); @@ -191,6 +232,12 @@ void setup() RUN_TEST(test_valid_max_codepoint); RUN_TEST(test_above_max_codepoint); + // clampLongName + RUN_TEST(test_clamp_long_name_short_unchanged); + RUN_TEST(test_clamp_long_name_exact_cap_unchanged); + RUN_TEST(test_clamp_long_name_truncates_39_bytes); + RUN_TEST(test_clamp_long_name_fixes_partial_rune_at_cut); + exit(UNITY_END()); }