Advert parser is non compliant with protocol

[jbrazio]

The protocol defines the payload of an advert as:

Field	Size (bytes)	Description
flags	1	specifies which of the fields are present, see below
latitude	4 (optional)	decimal latitude multiplied by 1000000, integer
longitude	4 (optional)	decimal longitude multiplied by 1000000, integer
feature 1	2 (optional)	reserved for future use
feature 2	2 (optional)	reserved for future use
name	rest of appdata	name of the node

The current parser (and AFAIK the mobile app) is not checking the flags bit and it's not parsing the feat1 and feat2 fields, it considers all app data as "name" which is incorrect.

[jbrazio]

Field	Size (bytes)	Description
header	1	Contains routing type, payload type, and payload version.
transport_codes	4 (optional)	2x 16-bit transport codes (if ROUTE_TYPE_TRANSPORT_*)
path_len	1	Length of the path field in bytes.
path	up to 64 (MAX_PATH_SIZE)	Stores the routing path if applicable.
payload	up to 184 (MAX_PACKET_PAYLOAD)	The actual data being transmitted.

Total of 255 bytes for the entire packet, including header, routing information, and payload.

Field	Size (bytes)	Description
public key	32	Ed25519 public key of the node
timestamp	4	unix timestamp of advertisement
signature	64	Ed25519 signature of public key, timestamp, and app data
appdata	rest of payload	optional, see below

Total of 100 bytes, leaving 84 bytes for appdata.

Field	Size (bytes)	Description
flags	1	specifies which of the fields are present, see below
latitude	4 (optional)	decimal latitude multiplied by 1000000, integer
longitude	4 (optional)	decimal longitude multiplied by 1000000, integer
feature 1	2 (optional)	reserved for future use
feature 2	2 (optional)	reserved for future use
name	rest of appdata	name of the node

Total of 13 bytes with full options, leaving 71 bytes for name.

Looking at code, name is defined as char node_name[32].

Thus not only application is not processing all the fields but also it's cutting down the number of chars in the name, there is enough space to have 32 byte names. Currently only 23 chars are allowed.

@liamcottle

[liamcottle]

Hey mate, thanks for the report. I'll get the js lib and the mobile app patched so it parses those extra fields.

Please note that the MAX_ADVERT_DATA_SIZE is 32, so even though the name is up to 32, this unfortunately also includes all of those flags, hence the name gets cut down.

I haven't looked at the calculations to see if it's possible to increase that limit without overflowing the packet itself.

As far as I can tell, for the mobile app, the issue with the feature flags not being parsed should only affect the contact names in the RX Log adverts at this time. Since the contact list etc is handled/updated in the firmware. The app doesn't need to parse the raw adverts for anything else other than this feature.

[liamcottle]

This should do the trick. f59b0f7 I've applied the same patch to the mobile app.

[jbrazio]

Please note that the MAX_ADVERT_DATA_SIZE is 32

I missed that part, but is there a reason why it is capped at 32 ?
It would be good to have bigger names for repeaters as they might be tied to city names. Maybe not the full 71 bytes, but 32 + flags.

[liamcottle]

I missed that part, but is there a reason why it is capped at 32 ? It would be good to have bigger names for repeaters as they might be tied to city names. Maybe not the full 71 bytes, but 32 + flags.

Yeah, having the full name in advert regardless of what flags/extra data are present would be nice, but doesn't look like it will work without updating firmware on both sender and receiver

Not sure why it was capped to 32, but if it's changed, things like the advert blob store BlobRec would need to be purged/reinitialised, as on nrf52 it uses a static sized buffer per entry and all blobs are stored in a master file. Whereas on esp32 each blob is stored in its own file.

Also it looks like this code clamps the app data length for received adverts as well, which means older firmware would drop newer adverts as the signatures would no longer match, since it's computed on the recipient side from the clamped data.

https://github.com/meshcore-dev/MeshCore/blob/e812632235274ffd2382adf5354168aec765d416/src/Mesh.cpp#L258

If this was to ever get fixed, and was a breaking change, it probably needs to have a byte flag in the packet that says how long the advert data is, so the recipient doesn't have to use a hardcoded value.