Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

large session names will silently fail #128

Closed
folkertvanheusden opened this issue Aug 20, 2021 · 7 comments
Closed

large session names will silently fail #128

folkertvanheusden opened this issue Aug 20, 2021 · 7 comments
Labels

Comments

@folkertvanheusden
Copy link

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
Copy link
Owner

lathoub commented Aug 20, 2021

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
Copy link
Author

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

@lathoub
Copy link
Owner

lathoub commented Aug 20, 2021

@folkertvanheusden
Can you also check with shorter session names?

@folkertvanheusden
Copy link
Author

folkertvanheusden commented Aug 20, 2021 via email

@lathoub
Copy link
Owner

lathoub commented Aug 20, 2021

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

@lathoub
Copy link
Owner

lathoub commented Aug 20, 2021

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.

#ifdef KEEP_SESSION_NAME
uint16_t bi = 0;
while (i < buffer.size())
{
if (bi < DefaultSettings::MaxSessionNameLen)
invitation.sessionName[bi++] = buffer[i++];
else
i++;
}
invitation.sessionName[bi++] = '\0';
#else
while (i < buffer.size())
i++;
#endif
auto retVal = parserReturn::Processed;
// when given a Session Name and the buffer has been fully processed and the
// last character is not 'endl', then we got a very long sessionName. It will
// continue in the next memory chunk of the packet. We don't care, so indicated
// flush the remainder of the packet.
// First part if the session name is kept, processing continues
if (i > minimumLen)
if (i == buffer.size() && buffer[i] != 0x00)
retVal = parserReturn::SessionNameVeryLong;

else if (retVal == parserReturn::SessionNameVeryLong)
{
// purge the rest of the data in controlPort
while (controlPort.read() >= 0) {}
}

@folkertvanheusden
Copy link
Author

Thanks for the explanation.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants