diff --git a/src/WaypointStore.cpp b/src/WaypointStore.cpp index 6fb180617d..b5a0208740 100644 --- a/src/WaypointStore.cpp +++ b/src/WaypointStore.cpp @@ -10,6 +10,7 @@ #include "WaypointStore.h" #include "concurrency/LockGuard.h" #include "gps/RTC.h" +#include "meshUtils.h" #include #include #include @@ -78,13 +79,11 @@ void WaypointStore::notifyChanged() bool WaypointStore::isExpired(const meshtastic_Waypoint &wp, uint32_t now) { - if (wp.expire == 0) - return false; - + // getTime() counts from boot until the RTC is set, which reads every real expiry as future. if (now == 0) - now = getTime(); + now = getValidTime(RTCQuality::RTCQualityDevice); - return now != 0 && wp.expire <= now; + return !waypointIsActive(wp.expire, now); } bool WaypointStore::isExpired(const StoredWaypoint &entry, uint32_t now) @@ -203,7 +202,9 @@ bool WaypointStore::addFromPacket(const meshtastic_MeshPacket &packet, bool loca if (stored) *stored = entry; - if (isExpired(entry, entry.receivedTime)) { + // rx_time holds uptime, not an epoch, when has_rx_time is false; pass 0 so isExpired() resolves + // the clock itself rather than comparing an expiry against seconds since boot. + if (isExpired(entry, packet.has_rx_time ? packet.rx_time : 0)) { // Respect the lock: only the node a waypoint is locked to may delete it on our device. // An unauthorized deletion attempt is ignored entirely, rather than applied locally. for (const auto &storedEntry : waypoints) { @@ -230,11 +231,7 @@ bool WaypointStore::addFromPacket(const meshtastic_MeshPacket &packet, bool loca bool WaypointStore::purgeExpired(uint32_t now) { - if (now == 0) - now = getTime(); - if (now == 0) - return false; - + // No local clock normalization: isExpired() owns that policy, including the delete convention. bool changed = false; for (auto it = waypoints.begin(); it != waypoints.end();) { if (!isExpired(*it, now)) { diff --git a/test/test_waypoint_expiry/test_main.cpp b/test/test_waypoint_expiry/test_main.cpp index 887f9c7d19..b765207199 100644 --- a/test/test_waypoint_expiry/test_main.cpp +++ b/test/test_waypoint_expiry/test_main.cpp @@ -1,7 +1,9 @@ -// Unit tests for waypointIsActive() in src/meshUtils.h: the expire == 0 and expire == 1 sentinels, -// ordinary expiry, and an untrusted clock. +// Unit tests for waypointIsActive() and its caller WaypointStore::isExpired(): the expire == 0 and +// expire == 1 sentinels, ordinary expiry, and an untrusted clock. #include "TestUtil.h" +#include "WaypointStore.h" #include "meshUtils.h" +#include #include namespace @@ -55,6 +57,43 @@ void test_untrusted_clock_still_honours_delete() TEST_ASSERT_FALSE(waypointIsActive(1, 0)); } +// The production caller: an explicit now keeps these off the wall clock. +void test_store_expiry_matches_the_predicate() +{ + meshtastic_Waypoint wp = meshtastic_Waypoint_init_zero; + + wp.expire = 0; + TEST_ASSERT_FALSE(WaypointStore::isExpired(wp, NOW)); + wp.expire = 1; + TEST_ASSERT_TRUE(WaypointStore::isExpired(wp, NOW)); + wp.expire = NOW + 3600; + TEST_ASSERT_FALSE(WaypointStore::isExpired(wp, NOW)); + wp.expire = NOW - 1; + TEST_ASSERT_TRUE(WaypointStore::isExpired(wp, NOW)); +} + +// rx_time carries uptime rather than an epoch when has_rx_time is false (Router::computeRxTimeStamp), +// so an already-expired waypoint must still be rejected rather than compared against seconds of uptime. +void test_packet_without_rx_time_still_expires() +{ + meshtastic_Waypoint wp = meshtastic_Waypoint_init_zero; + wp.id = 4242; + wp.expire = 1600000000; // September 2020 + + meshtastic_MeshPacket packet = meshtastic_MeshPacket_init_zero; + packet.from = 0x11223344; + packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + packet.decoded.payload.size = (uint16_t)pb_encode_to_bytes(packet.decoded.payload.bytes, sizeof(packet.decoded.payload.bytes), + &meshtastic_Waypoint_msg, &wp); + packet.has_rx_time = false; + packet.rx_time = 300; // uptime seconds, not an epoch + + waypointStore.clearAllWaypoints(); + TEST_ASSERT_TRUE(waypointStore.addFromPacket(packet, false)); + TEST_ASSERT_NULL(waypointStore.findWaypoint(wp.id)); + waypointStore.clearAllWaypoints(); +} + void setup() { initializeTestEnvironment(); @@ -67,6 +106,8 @@ void setup() RUN_TEST(test_int32_max_is_active); RUN_TEST(test_untrusted_clock_expires_nothing); RUN_TEST(test_untrusted_clock_still_honours_delete); + RUN_TEST(test_store_expiry_matches_the_predicate); + RUN_TEST(test_packet_without_rx_time_still_expires); exit(UNITY_END()); }