Reject oversized USB frames and bound serial receive work

This commit is contained in:
mikecarper committed 2026-10-03 18:55:23 -07:00
1 parent d9088f9e86
commit ecdaacdd58
3 files changed
+387 -33

No files matched your search

+46 -23
View File
@@ -7,12 +7,12 @@
#define RECV_STATE_LEN2_FOUND 3
void ArduinoSerialInterface::resetControlSequenceState() {
_controlSequencePos = 0;
_secondaryControlSequencePos = 0;
_controlSequenceCandidate =
_controlSequence != nullptr && _controlSequence[0] != 0;
_secondaryControlSequenceCandidate =
_secondaryControlSequence != nullptr && _secondaryControlSequence[0] != 0;
_controlSequenceCursor =
_controlSequence != nullptr && _controlSequence[0] != 0
? _controlSequence : nullptr;
_secondaryControlSequenceCursor =
_secondaryControlSequence != nullptr && _secondaryControlSequence[0] != 0
? _secondaryControlSequence : nullptr;
}
void ArduinoSerialInterface::resetReceiveState() {
@@ -121,35 +121,39 @@ void ArduinoSerialInterface::serviceTransmit() {
bool ArduinoSerialInterface::checkControlLineByte(uint8_t c) {
if (c == '\r' || c == '\n') {
const bool primary = _controlSequenceCandidate
&& _controlSequence[_controlSequencePos] == 0;
const bool secondary = _secondaryControlSequenceCandidate
&& _secondaryControlSequence[_secondaryControlSequencePos] == 0;
const bool primary = _controlSequenceCursor != nullptr
&& *_controlSequenceCursor == 0;
const bool secondary = _secondaryControlSequenceCursor != nullptr
&& *_secondaryControlSequenceCursor == 0;
resetControlSequenceState();
if (primary) _controlSequenceReceived = true;
if (secondary) _secondaryControlSequenceReceived = true;
return primary || secondary;
}
if (_controlSequenceCandidate) {
if (_controlSequence[_controlSequencePos] != 0
&& c == (uint8_t)_controlSequence[_controlSequencePos]) {
++_controlSequencePos;
if (_controlSequenceCursor != nullptr) {
if (*_controlSequenceCursor != 0
&& c == (uint8_t)*_controlSequenceCursor) {
++_controlSequenceCursor;
} else {
_controlSequenceCandidate = false;
_controlSequenceCursor = nullptr;
}
}
if (_secondaryControlSequenceCandidate) {
if (_secondaryControlSequence[_secondaryControlSequencePos] != 0
&& c == (uint8_t)_secondaryControlSequence[_secondaryControlSequencePos]) {
++_secondaryControlSequencePos;
if (_secondaryControlSequenceCursor != nullptr) {
if (*_secondaryControlSequenceCursor != 0
&& c == (uint8_t)*_secondaryControlSequenceCursor) {
++_secondaryControlSequenceCursor;
} else {
_secondaryControlSequenceCandidate = false;
_secondaryControlSequenceCursor = nullptr;
}
}
return false;
}
#if defined(STM32_PLATFORM) && defined(__GNUC__) && !defined(__clang__)
// Keep repeated host-ownership resets shared on flash-constrained STM32.
__attribute__((noinline, noclone))
#endif
void ArduinoSerialInterface::setPassthroughMode(bool enabled) {
_passthroughMode = enabled;
_controlSequenceReceived = false;
@@ -158,6 +162,9 @@ void ArduinoSerialInterface::setPassthroughMode(bool enabled) {
resetTransmitState();
}
#if defined(STM32_PLATFORM) && defined(__GNUC__) && !defined(__clang__)
__attribute__((noinline, noclone))
#endif
void ArduinoSerialInterface::resetSessionState() {
_controlSequenceReceived = false;
_secondaryControlSequenceReceived = false;
@@ -250,7 +257,10 @@ size_t ArduinoSerialInterface::checkRecvFrame(uint8_t dest[]) {
serviceTransmit();
if (_passthroughMode) return 0;
while (_serial->available()) {
// Newly arriving bytes belong to the next mesh loop; a continuously writing
// host must not keep the radio/watchdog loop trapped in this parser.
int pending = _serial->available();
while (pending-- > 0) {
int c = _serial->read();
if (c < 0) break;
_last_rx_byte_ms = millis();
@@ -281,11 +291,24 @@ size_t ArduinoSerialInterface::checkRecvFrame(uint8_t dest[]) {
}
rx_len++;
if (rx_len >= _frame_len) { // received a complete frame?
if (_frame_len > MAX_FRAME_SIZE) _frame_len = MAX_FRAME_SIZE; // truncate
if (_frame_len > MAX_FRAME_SIZE) {
// Consume the entire declared body to preserve framing, but never
// execute a truncated command or grant Binary-client proof for an
// invalid frame. A following valid frame may still be dispatched.
resetReceiveState();
break;
}
// The final read already captured its completion time above.
const uint32_t completed_at = _last_rx_byte_ms;
if (_receive_frame_check != nullptr
&& !_receive_frame_check(completed_at)) {
resetReceiveState();
return 0;
}
memcpy(dest, rx_buf, _frame_len);
_state = RECV_STATE_IDLE; // reset state, for next frame
resetControlSequenceState();
_last_frame_ms = millis(); // a real client is talking to us
_last_frame_ms = completed_at; // a real client is talking to us
++_completed_frame_count;
_has_received_frame = true;
return _frame_len;
+16 -10
View File
@@ -7,6 +7,9 @@ class ArduinoSerialInterface : public BaseSerialInterface {
public:
// reports whether a client currently has this stream open (see setConnectedCheck)
typedef bool (*ConnectedCheck)();
// Optional protocol-owner gate, checked before a completed frame is exposed
// or recorded as client activity. Other transports retain their old behavior.
typedef bool (*ReceiveFrameCheck)(uint32_t completed_at);
private:
static constexpr uint8_t TX_QUEUE_SIZE = 4;
@@ -23,10 +26,13 @@ private:
bool _secondaryControlSequenceReceived;
bool _flow_ctl;
uint8_t _state;
size_t _controlSequencePos;
size_t _secondaryControlSequencePos;
bool _controlSequenceCandidate;
bool _secondaryControlSequenceCandidate;
uint8_t _tx_queue_len;
uint16_t _tx_offset;
// A null cursor excludes the current line; otherwise it points at the next
// token byte. This keeps arbitrary-length C-string tokens without separate
// candidate flags or a position limit.
const char* _controlSequenceCursor;
const char* _secondaryControlSequenceCursor;
uint16_t _frame_len;
uint16_t rx_len;
uint32_t _last_frame_ms;
@@ -37,10 +43,9 @@ private:
const char* _controlSequence;
const char* _secondaryControlSequence;
ConnectedCheck _conn_check;
ReceiveFrameCheck _receive_frame_check;
uint8_t rx_buf[MAX_FRAME_SIZE];
TxFrame _tx_queue[TX_QUEUE_SIZE];
uint8_t _tx_queue_len;
uint16_t _tx_offset;
void resetControlSequenceState();
bool checkControlLineByte(uint8_t c);
@@ -54,14 +59,14 @@ public:
ArduinoSerialInterface()
: _isEnabled(false), _passthroughMode(false),
_controlSequenceReceived(false), _secondaryControlSequenceReceived(false),
_flow_ctl(false), _state(0), _controlSequencePos(0),
_secondaryControlSequencePos(0), _controlSequenceCandidate(false),
_secondaryControlSequenceCandidate(false), _frame_len(0), rx_len(0),
_flow_ctl(false), _state(0), _tx_queue_len(0), _tx_offset(0),
_controlSequenceCursor(nullptr), _secondaryControlSequenceCursor(nullptr),
_frame_len(0), rx_len(0),
_last_frame_ms(0), _completed_frame_count(0), _last_rx_byte_ms(0),
_has_received_frame(false),
_serial(nullptr), _controlSequence(nullptr),
_secondaryControlSequence(nullptr), _conn_check(nullptr),
_tx_queue_len(0), _tx_offset(0) {}
_receive_frame_check(nullptr) {}
void begin(Stream& serial, const char* controlSequence = nullptr,
const char* secondaryControlSequence = nullptr) {
@@ -94,6 +99,7 @@ public:
// Optional: let the target report the real link state, e.g. USB-CDC DTR.
// Without it isConnected() assumes true, as a plain UART has no way of knowing.
void setConnectedCheck(ConnectedCheck fn) { _conn_check = fn; }
void setReceiveFrameCheck(ReceiveFrameCheck fn) { _receive_frame_check = fn; }
// Clear all protocol state owned by one host session while retaining the
// monotonic completed-frame counter used by startup handoff arbitration.
@@ -3,6 +3,7 @@
#include <algorithm>
#include <deque>
#include <limits>
#include <string>
#include <vector>
#include "helpers/ArduinoSerialInterface.h"
@@ -716,6 +717,109 @@ TEST(SerialModeSwitch, NewlineStartsAFreshControlLine) {
EXPECT_TRUE(interface.takeControlSequence());
}
TEST(SerialModeSwitch, EmptyNullAndIndependentControlConfigurations) {
uint8_t frame[MAX_FRAME_SIZE] = {};
for (const char* primary : {(const char*)nullptr, ""}) {
for (const char* secondary : {(const char*)nullptr, ""}) {
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, primary, secondary);
stream.push("\r\nwrong\r\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
}
}
for (bool secondary : {false, true}) {
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, secondary ? nullptr : "token",
secondary ? "token" : nullptr);
stream.push("token\r\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_EQ(interface.takeControlSequence(), !secondary);
EXPECT_EQ(interface.takeSecondaryControlSequence(), secondary);
}
}
TEST(SerialModeSwitch, IdenticalAndPrefixControlTokensRemainIndependent) {
uint8_t frame[MAX_FRAME_SIZE] = {};
for (bool identical : {false, true}) {
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, "abc", identical ? "abc" : "abcd");
stream.push("ab");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
stream.push("c\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.takeControlSequence());
EXPECT_EQ(interface.takeSecondaryControlSequence(), identical);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
stream.push("abcd\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_EQ(interface.takeSecondaryControlSequence(), !identical);
}
}
TEST(SerialModeSwitch, ArbitraryLengthControlTokensAndHighBitBytesArePreserved) {
uint8_t frame[MAX_FRAME_SIZE] = {};
// The public token API is not limited by an 8-bit or 16-bit position.
std::string token(70000, 'x');
token.back() = 'z';
const char high_bit[] = {'a', (char)0x80, (char)0xFF, 0};
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, token.c_str(), high_bit);
for (size_t offset = 0; offset < token.size(); offset += 113) {
stream.push((const uint8_t*)token.data() + offset,
std::min((size_t)113, token.size() - offset));
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
}
stream.push("\r");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
const uint8_t high_bit_line[] = {'a', 0x80, 0xFF, '\n'};
stream.push(high_bit_line, sizeof(high_bit_line));
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_TRUE(interface.takeSecondaryControlSequence());
}
TEST(SerialModeSwitch, ResetAndMutableControlStorageRemainCompatible) {
uint8_t frame[MAX_FRAME_SIZE] = {};
char token[] = "ab";
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, token);
stream.push("a");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
token[1] = 'c';
stream.push("c\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.takeControlSequence());
token[0] = 0;
interface.resetSessionState();
stream.push("\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_FALSE(interface.takeControlSequence());
token[0] = 'a';
interface.setPassthroughMode(false);
stream.push("a");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
interface.begin(stream, "replacement", "second");
stream.push("c\nreplacement\n");
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
}
TEST(SerialModeSwitch, PassthroughLeavesInputAndSuppressesBinaryOutput) {
BufferStream stream;
ArduinoSerialInterface interface;
@@ -770,6 +874,227 @@ TEST(SerialModeSwitch, AsciiStartupProbeRequiresAnEmptyPrompt) {
EXPECT_FALSE(probe.shouldStart(true, false, -1));
}
TEST(SerialModeSwitch, OversizedFramesDoNotDispatchOrConfirmBinaryMode) {
for (uint16_t declared : {uint16_t(MAX_FRAME_SIZE + 1), uint16_t(65535)}) {
SCOPED_TRACE(declared);
resetArduinoMock();
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, START_TOKEN, SEEDER_TOKEN);
mesh::UsbBinaryStartupProbe probe;
probe.start(millis(), interface.getCompletedFrameCount());
uint8_t frame[MAX_FRAME_SIZE];
std::fill(frame, frame + sizeof(frame), 0xCC);
const uint8_t header[] = {'<', uint8_t(declared), uint8_t(declared >> 8)};
stream.push(header, sizeof(header));
std::vector<uint8_t> body(declared, 0x16); // A plausible command prefix.
const char embedded[] = "\r\n+++MESHCORE-TERM-START\r\nota folder on\r\n";
std::copy(embedded, embedded + sizeof(embedded) - 1, body.begin() + 8);
stream.push(body.data(), body.size());
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(std::all_of(frame, frame + sizeof(frame),
[](uint8_t byte) { return byte == 0xCC; }));
EXPECT_FALSE(interface.isReadBusy());
EXPECT_FALSE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 0u);
EXPECT_EQ(interface.getLastFrameMillis(), 0u);
EXPECT_FALSE(interface.takeControlSequence());
EXPECT_FALSE(interface.takeSecondaryControlSequence());
EXPECT_EQ(probe.poll(millis(), interface.getCompletedFrameCount(),
interface.getLastFrameMillis()),
mesh::UsbBinaryStartupProbe::Result::WAITING);
delay(1000);
EXPECT_EQ(probe.poll(millis(), interface.getCompletedFrameCount(),
interface.getLastFrameMillis()),
mesh::UsbBinaryStartupProbe::Result::RETURN_TO_ASCII);
}
}
TEST(SerialModeSwitch, FragmentedOversizedBodyKeepsFollowingFrameSynchronized) {
for (uint16_t declared : {uint16_t(MAX_FRAME_SIZE + 1), uint16_t(65535)}) {
SCOPED_TRACE(declared);
resetArduinoMock();
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream, START_TOKEN);
mesh::UsbBinaryStartupProbe probe;
probe.start(millis(), interface.getCompletedFrameCount());
uint8_t frame[MAX_FRAME_SIZE] = {};
const uint8_t header[] = {'<', uint8_t(declared), uint8_t(declared >> 8)};
for (uint8_t byte : header) {
stream.push(&byte, 1);
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.isReadBusy());
delay(1);
}
const std::vector<uint8_t> body(declared, '<');
for (size_t offset = 0; offset < body.size();) {
const size_t chunk = std::min(size_t(2048), body.size() - offset);
stream.push(body.data() + offset, chunk);
offset += chunk;
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_EQ(interface.isReadBusy(), offset < body.size());
EXPECT_FALSE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 0u);
delay(1);
}
const uint8_t valid[] = {'<', 2, 0, 0x16, 0x03};
stream.push(valid, sizeof(valid));
ASSERT_EQ(interface.checkRecvFrame(frame), 2u);
EXPECT_EQ(frame[0], 0x16);
EXPECT_EQ(frame[1], 0x03);
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
EXPECT_EQ(interface.getLastFrameMillis(), millis());
EXPECT_TRUE(interface.hasReceivedFrame());
EXPECT_EQ(probe.poll(millis(), interface.getCompletedFrameCount(),
interface.getLastFrameMillis()),
mesh::UsbBinaryStartupProbe::Result::BINARY_CONFIRMED);
}
}
TEST(SerialModeSwitch, ValidFrameImmediatelyAfterOversizedBodyDispatchesAlone) {
for (uint16_t declared : {uint16_t(MAX_FRAME_SIZE + 1), uint16_t(65535)}) {
SCOPED_TRACE(declared);
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream);
const uint8_t header[] = {'<', uint8_t(declared), uint8_t(declared >> 8)};
stream.push(header, sizeof(header));
const std::vector<uint8_t> body(declared, 0x16);
stream.push(body.data(), body.size());
const uint8_t valid[] = {'<', 1, 0, 0xA5};
stream.push(valid, sizeof(valid));
uint8_t frame[MAX_FRAME_SIZE] = {};
ASSERT_EQ(interface.checkRecvFrame(frame), 1u);
EXPECT_EQ(frame[0], 0xA5);
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
EXPECT_EQ(stream.available(), 0);
}
}
TEST(SerialModeSwitch, MaximumValidFrameStillDispatchesWithoutTruncation) {
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream);
const uint8_t header[] = {'<', MAX_FRAME_SIZE, 0};
stream.push(header, sizeof(header));
std::vector<uint8_t> body(MAX_FRAME_SIZE, 0xA5);
stream.push(body.data(), body.size());
uint8_t frame[MAX_FRAME_SIZE] = {};
EXPECT_EQ(interface.checkRecvFrame(frame), size_t(MAX_FRAME_SIZE));
EXPECT_TRUE(std::equal(body.begin(), body.end(), frame));
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
}
static uint32_t rejected_completion_at = 0;
static bool rejectCompletedFrame(uint32_t completed_at) {
rejected_completion_at = completed_at;
return false;
}
TEST(SerialModeSwitch, OptionalCompletionGateRejectsBeforeDispatchAndProof) {
resetArduinoMock();
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream);
interface.setReceiveFrameCheck(rejectCompletedFrame);
g_mock_millis = 1234;
const uint8_t valid[] = {'<', 1, 0, 0xA5};
stream.push(valid, sizeof(valid));
uint8_t frame[MAX_FRAME_SIZE];
std::fill(frame, frame + sizeof(frame), 0xCC);
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_EQ(rejected_completion_at, 1234u);
EXPECT_TRUE(std::all_of(frame, frame + sizeof(frame),
[](uint8_t byte) { return byte == 0xCC; }));
EXPECT_FALSE(interface.isReadBusy());
EXPECT_FALSE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 0u);
EXPECT_EQ(interface.getLastFrameMillis(), 0u);
// Clearing the optional callback restores ordinary transport behavior.
interface.setReceiveFrameCheck(nullptr);
stream.push(valid, sizeof(valid));
EXPECT_EQ(interface.checkRecvFrame(frame), 1u);
EXPECT_EQ(frame[0], 0xA5);
EXPECT_TRUE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
EXPECT_EQ(interface.getLastFrameMillis(), 1234u);
interface.setReceiveFrameCheck(rejectCompletedFrame);
++g_mock_millis;
stream.push(valid, sizeof(valid));
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_TRUE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
EXPECT_EQ(interface.getLastFrameMillis(), 1234u);
}
static unsigned completion_checks = 0;
static bool rejectFirstCompletedFrame(uint32_t) {
return ++completion_checks > 1;
}
TEST(SerialModeSwitch, RejectedFrameLeavesFollowingAllowedFrameForNextRead) {
resetArduinoMock();
completion_checks = 0;
BufferStream stream;
ArduinoSerialInterface interface;
interface.begin(stream);
interface.setReceiveFrameCheck(rejectFirstCompletedFrame);
const uint8_t frames[] = {'<', 1, 0, 0xA5, '<', 1, 0, 0xB6};
stream.push(frames, sizeof(frames));
uint8_t frame[MAX_FRAME_SIZE] = {};
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_EQ(completion_checks, 1u);
EXPECT_EQ(interface.getCompletedFrameCount(), 0u);
EXPECT_EQ(interface.checkRecvFrame(frame), 1u);
EXPECT_EQ(frame[0], 0xB6);
EXPECT_EQ(completion_checks, 2u);
EXPECT_EQ(interface.getCompletedFrameCount(), 1u);
EXPECT_EQ(stream.available(), 0);
}
TEST(SerialModeSwitch, ContinuouslyRefillingIdleInputYieldsToTheMeshLoop) {
class RefillingStream : public BufferStream {
public:
unsigned read_count = 0;
unsigned refill_remaining = 512;
int read() override {
const int value = BufferStream::read();
if (value >= 0) {
++read_count;
if (refill_remaining) {
--refill_remaining;
input.push_back('x');
}
}
return value;
}
} stream;
ArduinoSerialInterface interface;
interface.begin(stream, START_TOKEN);
stream.push("xxxx");
uint8_t frame[MAX_FRAME_SIZE] = {};
for (unsigned pass = 0; pass < 16; ++pass) {
const unsigned before = stream.read_count;
EXPECT_EQ(interface.checkRecvFrame(frame), 0u);
EXPECT_EQ(stream.read_count, before + 4);
EXPECT_EQ(stream.available(), 4);
EXPECT_FALSE(interface.hasReceivedFrame());
EXPECT_EQ(interface.getCompletedFrameCount(), 0u);
}
stream.refill_remaining = 0;
stream.push("\r\n");
const uint8_t valid[] = {'<', 1, 0, 0xA5};
stream.push(valid, sizeof(valid));
ASSERT_EQ(interface.checkRecvFrame(frame), 1u);
EXPECT_EQ(frame[0], 0xA5);
}
TEST(SerialModeSwitch, IncompleteBinaryProbeReturnsToAsciiAfterTimeout) {
mesh::UsbBinaryStartupProbe probe;
probe.start(0xFFFFFFF0u, 7);