From bff24f5395353563a22e1a7a1d6d08fe36953fbe Mon Sep 17 00:00:00 2001 From: Elektr0Vodka <211697683+Elektr0Vodka@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:16:08 +0200 Subject: [PATCH] fix(filter): always build the rate limiters from the active prefs Filter::load() returned before initialising the per-type limiters when /filter_prefs did not exist yet, and `filter reset` restored the default prefs without touching the limiters. A default Limiter has limit 0, which means "no limit": on a fresh repeater `filter on` applied none of the default rate limits (5-20 per 60 s) until the first reboot after a save, and after `filter reset` the last `filter rate` kept being enforced while `filter rate` reported the defaults. All three paths (constructor, resetPrefs, load) now end in initLimiters(). Test: test_filter_limits drives Filter through load/handleCommand/ allowPacketForward on the native env. That is the first test that compiles Filter.cpp natively; it needed `constrain` in the Arduino mock. --- examples/simple_repeater/Filter.cpp | 2 +- examples/simple_repeater/Filter.h | 11 +++ platformio.ini | 1 + test/mocks/Arduino.h | 5 ++ .../test_filter_limits/test_filter_limits.cpp | 85 +++++++++++++++++++ 5 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 test/test_filter_limits/test_filter_limits.cpp diff --git a/examples/simple_repeater/Filter.cpp b/examples/simple_repeater/Filter.cpp index 51cb756856a..bccf2626a42 100644 --- a/examples/simple_repeater/Filter.cpp +++ b/examples/simple_repeater/Filter.cpp @@ -775,8 +775,8 @@ bool Filter::load(FILESYSTEM* fs) { _prefs.payload_prefs[i].hops_max = constrain(_prefs.payload_prefs[i].hops_max, 0, 64); // a soft cutoff at/above the hard limit leaves no ramp room -> treat as off if (_prefs.soft_limit[i] >= _prefs.payload_prefs[i].rate_limit) _prefs.soft_limit[i] = 0; - _limiters[i].init(_prefs.payload_prefs[i].rate_limit, _prefs.payload_prefs[i].rate_secs, _prefs.soft_limit[i]); } + initLimiters(); _prefs.minimal_hash_bytes = constrain(_prefs.minimal_hash_bytes, 1, 3); _prefs.filter_malformed = constrain(_prefs.filter_malformed, 0, 1); _prefs.dryrun = constrain(_prefs.dryrun, 0, 1); diff --git a/examples/simple_repeater/Filter.h b/examples/simple_repeater/Filter.h index fc7695b4c60..ee0db032148 100644 --- a/examples/simple_repeater/Filter.h +++ b/examples/simple_repeater/Filter.h @@ -86,6 +86,15 @@ class Filter { // to every reason alike. Returns the forwarding verdict. bool drop(const mesh::Packet *packet); + // Rebuild the per-type rate limiters from _prefs. Every path that replaces + // the prefs (defaults, reset, load) must end here, or the limiters keep + // enforcing whatever they held before (all-zero = no limit at all). + void initLimiters() { + for (uint8_t i = 0; i < PAYLOAD_TYPE_COUNT; ++i) { + _limiters[i].init(_prefs.payload_prefs[i].rate_limit, _prefs.payload_prefs[i].rate_secs, _prefs.soft_limit[i]); + } + } + // xorshift32: cheap, dependency-free source of a random byte for the soft // cutoff. State is never allowed to reach 0 (that would freeze the sequence). uint8_t nextRandom() { @@ -106,6 +115,7 @@ class Filter { Filter(ClientACL &acl, mesh::RTCClock &rtc) : _acl(&acl), _rtc(&rtc), _radio(nullptr), _rng_state(0xA5A5F00D) { memset(_sender_last, 0, sizeof(_sender_last)); memset(_text_last, 0, sizeof(_text_last)); + initLimiters(); } void setRadio(mesh::Radio *radio) { _radio = radio; } void resetPrefs(void) { @@ -114,6 +124,7 @@ class Filter { _advert.clear(); memset(_sender_last, 0, sizeof(_sender_last)); memset(_text_last, 0, sizeof(_text_last)); + initLimiters(); } void resetStats(void) { _cnt.reset(); } bool allowPacketForward(const mesh::Packet *packet); diff --git a/platformio.ini b/platformio.ini index 27e7585a493..ab02d368f2f 100644 --- a/platformio.ini +++ b/platformio.ini @@ -176,6 +176,7 @@ build_src_filter = +<../src/helpers/DynamicConfigSerializer.cpp> +<../src/helpers/DutyCycleLimits.cpp> +<../examples/companion_radio/CompanionFilter.cpp> + +<../examples/simple_repeater/Filter.cpp> lib_deps = google/googletest @ 1.17.0 diff --git a/test/mocks/Arduino.h b/test/mocks/Arduino.h index 960a7504d86..4aed764ed2c 100644 --- a/test/mocks/Arduino.h +++ b/test/mocks/Arduino.h @@ -28,6 +28,11 @@ inline void delay(uint32_t ms) { g_mock_millis += ms; } +// Same definition as the Arduino core macro. +#ifndef constrain + #define constrain(amt, low, high) ((amt) < (low) ? (low) : ((amt) > (high) ? (high) : (amt))) +#endif + // ---- Arduino string helpers not present in the host libc -------------------- // (MinGW/Windows already declares ltoa in , so only define it where // the host libc lacks it — e.g. glibc on Linux.) diff --git a/test/test_filter_limits/test_filter_limits.cpp b/test/test_filter_limits/test_filter_limits.cpp new file mode 100644 index 00000000000..67f17596a17 --- /dev/null +++ b/test/test_filter_limits/test_filter_limits.cpp @@ -0,0 +1,85 @@ +#include + +// Drives the repeater Filter through its own interface (load, handleCommand, +// allowPacketForward) to check that the per-type rate limiters always match +// the active prefs, whichever way those prefs were arrived at. +#include "../../examples/simple_repeater/Filter.h" + +// src/Identity.cpp needs the ed25519 library, which the native env does not +// build. ClientACL only needs the default constructor. +mesh::Identity::Identity() { memset(pub_key, 0, sizeof(pub_key)); } + +namespace { + +class FixedClock : public mesh::RTCClock { + uint32_t _now = 1700000000; +public: + uint32_t getCurrentTime() override { return _now; } + void setCurrentTime(uint32_t time) override { _now = time; } +}; + +// A flood ACK: not a priority type, so it reaches the per-type rate limiter. +// The default limit for ACK is 5 per 60 s (FilterPrefs in Filter.h). +mesh::Packet floodAck() { + mesh::Packet p; + p.header = ROUTE_TYPE_FLOOD | (PAYLOAD_TYPE_ACK << PH_TYPE_SHIFT); + p.setPathHashSizeAndCount(1, 0); + p.payload_len = 4; + memset(p.payload, 0, sizeof(p.payload)); + return p; +} + +// Number of `sent` identical flood ACKs (same second) the filter forwards. +int forwarded(Filter& filter, int sent) { + mesh::Packet p = floodAck(); + int n = 0; + for (int i = 0; i < sent; i++) { + if (filter.allowPacketForward(&p)) n++; + } + return n; +} + +void command(Filter& filter, FILESYSTEM& fs, const char* cmd) { + char buf[64]; + char reply[FILTER_REPLY_SIZE] = ""; + strncpy(buf, cmd, sizeof(buf) - 1); + buf[sizeof(buf) - 1] = '\0'; + filter.handleCommand(&fs, buf, reply); +} + +} // namespace + +// A repeater with no /filter_prefs yet (the mock FS has no files) must still +// enforce the default rate limits once the filter is switched on. +TEST(FilterLimits, FreshDeviceAppliesDefaultRateLimit) { + ClientACL acl; + FixedClock clock; + FsMock fs; + Filter filter(acl, clock); + + ASSERT_TRUE(filter.load(&fs)); + command(filter, fs, "filter on"); + + EXPECT_EQ(forwarded(filter, 6), 5); +} + +// `filter reset` restores the default prefs, so it must restore the default +// limiter too instead of keeping the last `filter rate` setting active. +TEST(FilterLimits, ResetRestoresDefaultRateLimit) { + ClientACL acl; + FixedClock clock; + FsMock fs; + Filter filter(acl, clock); + + ASSERT_TRUE(filter.load(&fs)); + command(filter, fs, "filter rate 3 100 60"); + command(filter, fs, "filter reset"); + command(filter, fs, "filter on"); + + EXPECT_EQ(forwarded(filter, 6), 5); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}