From e25192cf17daeae879ffbcb174ad24c7f31b21c5 Mon Sep 17 00:00:00 2001 From: ladyada Date: Tue, 15 Sep 2026 19:07:35 -0400 Subject: [PATCH] Reuse NMEA validation in GPS sentence checks Use the shared validator before source and sentence recognition, reject NULL input safely, and require an exact sentence name instead of a recognized prefix. Cover recognition diagnostics, malformed payloads, checksum trailers, and zero-field messages. Co-authored-by: Limor Fried --- .../tests/gps_validation/gps_validation.ino | 131 ++++++++++++++++++ src/NMEA_parse.cpp | 47 ++----- 2 files changed, 143 insertions(+), 35 deletions(-) create mode 100644 extras/tests/gps_validation/gps_validation.ino diff --git a/extras/tests/gps_validation/gps_validation.ino b/extras/tests/gps_validation/gps_validation.ino new file mode 100644 index 0000000..a3d5708 --- /dev/null +++ b/extras/tests/gps_validation/gps_validation.ino @@ -0,0 +1,131 @@ +#include + +Adafruit_GPS GPS; +bool testsPassed = false; +bool checkSentence(const char *body, bool accepted, int checks, + const char *source, const char *sentence); + +void setup() { + Serial.begin(115200); + Serial.println(F("Adafruit GPS shared validation regression")); + int framed = NMEA_HAS_DOLLAR + NMEA_HAS_CHECKSUM; + int sourced = framed + NMEA_HAS_SOURCE; + int parsed = sourced + NMEA_HAS_SENTENCE + NMEA_HAS_SENTENCE_P; + if (!checkSentence("$GPGGA,", true, parsed, "GP", "GGA") || + !checkSentence("!GNGLL,", true, parsed, "GN", "GLL") || + !checkSentence("$PGTOP,11,2", true, parsed, "PG", "TOP") || + !checkSentence("$PCD,11,1", true, parsed, "P", "CD") || + !checkSentence("$GPZZZ,", false, sourced, "GP", "ZZZ") || + !checkSentence("$XXGGA,", false, framed, "", "")) { + return; + } +#ifdef NMEA_EXTENSIONS + if (!checkSentence("$GPGSV,", false, sourced + NMEA_HAS_SENTENCE, "GP", "GSV")) { + return; + } +#else + if (!checkSentence("$GPDBT,", false, sourced + NMEA_HAS_SENTENCE, "GP", "DBT")) { + return; + } +#endif + Serial.println(F("PASS: recognized, unknown, proprietary, and known-only IDs")); + + // A recognized prefix must not select the wrong decoder. + if (!checkSentence("$GPGGAX,", false, sourced, "GP", "GGAX") || + !checkSentence("$GPGSVX,", false, sourced, "GP", "GSVX") || + !checkSentence("$PCDX,11,1", false, sourced, "P", "CDX")) { + return; + } + char extended[100] = + "$GPGGAX,123519,4807.038,N,01131.000,E,1,08,0.9,545.4,M,46.9,M,,"; + GPS.addChecksum(extended); + GPS.hour = 7; + GPS.altitude = 12; + if (GPS.parse(extended) || GPS.hour != 7 || GPS.altitude != 12) { + Serial.println(F("FAIL: an extended address changed navigation data")); + return; + } + Serial.println(F("PASS: sentence names require an exact match")); + + // These retain a matching XOR checksum but have invalid sentence syntax. + const char *malformed[] = {"$GPGGA!,1", "$GPGGA,1\t2", "$GPGGA,1$2", + "$GPGGA,1!2", "$GPGGA,1*2", "$GPGGA,1\n2"}; + for (uint8_t i = 0; i < sizeof(malformed) / sizeof(malformed[0]); i++) { + if (!checkSentence(malformed[i], false, NMEA_HAS_DOLLAR, "", "")) { + return; + } + } + Serial.println(F("PASS: malformed payloads are rejected despite valid XOR")); + + const char *shortLines[] = {"", "$", "!", "$GPGGA*", "$GPGGA*0", "$GPGGA*GG"}; + if (GPS.check(NULL) || GPS.parse(NULL) || GPS.thisCheck || GPS.thisSource[0] || + GPS.thisSentence[0]) { + Serial.println(F("FAIL: NULL input or stale diagnostic fields")); + return; + } + for (uint8_t i = 0; i < sizeof(shortLines) / sizeof(shortLines[0]); i++) { + char line[20]; + strcpy(line, shortLines[i]); + if (GPS.check(line) || GPS.thisSource[0] || GPS.thisSentence[0]) { + Serial.println(F("FAIL: incomplete input was accepted")); + return; + } + } + Serial.println(F("PASS: NULL, empty, and incomplete input")); + + // Zero data fields remain a framing success, independent of GPS decoding. + if (!checkSentence("$GPGGA", true, parsed, "GP", "GGA")) { + return; + } + char knownZeroFields[] = "$GPGGA*56"; + if (GPS.parse(knownZeroFields)) { + Serial.println(F("FAIL: GGA was decoded without its required fields")); + return; + } + char zeroFields[] = "$PQTMVERNO*58"; + if (Adafruit_NMEA::validate(zeroFields, strlen(zeroFields)).status != + NMEA_FRAME_VALID || GPS.check(zeroFields) || GPS.thisCheck != sourced) { + Serial.println(F("FAIL: zero-field proprietary response classification")); + return; + } + const char *endings[] = {"", "\r", "\n", "\r\n"}; + for (uint8_t i = 0; i < sizeof(endings) / sizeof(endings[0]); i++) { + char line[24] = "$GPGGA,*7a"; + strcat(line, endings[i]); + if (!GPS.check(line) || GPS.thisCheck != parsed) { + Serial.println(F("FAIL: lowercase checksum or supported line ending")); + return; + } + } + char badChecksum[] = "$GPGGA,*00"; + char badTrailer[] = "$GPGGA,*7Aextra"; + if (GPS.check(badChecksum) || GPS.thisCheck != NMEA_HAS_DOLLAR || + GPS.check(badTrailer) || GPS.thisCheck != NMEA_HAS_DOLLAR) { + Serial.println(F("FAIL: bad checksum or trailer")); + return; + } + Serial.println(F("PASS: zero fields, checksum cases, and line endings")); + testsPassed = true; +} + +void loop() { + if (testsPassed) { + Serial.println(F("PASS: GPS shared validation regression")); + } else { + Serial.println(F("FAIL: GPS shared validation regression")); + } +} + +bool checkSentence(const char *body, bool accepted, int checks, + const char *source, const char *sentence) { + char line[100]; + strcpy(line, body); + GPS.addChecksum(line); + if (GPS.check(line) != accepted || GPS.thisCheck != checks || + strcmp(GPS.thisSource, source) || strcmp(GPS.thisSentence, sentence)) { + Serial.print(F("FAIL: validation or diagnostics for ")); + Serial.println(body); + return false; + } + return true; +} diff --git a/src/NMEA_parse.cpp b/src/NMEA_parse.cpp index 8a9cab8..a634692 100644 --- a/src/NMEA_parse.cpp +++ b/src/NMEA_parse.cpp @@ -629,9 +629,8 @@ bool Adafruit_GPS::parse(char *nmea) { /**************************************************************************/ /*! - @brief Check an NMEA string for basic format, valid source ID and valid - and valid sentence ID. Update the values of thisCheck, thisSource and - thisSentence. + @brief Validate NMEA framing and checksum, then recognize its source and + complete sentence ID. Update thisCheck, thisSource and thisSentence. @param nmea Pointer to the NMEA string @return True if well formed, false if it has problems */ @@ -639,39 +638,16 @@ bool Adafruit_GPS::parse(char *nmea) { bool Adafruit_GPS::check(char *nmea) { thisCheck = 0; // new check *thisSentence = *thisSource = 0; - if (*nmea != '$' && *nmea != '!') + if (!nmea || (*nmea != '$' && *nmea != '!')) return false; // doesn't start with $ or ! else thisCheck += NMEA_HAS_DOLLAR; - // do checksum check -- first look if we even have one -- ignore all but last - // * - char *ast = nmea; // not strchr(nmea,'*'); for first * - while (*ast) - ast++; // go to the end - while (*ast != '*' && ast > nmea) - ast--; // then back to * if it's there - if (*ast != '*') - return false; // there is no asterisk - else { - if (!isxdigit((unsigned char)ast[1]) || !isxdigit((unsigned char)ast[2])) - return false; // checksum must contain two hexadecimal digits - char *end = ast + 3; - if (*end == '\r') - end++; - if (*end == '\n') - end++; - if (*end != '\0') - return false; // only a line ending may follow the checksum - uint16_t sum = parseHex(toupper((unsigned char)ast[1])) * 16; - sum += parseHex(toupper((unsigned char)ast[2])); - char *p = nmea; // check checksum - for (char *p1 = p + 1; p1 < ast; p1++) - sum ^= *p1; - if (sum != 0) - return false; // bad checksum :( - else - thisCheck += NMEA_HAS_CHECKSUM; - } + // Share framing and checksum rules with the byte receiver. Recognition + // remains here so unknown proprietary replies are still valid NMEA frames. + nmea_sentence_t sentence = Adafruit_NMEA::validate(nmea, strlen(nmea)); + if (sentence.status != NMEA_FRAME_VALID) + return false; + thisCheck += NMEA_HAS_CHECKSUM; // extract source of variable length char *p = nmea + 1; const char *src = tokenOnList(p, sources); @@ -681,14 +657,15 @@ bool Adafruit_GPS::check(char *nmea) { } else return false; p += strlen_P(src); + size_t sentenceLength = sentence.address.length - strlen_P(src); // extract sentence id and check if parsed const char *snc = tokenOnList(p, sentences_parsed); - if (snc) { + if (snc && strlen_P(snc) == sentenceLength) { strcpy_P(thisSentence, snc); thisCheck += NMEA_HAS_SENTENCE_P + NMEA_HAS_SENTENCE; } else { // check if known snc = tokenOnList(p, sentences_known); - if (snc) { + if (snc && strlen_P(snc) == sentenceLength) { strcpy_P(thisSentence, snc); thisCheck += NMEA_HAS_SENTENCE; return false; // known but not parsed