serializeMsgPack to dinamically allocated buffer of size len falls short.

[gnalbandian]

Hi @bblanchon.

I am extensively using your library along with ESPASyncWebServer from @me-no-dev.
Once a second I send to webSocket clients a stream of ~400bytes with data to update a web page data.

I am using a DynamicJsonBuffer to store all my data.

// data is serialized above this section
// buffer is defined above this section as well as len
// uint8_t* buffer = nullptr;
// size_t len = 0;

 if(ws.count())
            {
                len = measureMsgPack(jsonDoc);
                buffer = (uint8_t*)malloc(sizeof(uint8_t) * len);
                
                if(buffer)
                {
                    serializeMsgPack(jsonDoc, buffer, len); // **<- the issue relies here**
                }
            }

After successful buffer allocation and population, I send the data and free the buffer.

if(buffer)
    {
        ws.binaryAll(buffer, len);
        free(buffer);
        buffer = nullptr;
    }

When reading the received data, I found out one value is missing. MessagePack is correctly decoded by javascript client, and it parses fine as a json. It's just the value of a key that instead of showing its current value, it shows always 0. It's data type is int32_t

After many test i found that increasing by 1 the length of the buffer size, I receive all the data as it should. ie:
serializeMsgPack(jsonDoc, buffer, len + 1); // **<- the issue relies here**

The following section has not been modified:
ws.binaryAll(buffer, len); Never added +1

Unfortunately, applying this 'fix' leads to random crash, very likely because of buffer overflow, not sure thou.
Can yo shed some light over here. Thanks @bblanchon

[bblanchon]

Hi @gnalbandian,

Thank you very much for reporting this issue 👍

Indeed, I just realized that, like serializeJson(), serializeMsgPack() enforce a null-terminator at the end of the output.
While this makes sense for JSON because the output is text, it's obviously wrong for MessagePack, where the output is binary.

I'll fix this problem soon; meanwhile, increase the size of the allocated buffer by 1 byte (to make room for the terminator) and continue passing len + 1 to serializeMsgPack().

Best Regards,
Benoit

[gnalbandian]

Hi @bblanchon. As usuall, I appreciate your answer.
Just to clarify, until a fix is realease to remove the null-terminator, what you mean is:

 if(ws.count())
  {
      len = measureMsgPack(jsonDoc);
      buffer = (uint8_t*)malloc(sizeof(uint8_t) * len + 1); // **<- SEE HERE**
      
      if(buffer)
      {
          serializeMsgPack(jsonDoc, buffer, len + 1); // **<- SEE HERE**
      }
  }

Wouldn't measureMsgPack(jsonDoc) already consider the null-terminator in its answer?

[bblanchon]

Yes, that's what I meant.
No, measureMsgPack() doesn't include the null terminator in its result (and neither does measureJson()).

[bblanchon]

Fix published in ArduinoJson 6.18.0

[gnalbandian]

Great! Thanks @bblanchon.
Regards!