mavlink_msg_statustext_get_text returns garbage when not zero terminated

[julianoes]

There seems to be a bug/regression with mavlink_msg_statustext_get_text where it reads past the end of the string.

This has likely to do with the fact that the statustext.text is not required to be zero terminated.

I wonder if this has to do with the change in #922.

@DonLakeFlyer brought this up on Discord

[julianoes]

Having had a closer look, I think that's just expected, and I don't see a way to fix the existing method.

/**
 * @brief Get field text from statustext message
 *
 * @return  Status text message, without null termination character
 */
static inline uint16_t mavlink_msg_statustext_get_text(const mavlink_message_t* msg, char *text)
{
    return _MAV_RETURN_char_array(msg, text, 50,  1);
}

The caller doesn't supply the length of text, so we can't do anything smart inside _MAV_RETURN_char_array.

I would say that is also clearly documented in the docstring that there potentially is no null termination character.

[peterbarker]

The idiom is

    char buffer[MAVLINK_MSG_STATUSTEXT_FIELD_TEXT_LEN+1] {};
    mavlink_msg_statustext_get_text(msg, buffer);

This ensures buffer is null-terminated (the {} is important)

I'm not saying this is a good interface, but it is workable.

I think we can close this one?

[DonLakeFlyer]

Nope, that's not it. QGC would certainly have a lot of problems if it didn't understand the whole null termination thing! The unit test in QGC has been working fine for like 10 years. And then all of I sudden I pull a new mavlink and it fails.

I believe the problem is that length of the overall STATUSTEXT message is clipped to be shorter if the string is shorter than the full space for max status text. The message over the wire in this case also has no null. If I remember correctly I looked at why mavlink_msg_statustext_get_text worked and switching to mavlink_msg_statustext_decode fixed the problem. The reason I believe is because mavlink_msg_statustext_decode pays attention to the message length when it is unpacking the string. Whereas mavlink_msg_statustext_get_text does not and it should.

The problem happens when the string is shorter than the full storage space and has room to store the null. Not when the text fills the entire buffer and there is no null.

Look here for how decode works taking into account message length: https://github.com/mavlink/c_library_v2/blob/master/common/mavlink_msg_statustext.h#L330

[DonLakeFlyer]

Here's the QGC code which is suddenly failing:

QString StatusTextHandler::getMessageText(const mavlink_message_t &message)
{
    QByteArray b;

    b.resize(MAVLINK_MSG_STATUSTEXT_FIELD_TEXT_LEN + 1);
    (void) mavlink_msg_statustext_get_text(&message, b.data());

    // Ensure NUL-termination
    b[b.length()-1] = '\0';

    const QString text = QString::fromLocal8Bit(b, std::strlen(b.constData()));

    return text;
}

And here is what I changed to which works fine:

QString StatusTextHandler::getMessageText(const mavlink_message_t &message)
{
    // Warning: There is a bug in mavlink which causes mavlink_msg_statustext_get_text to work incorrect.
    // It ends up copying crap off the end of the buffer, so don't use it for now.

    mavlink_statustext_t statusText;
    mavlink_msg_statustext_decode(&message, &statusText);

    char buffer[MAVLINK_MSG_STATUSTEXT_FIELD_TEXT_LEN + 1];
    memcpy(buffer, statusText.text, MAVLINK_MSG_STATUSTEXT_FIELD_TEXT_LEN);
    buffer[MAVLINK_MSG_STATUSTEXT_FIELD_TEXT_LEN] = '\0';

    return QString(buffer);
}

[peterbarker]

On Thu, 30 Oct 2025, Don Gagne wrote: The problem happens when the string is shorter than the full storage space and has room to store the null. Not when the text fills the entire buffer and there is no null.

So I was wondering yesterday whether the buffer may be shorter because of mavlink2 zero-truncation. I thought, surely it must be, that's not a rabbit hole to go down. Is that what's going on here, however? Is it simple for you to run that same test but using mavlink1 not mavlink2 (to avoid the truncation)?

Look here for how decode works taking into account message length: https://github.com/mavlink/c_library_v2/blob/master/common/mavlink_msg_statustext.h#L330

Well... that's doing a different thing, decapsulating the payload into a mavlink_statustext_t. Definitely taking into account the zero-truncation problem.

[peterbarker]

On Thu, 30 Oct 2025, Don Gagne wrote: // It ends up copying crap off the end of the buffer, so don't use it fo r now.

That's the thing.... I'm not sure it's copying off the end of the buffer, it may be copying from memory after the entire truncated message!