Runtime Error in MQTTPacket_formatPayload Due to char Type Not Defaulting to Unsigned in MSVC 193833134 (Visual Studio 2022)

[liuyuanlins]

Issue Description:

Compilation Environment: Visual Studio 2022
Compiler Version: MSVC 193833134

In the file paho.mqtt.c\src\MQTTPacket.c, the function:

int MQTTPacket_formatPayload(int buflen, char* buf, int payloadlen, char* payload)

at line 863:

if (isprint(payload[i]))

the isprint causes an error during debugging with the isprint function. This is because the compiler does not default the char type to unsigned, as shown in the attached image.

convert isclype.cpp Line: 36 Expression:c>=-1&&c<= 255

[robinstampa]

The problem here is that the behavior for isprint(ch) is undefined for anything outside ch >= -1 && ch <= 255 per C standard:

The behavior is undefined if the value of ch is not representable as unsigned char and is not equal to EOF. ¹

Therefore, the CRT debug library correctly throws an assertion error if the character is outside this range to prevent undefined behavior:

When a debug CRT library is used and c isn't one of these values, the functions raise an assertion. ²

The problem is that although int isprint( int ch ); accepts an int, one must make sure parameter ch is not outside the allowed range.

Following code locations might produce the same issue:

• if (isprint(payload[i]))
• while (isprint(c))
• else if (isxdigit(*(p1 + 1)) && isxdigit(*(p1 + 2)))³

Footnotes

1. https://en.cppreference.com/w/c/string/byte/isprint ↩

2. See last paragraph of 'Return value' ↩

3. Same situation: undefined behavior ↩

[icraggs]

It does feel a little odd to accept an int but only be valid for an unsigned char!

I presume a cast to unsigned char would work in this case ...

[sergeibobyr]

Is this a new issue? (I see the code that calls isprint() was added 2 months ago...) I was happily using vcpkg paho-mqttpp3 dependency for a while now and just now decided to update vcpkg baseline to the latest. That pulled everything latest, and now I get this error when trying to run my client. Is the workaround going back to the older version? If yes, can you recommend which one?

I'm using protobuf for the payload (the publish method accepts a byte array) and the 'c' it fails on for me has a value -102 but the exact number is obviously irrelevant.

This happens on the async send thread:
Not Flagged > 26128 0 Worker Thread paho-mqtt3as.dll!_MQTTAsync_sendThread@4() ucrtbased.dll!_chvalidator

The error:

Debug Assertion Failed!
Program: xxxxx.exe
File: minkernel\crts\ucrt\src\appcrt\convert\isctype.cpp
Line: 36
Expression: c >= -1 && c <= 255

The call stack:

ucrtbased.dll!_chvalidator(int c, int mask) Line 36 C++
ucrtbased.dll!fast_check_initial_locale(const int c, const int mask) Line 22 C++
ucrtbased.dll!fast_check_current_locale(const int c, const int mask) Line 33 C++
ucrtbased.dll!isprint(int c) Line 198 C++
paho-mqtt3as.dll!MQTTPacket_formatPayload(int buflen, char * buf, int payloadlen, char * payload) Line 863 C
paho-mqtt3as.dll!MQTTPacket_send_publish(Publish * pack, int dup, int qos, int retained, networkHandles * net, const char * clientID) Line 956 C
paho-mqtt3as.dll!MQTTProtocol_startPublishCommon(Clients * pubclient, Publish * publish, int qos, int retained) Line 154 C
paho-mqtt3as.dll!MQTTProtocol_startPublish(Clients * pubclient, Publish * publish, int qos, int retained, Messages * * mm) Line 189 C
paho-mqtt3as.dll!MQTTAsync_processCommand() Line 1472 C
paho-mqtt3as.dll!MQTTAsync_sendThread(void * n) Line 1844 C

The 'char' in this function signature 'int MQTTPacket_formatPayload(int buflen, char* buf, int payloadlen, char* payload)' can obviously be any byte character. To be honest, I'm surprised the isprint() has an assert in debug version. Like was mentions already it should be cast to 'unsighned char' to safely call isprint():

Like all other functions from , the behavior of std::isprint is undefined if the argument's value is neither representable as unsigned char nor equal to EOF. To use these functions safely with plain chars (or signed chars), the argument should first be converted to unsigned char:

bool my_isprint(char ch)
{
return std::isprint(static_cast(ch));
}

[icraggs]

Yes, it was introduced two months ago. The previous version 1.3.13 doesn't have it.