fix(mqtt): Align JWT reuse with renewal buffer

This commit is contained in:
agessaman committed 2026-08-17 10:42:09 -07:00
1 parent 8ef671ba09
commit 1a0452121c
3 files changed
+67 -18

No files matched your search

+12 -2
View File
@@ -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) {
+19 -8
View File
@@ -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<uint32_t>(slotTokenLifetime(index)));
const bool reuse_token = MQTTConnectionPolicy::canReuseJwtForReconnect(
time_synced, has_token, force_mint, static_cast<uint32_t>(current_time),
static_cast<uint32_t>(expires_at));
static_cast<uint32_t>(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);
}
}
@@ -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<uint32_t>::max()));
}
TEST(MQTTConnectionPolicy, RenewalThrottleHasExactBoundaryAndHandlesRollover) {