Validate command replies before accepting a matching prefix

Co-authored-by: Limor Fried <ladyada@users.noreply.github.com>

extras/tests/README.md

@@ -24,7 +24,9 @@ reject the unsupported sentence and extended builds decode it without partial
 updates on error. Standalone C++ core tests run once.
 
 `extras/test_support` provides serial output and a real monotonic clock; the
-wakeup timeout test takes ten seconds per configuration.
+wakeup timeout test takes ten seconds per configuration. Command-wait tests
+also reject matching replies with invalid framing/checksums, keep invalid lines
+in the sentence budget, and accept proprietary replies mixed with navigation.
 I2C, SPI, GPIO, and hardware serial input abort if used. This runs the parser
 and mock-stream regressions, not physical GPS/SD hardware or MCU emulation.
 The sketches remain usable on Arduino boards with the normal Arduino core.

extras/tests/wait_for_sentence/wait_for_sentence.ino

@@ -4,15 +4,21 @@
 class TestStream : public Stream {
 public:
   const char *input = "";
+  const char *end = NULL; // Optional explicit end for embedded-NUL input.
 
-  int available() { return *input != 0; }
+  int available() {
+    if (end) {
+      return input < end;
+    }
+    return *input != 0;
+  }
   int read() {
-    if (*input) {
+    if (available()) {
       return *input++;
     }
     return -1;
   }
-  int peek() { return *input ? *input : -1; }
+  int peek() { return available() ? *input : -1; }
   void flush() {}
   size_t write(uint8_t) { return 1; }
 };
@@ -79,6 +85,94 @@ void setup() {
   }
   Serial.println(F("PASS: zero limits return without consuming data"));
 
+  // A matching prefix alone must not turn a corrupt reply into success.
+  const char *invalid[] = {
+      "$PMTK010,002*00\r\n", // Wrong checksum.
+      "$PMTK010,002*2Z\r\n", // Non-hex checksum.
+      "$PMTK010,002\r\n",    // Missing checksum.
+      PMTK_AWAKE "junk\r\n", // Extra trailer after a valid checksum.
+  };
+  for (uint8_t i = 0; i < sizeof(invalid) / sizeof(invalid[0]); i++) {
+    stream.input = invalid[i];
+    if (GPS.waitForSentence("$PMTK010", 1, false, 100) ||
+        strcmp(GPS.lastNMEA(), invalid[i])) {
+      Serial.println(F("FAIL: corrupt reply accepted or raw response lost"));
+      return;
+    }
+  }
+  char embeddedNul[] = PMTK_AWAKE "\0junk\r\n";
+  stream.input = embeddedNul;
+  stream.end = embeddedNul + sizeof(embeddedNul) - 1;
+  bool accepted = GPS.waitForSentence(PMTK_AWAKE, 1, false, 100);
+  stream.end = NULL;
+  if (accepted) {
+    Serial.println(F("FAIL: embedded NUL hid corrupt bytes after a valid prefix"));
+    return;
+  }
+  Serial.println(F("PASS: matching corrupt replies and embedded NUL rejected"));
+
+  stream.input = PMTK_AWAKE "junk\r\n" PMTK_AWAKE "\r\n";
+  if (GPS.waitForSentence(PMTK_AWAKE, 1, false, 100) ||
+      !GPS.waitForSentence(PMTK_AWAKE, 1, false, 100)) {
+    Serial.println(F("FAIL: invalid reply did not consume exactly one sentence"));
+    return;
+  }
+  stream.input = "$PMTK010,002*00\r\n" PMTK_AWAKE "\r\n";
+  if (!GPS.waitForSentence("$PMTK010", 2, false, 100) ||
+      strcmp(GPS.lastNMEA(), PMTK_AWAKE "\r\n")) {
+    Serial.println(F("FAIL: valid reply after a corrupt match was lost"));
+    return;
+  }
+  Serial.println(F("PASS: invalid replies count toward the limit and waits can continue"));
+
+  char replies[160];
+  const char *navigation = "GNGLL,4807.038,N,01131.000,E,123519,A";
+  size_t length = Adafruit_NMEA::buildCommand(replies, sizeof(replies),
+                                              navigation, strlen(navigation));
+  const char *reply = "PAIR001,062,0";
+  if (!length || !Adafruit_NMEA::buildCommand(replies + length,
+      sizeof(replies) - length, reply, strlen(reply))) {
+    Serial.println(F("FAIL: constructing mixed reply fixture"));
+    return;
+  }
+  stream.input = replies;
+  if (!GPS.waitForSentence("$PAIR001,062,", 2, false, 100) ||
+      strncmp(GPS.lastNMEA(), "$PAIR001,062,0*", 14)) {
+    Serial.println(F("FAIL: proprietary reply after navigation was not accepted"));
+    return;
+  }
+  stream.input = "$PQTMVERNO*58\n";
+  if (!GPS.waitForSentence("$PQTMVERNO", 1, false, 100)) {
+    Serial.println(F("FAIL: valid unknown zero-field response was rejected"));
+    return;
+  }
+  Serial.println(F("PASS: proprietary replies and zero-field messages need no GPS whitelist"));
+
+  // Simulate a complete line already published by an interrupt reader.
+  stream.input = PMTK_AWAKE "\r\n";
+  while (stream.available()) {
+    GPS.read();
+  }
+  if (!GPS.waitForSentence(PMTK_AWAKE, 1, true, 100)) {
+    Serial.println(F("FAIL: valid interrupt-published response was rejected"));
+    return;
+  }
+  stream.input = PMTK_AWAKE "junk\r\n";
+  while (stream.available()) {
+    GPS.read();
+  }
+  if (GPS.waitForSentence(PMTK_AWAKE, 1, true, 100)) {
+    Serial.println(F("FAIL: invalid interrupt-published response was accepted"));
+    return;
+  }
+  stream.input = PMTK_AWAKE "\r\n";
+  if (GPS.waitForSentence(NULL, 1, false, 100) ||
+      !GPS.waitForSentence(PMTK_AWAKE, 1, false, 100)) {
+    Serial.println(F("FAIL: NULL prefix consumed input or was accepted"));
+    return;
+  }
+  Serial.println(F("PASS: interrupt-published replies and NULL prefix handling"));
+
   GPS.standby();
   start = millis();
   Serial.println(F("Testing failed wakeup (10 seconds)..."));

src/Adafruit_GPS.cpp

@@ -442,11 +442,19 @@ gnss_position_t Adafruit_GPS::lastPosition() const {
  * the GPS (default false)
  * @param timeout Time limit in milliseconds (default
  * 10000)
- * @return True on a match, false when either limit is reached
+ * @return True on a valid matching frame, false on NULL prefix or either limit.
+ *
+ * Matching remains a text-prefix comparison, but the complete NMEA frame must
+ * have valid syntax and checksum. Unknown proprietary addresses are allowed;
+ * no GPS sentence whitelist or command-result interpretation is applied.
+ * Every completed line, including invalid and nonmatching lines, counts toward
+ * max. The accepted raw reply remains available through lastNMEA().
  */
 /**************************************************************************/
 bool Adafruit_GPS::waitForSentence(const char *wait4me, uint8_t max,
                                    bool usingInterrupts, uint32_t timeout) {
+  if (!wait4me)
+    return false;
   uint8_t i = 0;
   uint32_t start = millis();
   // A silent GPS, including an I2C device that NAKs, never advances i.
@@ -458,9 +466,17 @@ bool Adafruit_GPS::waitForSentence(const char *wait4me, uint8_t max,
       char *nmea = lastNMEA();
       i++;
 
-      if (strStartsWith(nmea, wait4me))
+      if (strStartsWith(nmea, wait4me)) {
+        // read() publishes a complete LF-terminated receive buffer. Bound the
+        // whole line, including any embedded NUL, so a valid-looking prefix
+        // cannot hide corrupt bytes from checksum/framing validation.
+        const char *end = (const char *)memchr(nmea, '\n', MAXLINELENGTH);
+        if (end &&
+            Adafruit_NMEA::validate(nmea, (size_t)(end - nmea) + 1).status ==
+                NMEA_FRAME_VALID)
           return true;
       }
+    }
     yield();
   }