From 10aaa210a1761451e5febc1e086758b331a1d9b7 Mon Sep 17 00:00:00 2001 From: nullrouten0 <201262769+nullrouten0@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:33:36 +0000 Subject: [PATCH 1/2] test: add a native GPS/NMEA test environment Add a native_gps PlatformIO environment that compiles the real MicroNMEALocationProvider header against the real MicroNMEA 2.0.6 library, with the address and undefined-behaviour sanitizers on, and run it in the unit-test workflow. The mocks gain what the provider header needs to compile on a host: yield(), the C character classification functions, minimal digital I/O, mesh::RTCClock and a DateTime stand-in. The suite has a shared helper header and starts with a parsing smoke test on well-formed GGA and RMC sentences; later changes add their own test files to it. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/run-unit-tests.yml | 2 +- platformio.ini | 25 +++++++++++++- test/mocks/Arduino.h | 24 +++++++++++++ test/mocks/Mesh.h | 7 ++++ test/mocks/RTClib.h | 39 +++++++++++++++++++++ test/test_gps_nmea/gps_test_support.h | 49 +++++++++++++++++++++++++++ test/test_gps_nmea/test_main.cpp | 9 +++++ test/test_gps_nmea/test_parsing.cpp | 30 ++++++++++++++++ 8 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 test/mocks/RTClib.h create mode 100644 test/test_gps_nmea/gps_test_support.h create mode 100644 test/test_gps_nmea/test_main.cpp create mode 100644 test/test_gps_nmea/test_parsing.cpp diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 5d48f4c67be..edce11a9668 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -20,7 +20,7 @@ jobs: uses: ./.github/actions/setup-build-environment - name: Run Unit Tests - run: pio test -e native -e native_kiss_modem -vv + run: pio test -e native -e native_kiss_modem -e native_gps -vv - name: Upload Test Results # Upload test results even if the test step failed. diff --git a/platformio.ini b/platformio.ini index f296f838213..44ca4a45f53 100644 --- a/platformio.ini +++ b/platformio.ini @@ -167,7 +167,7 @@ build_flags = -std=c++17 -I src -I test/mocks test_build_src = yes -test_ignore = test_kiss_modem +test_ignore = test_kiss_modem, test_gps_nmea build_src_filter = -<*> +<../src/Utils.cpp> @@ -191,3 +191,26 @@ build_src_filter = +<../examples/kiss_modem/KissModem.cpp> lib_deps = google/googletest @ 1.17.0 + +; GPS/NMEA provider tests compile the real provider header against the real +; MicroNMEA library. The mocks must shadow src/ (same ordering as the KISS +; modem suite). MicroNMEA is an Arduino-format library, so compatibility +; checks are off to let it build for the native platform. Sanitizers make +; out-of-bounds reads in the parser fail the test instead of passing silently. +[env:native_gps] +platform = native +test_framework = googletest +build_flags = -std=c++17 + -I test/mocks + -I src + -fsanitize=address,undefined + -fno-sanitize-recover=all + -fno-omit-frame-pointer +test_build_src = yes +test_filter = test_gps_nmea +build_src_filter = + -<*> +lib_compat_mode = off +lib_deps = + google/googletest @ 1.17.0 + stevemarple/MicroNMEA @ 2.0.6 diff --git a/test/mocks/Arduino.h b/test/mocks/Arduino.h index 77499fe4145..bcd6620c4f4 100644 --- a/test/mocks/Arduino.h +++ b/test/mocks/Arduino.h @@ -2,6 +2,7 @@ #include #include +#include // Arduino.h provides the C character classification functions #include "Stream.h" inline uint32_t g_mock_millis = 0; @@ -15,3 +16,26 @@ inline uint32_t millis() { inline void delay(uint32_t ms) { g_mock_millis += ms; } + +// Busy-wait loops call yield(); let mock time move so they can time out. +inline void yield() { + g_mock_millis += 1; +} + +// Minimal digital I/O so headers that drive enable/reset pins compile natively. +constexpr uint8_t LOW = 0; +constexpr uint8_t HIGH = 1; +constexpr uint8_t INPUT = 0; +constexpr uint8_t OUTPUT = 1; + +inline uint8_t g_mock_pin_levels[64] = {}; + +inline void pinMode(uint8_t, uint8_t) {} + +inline void digitalWrite(uint8_t pin, uint8_t level) { + if (pin < 64) g_mock_pin_levels[pin] = level; +} + +inline int digitalRead(uint8_t pin) { + return pin < 64 ? g_mock_pin_levels[pin] : LOW; +} diff --git a/test/mocks/Mesh.h b/test/mocks/Mesh.h index b6c263c199a..6f73b0b0bc4 100644 --- a/test/mocks/Mesh.h +++ b/test/mocks/Mesh.h @@ -15,6 +15,13 @@ class Radio { virtual int16_t getNoiseFloor() { return -120; } }; +class RTCClock { +public: + virtual ~RTCClock() = default; + virtual uint32_t getCurrentTime() = 0; + virtual void setCurrentTime(uint32_t time) = 0; +}; + class MainBoard { public: virtual ~MainBoard() = default; diff --git a/test/mocks/RTClib.h b/test/mocks/RTClib.h new file mode 100644 index 00000000000..6a02326a04b --- /dev/null +++ b/test/mocks/RTClib.h @@ -0,0 +1,39 @@ +#pragma once + +#include + +// Minimal DateTime stand-in for native tests. unixtime() matches RTClib for +// valid dates from 2000 to 2099, the range RTClib supports (seconds since +// 1970-01-01 UTC). Out-of-range fields map to a deterministic value instead of +// indexing past the month table, so tests can compare two parses of the same +// bytes; such values are not what RTClib would return. +class DateTime { + uint16_t _year; + uint8_t _month, _day, _hour, _minute, _second; + + static bool leap(uint16_t year) { + return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); + } + +public: + DateTime(uint16_t year, uint8_t month, uint8_t day, uint8_t hour, + uint8_t minute, uint8_t second) + : _year(year), _month(month), _day(day), _hour(hour), + _minute(minute), _second(second) {} + + uint32_t unixtime() const { + uint32_t days = 0; + for (uint16_t year = 1970; year < _year; ++year) + days += leap(year) ? 366 : 365; + if (_month >= 1 && _month <= 12) { + const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, + 31, 31, 30, 31, 30, 31}; + for (uint8_t month = 1; month < _month; ++month) + days += month_days[month - 1] + (month == 2 && leap(_year) ? 1 : 0); + days += _day - 1; + } else { + days += 31UL * _month + _day; // deterministic, not calendar-correct + } + return days * 86400UL + _hour * 3600UL + _minute * 60UL + _second; + } +}; diff --git a/test/test_gps_nmea/gps_test_support.h b/test/test_gps_nmea/gps_test_support.h new file mode 100644 index 00000000000..0a7b733ef4c --- /dev/null +++ b/test/test_gps_nmea/gps_test_support.h @@ -0,0 +1,49 @@ +#pragma once + +// Shared helpers for the native GPS/NMEA tests: the real +// MicroNMEALocationProvider and MicroNMEA, fed through a mock UART. + +#include + +#include +#include + +#include +#include +#include +#include + + +struct MockSerial : Stream { + std::deque queue; + int available() override { return (int)queue.size(); } + int read() override { + if (queue.empty()) return -1; + char c = queue.front(); + queue.pop_front(); + return (unsigned char)c; + } + void feed(const std::string& text) { + for (char c : text) queue.push_back(c); + } +}; + +// "$" + body + "*" + two hex digits, the checksum MicroNMEA expects. +inline std::string withChecksum(const std::string& body) { + uint8_t sum = 0; + for (char c : body) sum ^= (uint8_t)c; + char hex[4]; + snprintf(hex, sizeof(hex), "%02X", sum); + return "$" + body + "*" + hex; +} + +inline void feedAndLoop(MicroNMEALocationProvider& gps, MockSerial& serial, + const std::string& line) { + serial.feed(line + "\r\n"); + gps.loop(); +} + +inline constexpr const char* kGoodRmc = + "GPRMC,120034.00,A,4716.1234,N,12233.5678,W,0.12,34.5,171026,003.1,W,A"; +inline constexpr const char* kGoodGga = + "GPGGA,120034.00,4716.1234,N,12233.5678,W,1,08,0.9,125.0,M,-17.0,M,,"; diff --git a/test/test_gps_nmea/test_main.cpp b/test/test_gps_nmea/test_main.cpp new file mode 100644 index 00000000000..c26f6efeba8 --- /dev/null +++ b/test/test_gps_nmea/test_main.cpp @@ -0,0 +1,9 @@ +#include "gps_test_support.h" + +// Defined in EnvironmentSensorManager.cpp, which is not part of this build. +void LocationProvider::sendSentence(const char*) {} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/test_gps_nmea/test_parsing.cpp b/test/test_gps_nmea/test_parsing.cpp new file mode 100644 index 00000000000..4755ba77e0c --- /dev/null +++ b/test/test_gps_nmea/test_parsing.cpp @@ -0,0 +1,30 @@ +#include "gps_test_support.h" + +TEST(MicroNMEAProvider, ParsesWellFormedSentences) { + g_mock_millis = 1000; + MockSerial serial; + MicroNMEALocationProvider gps(serial, nullptr, -1, -1); + feedAndLoop(gps, serial, withChecksum(kGoodGga)); + feedAndLoop(gps, serial, withChecksum(kGoodRmc)); + + EXPECT_TRUE(gps.isValid()); + EXPECT_EQ(47268723L, gps.getLatitude()); // 47 deg 16.1234' N + EXPECT_EQ(-122559463L, gps.getLongitude()); // 122 deg 33.5678' W + EXPECT_EQ(8L, gps.satellitesCount()); + EXPECT_EQ(125000L, gps.getAltitude()); + EXPECT_EQ(1792238434L, gps.getTimestamp()); // 2026-10-17 12:00:34 UTC +} + +TEST(MicroNMEAProvider, LongestSentenceTheBufferHoldsStillParses) { + std::string body = kGoodRmc; + while (body.size() < 99 - 4) body.push_back('Z'); // pad the final field + const std::string sentence = withChecksum(body); + ASSERT_EQ(99u, sentence.size()); + + g_mock_millis = 1000; + MockSerial serial; + MicroNMEALocationProvider gps(serial, nullptr, -1, -1); + feedAndLoop(gps, serial, sentence); + EXPECT_TRUE(gps.isValid()); + EXPECT_EQ(47268723L, gps.getLatitude()); +} From 1ac389bbef0084623a3115c526156ae8f414d3ea Mon Sep 17 00:00:00 2001 From: nullrouten0 <201262769+nullrouten0@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:40:53 +0000 Subject: [PATCH 2/2] GPS: keep MicroNMEA's over-reads inside the provider's NMEA buffer MicroNMEA 2.0.6 never writes past the buffer length it is given, but its field parsers use fixed-width skips that do not check for the end of the sentence, and its checksum test stops at the first '*'. A 99-character sentence whose checksum covers only its first few fields therefore makes the parser read up to five bytes past the terminating NUL, into the MicroNMEA object that follows the buffer in MicroNMEALocationProvider. The result depends on object layout, and the parse can leave isValid() true with a meaningless position. Reserve a zeroed 16-byte tail behind the 100 bytes the library is given. The accepted sentence length and the parse of every well-formed sentence are unchanged; malformed sentences now parse as if followed by NULs. Tests: a heap buffer of exactly the reserved size under the address sanitizer (a 4-byte guard fails it, 5 passes), the tail is never written, sentences of 100+ characters are still rejected, and a provider built in non-zero memory parses a crafted sentence's position the same as a NUL-padded reference. Co-Authored-By: Claude Opus 5.5 --- .../sensors/MicroNMEALocationProvider.h | 14 +- test/test_gps_nmea/test_buffer_guard.cpp | 163 ++++++++++++++++++ 2 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 test/test_gps_nmea/test_buffer_guard.cpp diff --git a/src/helpers/sensors/MicroNMEALocationProvider.h b/src/helpers/sensors/MicroNMEALocationProvider.h index 37ef5762c48..3168d8bac30 100644 --- a/src/helpers/sensors/MicroNMEALocationProvider.h +++ b/src/helpers/sensors/MicroNMEALocationProvider.h @@ -39,7 +39,17 @@ #endif class MicroNMEALocationProvider : public LocationProvider { - char _nmeaBuffer[100]; +public: + // MicroNMEA never writes past the length it is given, but its fixed-width + // field skips can read a few bytes past the terminating NUL of a malformed + // sentence whose checksum still passes. The zeroed tail behind the length + // the library sees makes those reads land on NUL bytes, so the parser stops + // there instead of reading the members that follow the buffer. + static constexpr uint8_t NMEA_BUFFER_LEN = 100; + static constexpr size_t NMEA_BUFFER_GUARD = 16; + +private: + char _nmeaBuffer[NMEA_BUFFER_LEN + NMEA_BUFFER_GUARD] = {}; MicroNMEA nmea; mesh::RTCClock* _clock; Stream* _gps_serial; @@ -90,7 +100,7 @@ class MicroNMEALocationProvider : public LocationProvider { public : MicroNMEALocationProvider(Stream& ser, mesh::RTCClock* clock = NULL, int pin_reset = GPS_RESET, int pin_en = GPS_EN,RefCountedDigitalPin* peripher_power=NULL) : - nmea(_nmeaBuffer, sizeof(_nmeaBuffer)), _clock(clock), _gps_serial(&ser), _peripher_power(peripher_power), _pin_reset(pin_reset), _pin_en(pin_en) { + nmea(_nmeaBuffer, NMEA_BUFFER_LEN), _clock(clock), _gps_serial(&ser), _peripher_power(peripher_power), _pin_reset(pin_reset), _pin_en(pin_en) { if (_pin_reset != -1) { pinMode(_pin_reset, OUTPUT); digitalWrite(_pin_reset, GPS_RESET_ACTIVE); diff --git a/test/test_gps_nmea/test_buffer_guard.cpp b/test/test_gps_nmea/test_buffer_guard.cpp new file mode 100644 index 00000000000..f93dce51e0a --- /dev/null +++ b/test/test_gps_nmea/test_buffer_guard.cpp @@ -0,0 +1,163 @@ +#include "gps_test_support.h" + +#include +#include +#include + +namespace { + +constexpr size_t kLen = MicroNMEALocationProvider::NMEA_BUFFER_LEN; +constexpr size_t kGuard = MicroNMEALocationProvider::NMEA_BUFFER_GUARD; + +// The library's checksum test stops at the first '*', while parseTime() +// skips a fixed nine characters. A sentence whose checksum covers only +// "GPRMC,1" therefore passes, and the bytes after the checksum are parsed as +// fields. Both sentences below are 99 characters, the longest the buffer +// holds, so the sentence NUL sits at index 99 and the last fixed-width reads +// land past it. + +// Validity flag at index 97: processRMC steps onto the NUL and the latitude +// parse reads indices 99 and 100. +std::string overreadAtLatitude() { + std::string s = withChecksum("GPRMC,1"); // 11 chars + while (s.size() < 96) s.push_back('X'); + s += ",AB"; // ',' @96, 'A' @97, 'B' @98 + return s; +} + +// Date field at index 98: parseDate reads six bytes from index 98 (to 103) +// and then looks for the next field at index 104. +std::string overreadAtDate() { + std::string s = withChecksum("GPRMC,1"); // idx 0..10 + s += "AAAAA"; // rest of the 9-char time field + s += ",A,4716.1,N,"; // validity, latitude, hemisphere + const std::string tail = ",W,0.1,0.2,"; // longitude end .. course + s += std::string(98 - tail.size() - s.size(), 'X'); // longitude filler + s += tail; + s += "1"; // date field starts at idx 98 + return s; +} + +// A line longer than the buffer: the library keeps overwriting its last byte. +std::string longJunkLine() { return std::string(180, '7'); } + +// Parse the same bytes with a library instance whose buffer is followed by +// NUL bytes, which is what the provider's guard tail must guarantee. +struct ReferenceParse { + char buffer[100 + 64]; + MicroNMEA nmea; + ReferenceParse() : buffer{}, nmea(buffer, 100) {} + void feed(const std::string& line) { + for (char c : line) nmea.process(c); + nmea.process('\n'); + } +}; + +// Build the provider in memory that is not zero, so the test does not rely on +// a freshly zeroed stack or heap to make the guard tail look clean. +struct DirtyProvider { + alignas(MicroNMEALocationProvider) unsigned char storage[sizeof(MicroNMEALocationProvider)]; + MicroNMEALocationProvider* gps; + explicit DirtyProvider(Stream& serial) { + memset(storage, 0xA5, sizeof(storage)); + gps = new (storage) MicroNMEALocationProvider(serial, nullptr, -1, -1); + } + ~DirtyProvider() { gps->~MicroNMEALocationProvider(); } +}; + +// Checksum-valid sentences whose fields run up to the end of the buffer. +// Field values stay short: MicroNMEA's parseFloat() also overflows a signed +// long on 19+ digit fields, a separate library bug this suite does not cover. +std::string malformedNearEnd(std::mt19937& rng) { + static const char alphabet[] = ",,,,AANNWWSSEE0123456789..**X"; + std::string s = withChecksum(rng() % 2 ? "GPRMC,1" : "GPGGA,1"); + const size_t length = 80 + rng() % 20; // 80..99 characters + size_t run = 0; + while (s.size() < length) { + char c = alphabet[rng() % (sizeof(alphabet) - 1)]; + run = (c >= '0' && c <= '9') ? run + 1 : 0; + if (run > 12) c = ',', run = 0; + s.push_back(c); + } + return s; +} + +} // namespace + +TEST(MicroNMEAProvider, SentenceLongerThanTheBufferIsStillRejected) { + // The guard tail must not change which sentences are accepted: a sentence + // of 100 or more characters is truncated and fails its checksum, as before. + std::string body = kGoodRmc; + while (body.size() < 100 - 4) body.push_back('Z'); + const std::string sentence = withChecksum(body); + ASSERT_EQ(100u, sentence.size()); + + g_mock_millis = 1000; + MockSerial serial; + MicroNMEALocationProvider gps(serial, nullptr, -1, -1); + feedAndLoop(gps, serial, sentence); + EXPECT_FALSE(gps.isValid()); +} + +TEST(MicroNMEALibrary, NeverWritesPastTheLengthItIsGiven) { + // The premise of the guard tail: MicroNMEA 2.0.6 clamps writes at + // buffer[len - 1]. Long, truncated and malformed lines must leave the tail + // untouched, or it would stop being a run of NUL bytes. + std::unique_ptr buffer(new char[kLen + kGuard]()); + MicroNMEA nmea(buffer.get(), kLen); + std::mt19937 rng(7); + for (int i = 0; i < 2000; ++i) { + std::string line = (i % 3 == 0) ? longJunkLine() : malformedNearEnd(rng); + for (size_t k = rng() % 120; k > 0; --k) line.push_back((char)(' ' + rng() % 95)); + for (char c : line) nmea.process(c); + nmea.process((i % 2) ? '\n' : '\r'); + for (size_t k = kLen; k < kLen + kGuard; ++k) { + ASSERT_EQ(0, buffer[k]) << "tail byte " << k << " written at line " << i; + } + } +} + +TEST(MicroNMEALibrary, ReadsStayInsideTheGuardTail) { + // Heap buffer of exactly the size the provider reserves. With the address + // sanitizer on, any parser read past the guard aborts this test. + std::unique_ptr buffer(new char[kLen + kGuard]()); + MicroNMEA nmea(buffer.get(), kLen); + auto feed = [&nmea](const std::string& line) { + for (char c : line) nmea.process(c); + nmea.process('\n'); + }; + feed(overreadAtLatitude()); + feed(overreadAtDate()); + std::mt19937 rng(11); + for (int i = 0; i < 20000; ++i) feed(malformedNearEnd(rng)); + SUCCEED(); +} + +TEST(MicroNMEAProvider, MalformedSentenceParsesAsIfFollowedByNulBytes) { + // Without the guard tail the latitude parse reads the first byte of the + // MicroNMEA object that follows the buffer (the buffer length, 100), so the + // result depends on object layout. With it, the provider must parse the + // same latitude as a reference whose buffer is followed by NULs, also after + // a line long enough to have filled the whole buffer. Only raw parsed + // values are compared: isValid() and getTimestamp() are the provider's own + // decisions. The date-field over-read is covered by ReadsStayInsideTheGuardTail. + for (bool after_long_line : {false, true}) { + SCOPED_TRACE(after_long_line ? "after a long line" : "first line"); + const std::string sentence = overreadAtLatitude(); + ASSERT_EQ(99u, sentence.size()); + g_mock_millis = 1000; + MockSerial serial; + DirtyProvider dirty(serial); + MicroNMEALocationProvider& gps = *dirty.gps; + ReferenceParse reference; + if (after_long_line) { + feedAndLoop(gps, serial, longJunkLine()); + reference.feed(longJunkLine()); + } + feedAndLoop(gps, serial, sentence); + reference.feed(sentence); + ASSERT_TRUE(reference.nmea.isValid()); // the sentence does reach the latitude parse + EXPECT_EQ(reference.nmea.getLatitude(), gps.getLatitude()); + EXPECT_EQ(reference.nmea.getLongitude(), gps.getLongitude()); + } +}