diff --git a/src/helpers/MQTTConnectionPolicy.h b/src/helpers/MQTTConnectionPolicy.h index 096c5d38e..d7ff2f5e5 100644 --- a/src/helpers/MQTTConnectionPolicy.h +++ b/src/helpers/MQTTConnectionPolicy.h @@ -160,16 +160,26 @@ static inline bool tokenNeedsRenewal(bool time_synced, uint32_t current_time, // A reconnect may keep its credentials only when their validity is known to // outlast the next handshake; uncertainty refreshes them before reconnecting. +// Clearing the renewal buffer is not enough: tokenNeedsRenewal() fires the moment +// remaining reaches it, so reuse must clear it by the handshake margin too, or the +// reused token is renewed — and bounced, on a broker enforcing exp — seconds later. +// Pass 0 for renewal_buffer_secs to gate on the flat margin alone. static inline bool canReuseJwtForReconnect(bool time_synced, bool has_token, bool force_mint, uint32_t current_time, - uint32_t token_expires_at) { + uint32_t token_expires_at, + uint32_t renewal_buffer_secs) { + // Saturate rather than wrap: a wrapped floor would silently weaken this gate. + const uint32_t floor_secs = + renewal_buffer_secs > UINT32_MAX - kJwtReconnectSafetyMarginSecs + ? UINT32_MAX + : renewal_buffer_secs + kJwtReconnectSafetyMarginSecs; return time_synced && has_token && !force_mint && token_expires_at >= kMinimumValidEpoch && current_time < token_expires_at && - (token_expires_at - current_time) > kJwtReconnectSafetyMarginSecs; + (token_expires_at - current_time) > floor_secs; } static inline bool renewalAttemptAllowed(uint32_t now, uint32_t last_attempt) { diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 1ce0f1bba..efb33d69e 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -2001,8 +2001,11 @@ void MQTTBridge::teardownSlot(int index) { } // A stopped client needs connect(): reconnect() is a documented no-op on one, so reaching -// it here would strand the slot. The WiFi-transition teardown stops a client while leaving -// initial_connect_done set, so the ladder does see this state. +// it here would strand the slot. The producer is a failed esp_mqtt_client_start(), which +// leaves _started false while initial_connect_done stays set. Not the WiFi-drop teardown, +// which only stops slots still marked connected: a publishing slot's socket fails first, so +// the guard skips it — measured across a 62 s deauth, five slots, zero stops. An idle slot +// with no traffic to fail on is the one case that could still reach here that way. void MQTTBridge::reconnectSlotClient(int index) { if (index < 0 || index >= RUNTIME_MQTT_SLOTS) return; MQTTSlot& slot = _slots[index]; @@ -2221,9 +2224,13 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns : 0; const bool force_mint_after_refusal = _slot_force_jwt_mint[index]; force_mint = force_mint || force_mint_after_refusal; + // Same buffer the renewal path uses, so a reconnect never keeps a token that + // the next maintenance pass would renew (and bounce) seconds later. + const uint32_t renewal_buffer_secs = MQTTConnectionPolicy::renewalBufferSecs( + static_cast(slotTokenLifetime(index))); const bool reuse_token = MQTTConnectionPolicy::canReuseJwtForReconnect( time_synced, has_token, force_mint, static_cast(current_time), - static_cast(expires_at)); + static_cast(expires_at), renewal_buffer_secs); const char* mint_reason = "none"; if (!reuse_token) { if (backoff_level < 0) { @@ -2238,8 +2245,12 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns mint_reason = "invalid-expiry"; } else if (current_time >= expires_at) { mint_reason = "expired"; + } else if (remaining_secs <= MQTTConnectionPolicy::kJwtReconnectSafetyMarginSecs) { + mint_reason = "safety-margin"; // too little left to outlast the handshake + } else if (remaining_secs <= renewal_buffer_secs) { + mint_reason = "renewal-due"; // the renewal path wants this token now } else { - mint_reason = "safety-margin"; + mint_reason = "renewal-imminent"; // it will, within the handshake margin } } const char* mint_result = reuse_token ? "REUSED" : "FAILED"; @@ -2282,8 +2293,8 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns if (slot_uses_jwt) { prepareJwtReconnect(true, -1); } - // Via the helper: reconnect() is a no-op on a client the WiFi-drop path - // stopped, which would probe forever without ever starting it. + // Via the helper: reconnect() is a no-op on a client whose start failed, + // which would probe forever without ever starting it. reconnectSlotClient(index); // If the connect callback fires and sets slot.connected = true, // it will clear circuit_breaker_tripped via the onConnect handler @@ -2319,8 +2330,8 @@ void MQTTBridge::maintainSlotConnection(int index, unsigned long now_millis, uns // Non-JWT slots — lightweight reconnect on existing client. MQTT_DEBUG_PRINTLN("MQTT%d reconnect (non-JWT, backoff %d)", index + 1, slot.reconnect_backoff); } - // Via the helper: reconnect() is a no-op on a client the WiFi-drop path - // stopped, which would back off forever without ever starting it. + // Via the helper: reconnect() is a no-op on a client whose start failed, + // which would back off forever without ever starting it. reconnectSlotClient(index); } } diff --git a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp index c3cd469e4..f92e5b89e 100644 --- a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp +++ b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp @@ -141,16 +141,44 @@ TEST(MQTTConnectionPolicy, JwtReconnectReusesOnlyProvenValidCredentials) { const uint32_t now = 1735689600U; const uint32_t usable_expiry = now + Policy::kJwtReconnectSafetyMarginSecs + 1U; - EXPECT_TRUE(Policy::canReuseJwtForReconnect(true, true, false, now, usable_expiry)); + // Buffer 0 gates on the flat margin alone, which is what these cases cover. + EXPECT_TRUE(Policy::canReuseJwtForReconnect(true, true, false, now, usable_expiry, 0U)); EXPECT_FALSE(Policy::canReuseJwtForReconnect( - true, true, false, now, Policy::kMinimumValidEpoch - 1U)); - EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, 0U)); + true, true, false, now, Policy::kMinimumValidEpoch - 1U, 0U)); + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, 0U, 0U)); EXPECT_FALSE(Policy::canReuseJwtForReconnect( - true, true, false, now, now + Policy::kJwtReconnectSafetyMarginSecs)); - EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, now - 1U)); - EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, false, false, now, usable_expiry)); - EXPECT_FALSE(Policy::canReuseJwtForReconnect(false, true, false, now, usable_expiry)); - EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, true, now, usable_expiry)); + true, true, false, now, now + Policy::kJwtReconnectSafetyMarginSecs, 0U)); + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, now - 1U, 0U)); + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, false, false, now, usable_expiry, 0U)); + EXPECT_FALSE(Policy::canReuseJwtForReconnect(false, true, false, now, usable_expiry, 0U)); + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, true, now, usable_expiry, 0U)); +} + +TEST(MQTTConnectionPolicy, JwtReuseRefusesATokenTheRenewalPathIsAboutToBounce) { + const uint32_t now = 1735689600U; + const uint32_t buffer = Policy::renewalBufferSecs(3300U); // waev's 55-minute tokens + ASSERT_EQ(buffer, 300U); + const uint32_t floor_secs = buffer + Policy::kJwtReconnectSafetyMarginSecs; + + // The observed defect (d3, 2026-08-17T14:31): reused at 302 s remaining under the + // flat 60 s margin, then renewed five seconds later, which bounced the slot. + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, now + 302U, buffer)); + + // Anything the renewal path would act on is refused, and so is the slack above it. + EXPECT_FALSE(Policy::canReuseJwtForReconnect(true, true, false, now, now + buffer, buffer)); + EXPECT_FALSE( + Policy::canReuseJwtForReconnect(true, true, false, now, now + floor_secs, buffer)); + const uint32_t tightest_reuse = now + floor_secs + 1U; + EXPECT_TRUE(Policy::canReuseJwtForReconnect(true, true, false, now, tightest_reuse, buffer)); + + // What the margin above the buffer buys: the tightest token reuse will accept is still + // not due for renewal a full margin later, so no renewal can bounce in behind it. + EXPECT_FALSE(Policy::tokenNeedsRenewal( + true, now + Policy::kJwtReconnectSafetyMarginSecs, tightest_reuse, buffer)); + + // A buffer that would wrap the floor has to fail closed, not reuse a doomed token. + EXPECT_FALSE(Policy::canReuseJwtForReconnect( + true, true, false, now, now + 100000U, std::numeric_limits::max())); } TEST(MQTTConnectionPolicy, RenewalThrottleHasExactBoundaryAndHandlesRollover) {