mirror of
https://github.com/meshtastic/firmware.git
synced 2026-06-16 04:30:18 -04:00
Fix long name clamping and adjust related structures for compatibility
This commit is contained in:
Submodule protobufs updated: 7916a0ce81...485ede7422
@@ -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) {
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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" */
|
||||
|
||||
@@ -206,4 +206,10 @@ bool sanitizeUtf8(char *buf, size_t bufSize)
|
||||
}
|
||||
|
||||
return replaced;
|
||||
}
|
||||
}
|
||||
|
||||
void clampLongName(char *longName)
|
||||
{
|
||||
longName[MAX_LONG_NAME_BYTES] = '\0';
|
||||
sanitizeUtf8(longName, MAX_LONG_NAME_BYTES + 1);
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
{
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -58,7 +58,7 @@ class AdminModule : public ProtobufModule<meshtastic_AdminMessage>, public Obser
|
||||
/**
|
||||
* Setters
|
||||
*/
|
||||
void handleSetOwner(const meshtastic_User &o);
|
||||
void handleSetOwner(meshtastic_User &o);
|
||||
void handleSetChannel(const meshtastic_Channel &cc);
|
||||
|
||||
protected:
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 <cstdio>
|
||||
#include <cstring>
|
||||
#include <unity.h>
|
||||
@@ -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);
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user