search

lathoub / Arduino-AppleMIDI-Library

Send and receive MIDI messages over Ethernet (rtpMIDI or AppleMIDI)
Other
306 stars 66 forks source link

large session names will silently fail #128

Closed folkertvanheusden closed 3 years ago

folkertvanheusden commented 3 years ago

When the initiator uses a large session-name (more than 48 bytes, including 0x00) causing the applesession-data to be more than 64 bytes (see static const size_t MaxBufferSize = 64; in AppleMIDI/src/AppleMIDI_Settings.h) then the library will not respond to the request, not even a reject will be send.

See this zip-file for network traces (included are text-dumps):

network-traces.zip

lathoub commented 3 years ago

Thanks for reporting @folkertvanheusden and some initial checking

I have initiated a fix in this branch using MSVS (I don't have a device at hand here). Can you do a quick check?

Beware: the long session name will be truncated to DefaultSettings::MaxSessionNameLen, but processing should continue (ignoring the remainder of the packet)

folkertvanheusden commented 3 years ago

Yes, that fixes the problem. Thank you very much for the fast response!

lathoub commented 3 years ago

@folkertvanheusden Can you also check with shorter session names?

folkertvanheusden commented 3 years ago

Hi,

With short (e.g. "aseqdump (129:0)") names it crashes.

lathoub commented 3 years ago

tested with short and long names on an ESP32 @folkertvanheusden pls check again

lathoub commented 3 years ago

Some additional context, if you read this later: the parser is designed to have a minimal memory footprint: small code and memory footprint. The parser buffer is only 64 bytes. Even for longer MIDI messages, 64 bytes is enough - the packet is sliced in 64 bytes and processed in pieces.

For the initial AppleMIDI handshake, the session name can be longer than the 64 bytes buffer, the parser will truncate the session name (DefaultSettings::MaxSessionNameLen) and discard the remainder of the packet.

https://github.com/lathoub/Arduino-AppleMIDI-Library/blob/be0be95c14db975897e9e48ae88f4b59669727ce/src/AppleMIDI_Parser.h#L79-L102

https://github.com/lathoub/Arduino-AppleMIDI-Library/blob/be0be95c14db975897e9e48ae88f4b59669727ce/src/AppleMIDI.hpp#L41-L45

folkertvanheusden commented 3 years ago

Thanks for the explanation.

I've tested be0be95c14db975897e9e48ae88f4b59669727ce and it works fine now with both long and short names.