diff --git a/src/helpers/MQTTReplyFormat.h b/src/helpers/MQTTReplyFormat.h index bdb343d7..0ff5e38e 100644 --- a/src/helpers/MQTTReplyFormat.h +++ b/src/helpers/MQTTReplyFormat.h @@ -2,6 +2,7 @@ #include #include +#include #include // Bounded, clamping printf-append for the fixed-size CLI reply buffers used by @@ -41,3 +42,11 @@ static inline void replyAppendf(char* buf, size_t bufsize, int* pos, const char* *pos += n; if ((size_t)*pos >= bufsize) *pos = (int)bufsize - 1; // clamp truncated append } + +// Magnitude of an mbedTLS stack error, for printing as "-0x%04X". ESP-IDF 4.4 stores +// the positive magnitude (it captures -ret), so negating it produces a garbage unsigned; +// accepting either sign keeps this correct if a later SDK stores the negative code. +// The int64_t cast matters: negating INT32_MIN directly is undefined behaviour. +static inline uint32_t mbedtlsErrorMagnitude(int32_t stack_err) { + return (stack_err < 0) ? (uint32_t)(-(int64_t)stack_err) : (uint32_t)stack_err; +} diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index efb33d69..6013a4dd 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -559,9 +559,11 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) replyAppendf(buf, bufsize, &pos, ", tls:0x%04X", (unsigned)slot.last_tls_err); } } - // mbedTLS stack error (shown as negative hex per convention) + // mbedTLS stack error (shown as negative hex per convention). ESP-IDF stores the + // magnitude, not the negative mbedTLS code, so normalise instead of negating. if (slot.last_tls_stack_err != 0) { - replyAppendf(buf, bufsize, &pos, ", mbedtls:-0x%04X", (unsigned)(-slot.last_tls_stack_err)); + replyAppendf(buf, bufsize, &pos, ", mbedtls:-0x%04X", + (unsigned)mbedtlsErrorMagnitude(slot.last_tls_stack_err)); } // Socket errno if (slot.last_sock_errno != 0) { diff --git a/test/test_mqtt_reply_format/test_mqtt_reply_format.cpp b/test/test_mqtt_reply_format/test_mqtt_reply_format.cpp index 4b3726af..52ff33f8 100644 --- a/test/test_mqtt_reply_format/test_mqtt_reply_format.cpp +++ b/test/test_mqtt_reply_format/test_mqtt_reply_format.cpp @@ -88,6 +88,27 @@ TEST(ReplyAppendf, NoWritePastBufferAcrossOverflowingChain) { EXPECT_EQ(c.buf()[c.logical - 1], '\0'); // still NUL-terminated } +// The caller, not the helper, was where the mbedtls: line went wrong: production +// negated a value ESP-IDF already stores as a magnitude, so it rendered +// "mbedtls:-0xFFFF8100" instead of "-0x7F00". The chain test above never caught it +// because it passes the correct magnitude straight in. +TEST(MbedtlsErrorMagnitude, NormalisesEitherSignForTheNegativeHexConvention) { + EXPECT_EQ(mbedtlsErrorMagnitude(0x7F00), 0x7F00u); // ESP-IDF 4.4 stores +magnitude + EXPECT_EQ(mbedtlsErrorMagnitude(-0x7F00), 0x7F00u); // a later SDK may store the real code + EXPECT_EQ(mbedtlsErrorMagnitude(0), 0u); + + char buf[32]; + int pos = 0; + replyAppendf(buf, sizeof(buf), &pos, ", mbedtls:-0x%04X", + (unsigned)mbedtlsErrorMagnitude(0x7F00)); + EXPECT_STREQ(buf, ", mbedtls:-0x7F00"); +} + +// Negating INT32_MIN is undefined behaviour, which is why the helper widens first. +TEST(MbedtlsErrorMagnitude, HandlesInt32MinWithoutOverflow) { + EXPECT_EQ(mbedtlsErrorMagnitude(INT32_MIN), 2147483648u); +} + TEST(ReplyAppendf, ExactFitBoundary) { char buf[11]; // room for exactly "0123456789" + NUL int pos = 0;