From 695ee635310a36208d3b34f2c84f58baa8be81d5 Mon Sep 17 00:00:00 2001 From: Rijesh Augustine Date: Fri, 3 Jul 2026 08:44:30 -0600 Subject: [PATCH 1/6] Initial test improvements --- .../content/docs/reference/test-coverage.md | 13 +- .../boilerplate/c/frame_profiles.h | 57 ++- .../boilerplate/cpp/frame_base.hpp | 14 +- .../boilerplate/cpp/frame_profiles.hpp | 72 ++- .../cpp/struct_frame_sdk/serial_transport.hpp | 2 + .../cpp/struct_frame_sdk/struct_frame_sdk.hpp | 9 +- .../Framework/Framing/AccumulatingReader.cs | 79 ++- .../csharp/Framework/Framing/BufferReader.cs | 8 +- .../csharp/Framework/Types/MessageInfo.cs | 13 + .../boilerplate/js/frame-profiles.js | 123 ++++- .../js/struct-frame-sdk/struct-frame-sdk.js | 8 +- .../boilerplate/py/frame_profiles.py | 110 ++++- .../boilerplate/rust/frame_profiles.rs | 128 +++-- .../boilerplate/ts/frame-profiles.ts | 121 ++++- .../ts/struct-frame-sdk/struct-frame-sdk.ts | 8 +- src/struct_frame/cpp_gen.py | 11 +- src/struct_frame/csharp_gen.py | 7 +- src/struct_frame/js_gen.py | 4 +- src/struct_frame/py_gen.py | 6 +- src/struct_frame/ts_gen.py | 4 +- tests/NEGATIVE_TESTS.md | 37 +- .../api_stability/nuget/PublicAPI.Shipped.txt | 2 + tests/c/test_negative.c | 312 ++++++++++++ tests/coverage_spec.py | 27 +- tests/cpp/test_negative.cpp | 280 +++++++++++ tests/cpp/test_sdk_headers_compile.cpp | 23 + tests/csharp/TestNegative.cs | 271 ++++++++++ tests/js/test_negative.js | 240 +++++++++ tests/py/test_negative.py | 147 ++++++ tests/run_tests.py | 7 +- tests/rust/src/main.rs | 4 +- tests/rust/src/test_negative.rs | 464 ++++++++++++++++-- tests/ts/test_negative.ts | 240 +++++++++ tests/ts/test_request_response_sdk.ts | 37 ++ 34 files changed, 2703 insertions(+), 185 deletions(-) create mode 100644 tests/cpp/test_sdk_headers_compile.cpp diff --git a/docs/src/content/docs/reference/test-coverage.md b/docs/src/content/docs/reference/test-coverage.md index cff6aa1f..6656b649 100644 --- a/docs/src/content/docs/reference/test-coverage.md +++ b/docs/src/content/docs/reference/test-coverage.md @@ -226,26 +226,37 @@ Test files: `tests/{c,cpp,py,ts,js,csharp,rust}/test_negative.*` See `tests/NEGATIVE_TESTS.md` for full scenario descriptions. -The 20 scenarios in the table below are registered in every language's `test_negative.*` file. Individual languages carry additional language-specific scenarios: C/C++/TS/JS/C# (24 each) add bulk `pkg_id`/`msg_id` corruption, cross-package rejection, and network `pkg_id` corruption; Python (34) adds those plus diagnostic-counter and status-machine tests; Rust (20) currently matches the uniform set. +The 31 scenarios in the table below are registered in every language's `test_negative.*` file, covering corruption handling, the `tryNext` drain contract, diagnostic counters (unified semantics in buffer and stream mode), minimal-profile resync, and a chunk-boundary split sweep. All seven languages additionally carry the four package-corruption scenarios (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) for 35 scenarios each; Python (40) adds status-machine and buffer-mode diagnostic extras. | Error Scenario (test name) | C | C++ | Python | TS | JS | C# | Rust | |--------|--------|--------|--------|--------|--------|--------|--------| +| Buffer mode: CRC failure counters | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Buffer mode: Sequence gap counted | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Buffer mode: recovers after CRC failure | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Buffer reader: skips CRC-failed frame | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Bulk profile: Corrupted CRC | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Corrupted CRC detection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Corrupted length field detection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Diagnostics: CRC failure counter | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Diagnostics: Length error counter | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Diagnostics: Reset diagnostics | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Diagnostics: Sequence gap counter | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Diagnostics: Sync recovery counter | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Invalid message ID rejection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Invalid start bytes detection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| IPC buffer: unknown msg_id advances one byte | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Minimal profile: Truncated frame | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Multiple frames: CRC error then valid frame | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Multiple frames: Corrupted middle frame | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Network profile: SysId/CompId corruption | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Partial frame across buffer boundary | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Sensor buffer: unknown msg_id resync | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Split sweep: two frames at every boundary | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Split-buffer: CRC error status preserved | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Stream mode: recovers after garbage prefix | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Streaming: Corrupted CRC detection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Streaming: Garbage data handling | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Streaming: two frames byte-by-byte | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | TryNext drain: CRC/resync + valid | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | TryNext partial pending contract | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Truncated frame detection | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | diff --git a/src/struct_frame/boilerplate/c/frame_profiles.h b/src/struct_frame/boilerplate/c/frame_profiles.h index e05fc5ce..6505a15b 100644 --- a/src/struct_frame/boilerplate/c/frame_profiles.h +++ b/src/struct_frame/boilerplate/c/frame_profiles.h @@ -535,7 +535,9 @@ static inline frame_msg_info_t buffer_reader_next(buffer_reader_t* reader) reader->offset = reader->size; } } else { - reader->offset = reader->size; + /* No start bytes (e.g. IPC profile): advance one byte and retry so a + * single unknown msg_id doesn't discard the rest of the buffer. */ + reader->offset += 1; } result.status = FRAME_MSG_STATUS_SYNC_RECOVERY; result.frame_size = reader->offset - old_offset; @@ -739,6 +741,49 @@ static inline frame_msg_info_t _acc_parse_buffer( const uint8_t* buffer, size_t size); +/* Record per-frame diagnostics for a complete frame consumed in buffer mode, + * matching the stream-mode (push_byte) counter semantics: + * - cnt_len_errors when the header length is outside [min_size, size] + * - cnt_seq_gaps on valid frames for profiles that carry a sequence number + * frame_start points at the first byte of the frame (start bytes included). */ +static inline void _acc_record_frame_diagnostics( + accumulating_reader_t* reader, const uint8_t* frame_start, bool valid) +{ + const profile_config_t* config = reader->config; + if (config->payload.has_length && reader->get_message_info) { + size_t len_offset = config->header.num_start_bytes; + if (config->payload.has_seq) len_offset++; + if (config->payload.has_sys_id) len_offset++; + if (config->payload.has_comp_id) len_offset++; + size_t payload_len; + if (config->payload.length_bytes == 1) { + payload_len = frame_start[len_offset]; + } else { + payload_len = frame_start[len_offset] | ((size_t)frame_start[len_offset + 1] << 8); + } + uint8_t header_size = profile_header_size(config); + uint16_t full_msg_id = 0; + if (config->payload.has_pkg_id) { + full_msg_id = (uint16_t)frame_start[header_size - 2] << 8; + } + full_msg_id |= frame_start[header_size - 1]; + message_info_t info; + if (reader->get_message_info(full_msg_id, &info) && + (payload_len > info.size || payload_len < info.min_size)) { + reader->diagnostics.cnt_len_errors++; + } + } + if (valid && config->payload.has_seq) { + uint8_t seq = frame_start[config->header.num_start_bytes]; + if (reader->last_seq_valid) { + uint8_t expected_seq = (uint8_t)(reader->last_seq + 1); + if (seq != expected_seq) reader->diagnostics.cnt_seq_gaps++; + } + reader->last_seq = seq; + reader->last_seq_valid = true; + } +} + static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* reader) { frame_msg_info_t result = {false, 0, 0, NULL, FRAME_MSG_STATUS_NONE, 0, NULL}; @@ -763,6 +808,7 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r result = _acc_parse_buffer(reader, reader->internal_buffer, reader->internal_data_len); if (result.valid) { + _acc_record_frame_diagnostics(reader, reader->internal_buffer, true); size_t bytes_from_current = result.frame_size > partial_len ? result.frame_size - partial_len : 0; reader->current_offset = bytes_from_current; reader->internal_data_len = 0; @@ -775,6 +821,8 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r /* Complete frame but CRC failed — count it and skip */ reader->diagnostics.cnt_crc_failures++; reader->diagnostics.cnt_failed_bytes += (uint32_t)result.frame_size; + reader->diagnostics.cnt_sync_recoveries++; + _acc_record_frame_diagnostics(reader, reader->internal_buffer, false); size_t bytes_from_current = result.frame_size > partial_len ? result.frame_size - partial_len : 0; reader->current_offset = bytes_from_current; reader->internal_data_len = 0; @@ -797,6 +845,7 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r result = _acc_parse_buffer(reader, cur, cur_remaining); if (result.valid && result.frame_size > 0) { + _acc_record_frame_diagnostics(reader, cur, true); reader->current_offset += result.frame_size; return result; } @@ -805,6 +854,8 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r /* Complete frame with bad CRC — count it, skip it */ reader->diagnostics.cnt_crc_failures++; reader->diagnostics.cnt_failed_bytes += (uint32_t)result.frame_size; + reader->diagnostics.cnt_sync_recoveries++; + _acc_record_frame_diagnostics(reader, cur, false); reader->current_offset += result.frame_size; return result; } @@ -826,7 +877,9 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r reader->current_offset = reader->current_size; } } else { - reader->current_offset = reader->current_size; + /* No start bytes (e.g. IPC profile): advance one byte and retry so a + * single unknown msg_id doesn't discard the rest of the buffer. */ + reader->current_offset += 1; } result.status = FRAME_MSG_STATUS_SYNC_RECOVERY; result.frame_size = reader->current_offset - old_current_offset; diff --git a/src/struct_frame/boilerplate/cpp/frame_base.hpp b/src/struct_frame/boilerplate/cpp/frame_base.hpp index 311be396..ffd133a1 100644 --- a/src/struct_frame/boilerplate/cpp/frame_base.hpp +++ b/src/struct_frame/boilerplate/cpp/frame_base.hpp @@ -16,11 +16,15 @@ struct MessageInfo { uint8_t magic2; bool valid; // True if message info is valid size_t base_size; // Non-extension portion size (== size when no extensions) - - MessageInfo() : size(0), magic1(0), magic2(0), valid(false), base_size(0) {} - MessageInfo(size_t s, uint8_t m1, uint8_t m2) : size(s), magic1(m1), magic2(m2), valid(true), base_size(s) {} - MessageInfo(size_t s, uint8_t m1, uint8_t m2, size_t bs) : size(s), magic1(m1), magic2(m2), valid(true), base_size(bs) {} - + size_t min_size; // Minimum valid payload size: base_size for fixed messages, + // MIN_SIZE for variable messages + + MessageInfo() : size(0), magic1(0), magic2(0), valid(false), base_size(0), min_size(0) {} + MessageInfo(size_t s, uint8_t m1, uint8_t m2) : size(s), magic1(m1), magic2(m2), valid(true), base_size(s), min_size(s) {} + MessageInfo(size_t s, uint8_t m1, uint8_t m2, size_t bs) : size(s), magic1(m1), magic2(m2), valid(true), base_size(bs), min_size(bs) {} + MessageInfo(size_t s, uint8_t m1, uint8_t m2, size_t bs, size_t ms) + : size(s), magic1(m1), magic2(m2), valid(true), base_size(bs), min_size(ms) {} + explicit operator bool() const { return valid; } }; diff --git a/src/struct_frame/boilerplate/cpp/frame_profiles.hpp b/src/struct_frame/boilerplate/cpp/frame_profiles.hpp index 7211a14e..260e4187 100644 --- a/src/struct_frame/boilerplate/cpp/frame_profiles.hpp +++ b/src/struct_frame/boilerplate/cpp/frame_profiles.hpp @@ -409,8 +409,11 @@ class BufferParserMinimal { if (!is_null_callback(get_message_info)) { auto info = get_message_info(msg_id); if (!info) { + // Unknown message ID — this position is not a frame start. Report + // WaitingForStart (matching the C parser) so readers resync by + // scanning forward instead of stalling. FrameMsgInfo r; - r.status = FrameMsgStatus::SyncRecovery; + r.status = FrameMsgStatus::WaitingForStart; return r; } size_t msg_len = info.size; @@ -425,9 +428,9 @@ class BufferParserMinimal { } } - // No callback — cannot determine message length; report sync recovery + // No callback — cannot determine message length; report WaitingForStart FrameMsgInfo r; - r.status = FrameMsgStatus::SyncRecovery; + r.status = FrameMsgStatus::WaitingForStart; return r; } }; @@ -519,7 +522,9 @@ class BufferReader { offset_ = size_; } } else { - offset_ = size_; + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + offset_ += 1; } result.status = FrameMsgStatus::SyncRecovery; result.frame_size = offset_ - old_offset; @@ -818,6 +823,7 @@ class AccumulatingReader { FrameMsgInfo result = parse_frame(internal_buffer_, internal_data_len_); if (result.valid) { + record_frame_diagnostics(internal_buffer_, true); // How many bytes from the *current* buffer were consumed to complete this frame size_t bytes_from_current = result.frame_size > partial_len ? result.frame_size - partial_len : 0; current_offset_ = bytes_from_current; @@ -831,6 +837,8 @@ class AccumulatingReader { // Complete but invalid frame (CRC failure) — count it and skip diagnostics_.cnt_crc_failures++; diagnostics_.cnt_failed_bytes += static_cast(result.frame_size); + diagnostics_.cnt_sync_recoveries++; + record_frame_diagnostics(internal_buffer_, false); size_t bytes_from_current = result.frame_size > partial_len ? result.frame_size - partial_len : 0; current_offset_ = bytes_from_current; internal_data_len_ = 0; @@ -851,6 +859,7 @@ class AccumulatingReader { FrameMsgInfo result = parse_frame(current_buffer_ + current_offset_, current_size_ - current_offset_); if (result.valid && result.frame_size > 0) { + record_frame_diagnostics(current_buffer_ + current_offset_, true); current_offset_ += result.frame_size; return with_diagnostics(result); } @@ -859,6 +868,8 @@ class AccumulatingReader { // Complete frame with bad CRC — count it, skip it, let caller call next() again diagnostics_.cnt_crc_failures++; diagnostics_.cnt_failed_bytes += static_cast(result.frame_size); + diagnostics_.cnt_sync_recoveries++; + record_frame_diagnostics(current_buffer_ + current_offset_, false); current_offset_ += result.frame_size; return with_diagnostics(result); } @@ -880,7 +891,9 @@ class AccumulatingReader { current_offset_ = current_size_; } } else { - current_offset_ = current_size_; + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + current_offset_ += 1; } FrameMsgInfo r; r.status = FrameMsgStatus::SyncRecovery; @@ -1173,7 +1186,9 @@ class AccumulatingReader { } full_msg_id |= internal_buffer_[Config::header_size - 1]; auto info = get_message_info_(full_msg_id); - if (info && (payload_len > info.size || payload_len < info.base_size)) { + // Valid frames may carry anywhere from min_size (variable messages / + // truncated extensions) up to size bytes — only count outside that range. + if (info && (payload_len > info.size || payload_len < info.min_size)) { diagnostics_.cnt_len_errors++; } } @@ -1320,6 +1335,51 @@ class AccumulatingReader { return result; } + /** + * Record per-frame diagnostics for a complete frame consumed in buffer mode, + * matching the stream-mode (push_byte) counter semantics: + * - cnt_len_errors when the header length is outside [min_size, size] + * - cnt_seq_gaps on valid frames for profiles that carry a sequence number + * frame_start points at the first byte of the frame (start bytes included). + */ + void record_frame_diagnostics(const uint8_t* frame_start, bool valid) { + if constexpr (Config::has_length) { + if (get_message_info_) { + size_t len_offset = Config::num_start_bytes; + if constexpr (Config::has_seq) len_offset++; + if constexpr (Config::has_sys_id) len_offset++; + if constexpr (Config::has_comp_id) len_offset++; + size_t payload_len; + if constexpr (Config::length_bytes == 1) { + payload_len = frame_start[len_offset]; + } else { + payload_len = frame_start[len_offset] | (static_cast(frame_start[len_offset + 1]) << 8); + } + uint16_t full_msg_id = 0; + if constexpr (Config::has_pkg_id) { + full_msg_id = static_cast(frame_start[Config::header_size - 2]) << 8; + } + full_msg_id |= frame_start[Config::header_size - 1]; + auto info = get_message_info_(full_msg_id); + if (info && (payload_len > info.size || payload_len < info.min_size)) { + diagnostics_.cnt_len_errors++; + } + } + } + if (valid) { + if constexpr (Config::has_seq) { + uint8_t seq = frame_start[Config::num_start_bytes]; + if (last_seq_valid_) { + if (seq != static_cast(last_seq_ + 1)) { + diagnostics_.cnt_seq_gaps++; + } + } + last_seq_ = seq; + last_seq_valid_ = true; + } + } + } + /*========================================================================= * Shared Parsing *=========================================================================*/ diff --git a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/serial_transport.hpp b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/serial_transport.hpp index 0c69227b..3116ac14 100644 --- a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/serial_transport.hpp +++ b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/serial_transport.hpp @@ -88,6 +88,7 @@ class SerialTransport : public BaseTransport { } void Connect() { + if (!serialPort_) { HandleError("Serial port not initialized"); return; } @@ -109,6 +110,7 @@ class SerialTransport : public BaseTransport { } size_t Send(const uint8_t* data, size_t length) { + if (!serialPort_ || !connected_ || !serialPort_->isOpen()) { HandleError("Serial port not connected"); return 0; } diff --git a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp index f3943891..f0c189ff 100644 --- a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp +++ b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp @@ -15,6 +15,9 @@ #include #include +// Parser/encoder infrastructure (FrameMsgInfo, MessageInfo, FrameEncoder*). +// Included here so the SDK umbrella headers are self-contained. +#include "../frame_profiles.hpp" #include "transport.hpp" namespace structframe { @@ -595,9 +598,9 @@ class StructFrameSdkT : public IStructFrameSdk { offset = buffer_len_; } } else { - // No start bytes in this profile — cannot resync; discard all - offset = buffer_len_; - break; + // No start bytes in this profile (e.g. IPC): advance one byte and + // retry so a single unknown msg_id doesn't discard the rest. + offset += 1; } } else { // Collecting — frame is incomplete; need more data from next Feed() diff --git a/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs b/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs index 82d9ba64..112cb94c 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs @@ -173,7 +173,9 @@ public FrameMsgInfo Next() int found = (searchLen > 0 && _config.NumStartBytes > 0) ? Array.IndexOf(_internalBuffer, _config.ComputedStartByte1, 1, searchLen) : -1; - int discard = found > 0 ? found : _internalDataLen; + // No start bytes (e.g. IPC profile): discard one byte at a time so a + // single unknown msg_id doesn't drop the rest of the buffered data. + int discard = found > 0 ? found : (_config.NumStartBytes == 0 ? 1 : _internalDataLen); _diagnostics.CntSyncRecoveries++; _diagnostics.CntFailedBytes += discard; int keep = _internalDataLen - discard; @@ -188,12 +190,19 @@ public FrameMsgInfo Next() { // Frame completed (valid or CRC-failed). Advance _currentOffset past the // bytes in the current buffer that formed the tail of this cross-chunk frame. + if (!result.Valid && _config.HasCrc) + { + _diagnostics.CntCrcFailures++; + _diagnostics.CntFailedBytes += result.FrameSize; + _diagnostics.CntSyncRecoveries++; + } + RecordFrameDiagnostics(_internalBuffer, 0, result.Valid); int internalPrior = _internalDataLen - _bytesAppendedToInternal; int bytesFromCurrent = Math.Max(0, result.FrameSize - internalPrior); _currentOffset += bytesFromCurrent; _internalDataLen = 0; _bytesAppendedToInternal = 0; - return result; + return AttachDiagnostics(result); } if (result.Status == FrameMsgStatus.Collecting) @@ -233,8 +242,15 @@ public FrameMsgInfo Next() if (parseResult.FrameSize > 0) { + if (!parseResult.Valid && _config.HasCrc) + { + _diagnostics.CntCrcFailures++; + _diagnostics.CntFailedBytes += parseResult.FrameSize; + _diagnostics.CntSyncRecoveries++; + } + RecordFrameDiagnostics(_currentBuffer, _currentOffset, parseResult.Valid); _currentOffset += parseResult.FrameSize; - return parseResult; + return AttachDiagnostics(parseResult); } // Parse returned no frame — tail of buffer is a partial frame. @@ -464,8 +480,10 @@ private FrameMsgInfo HandleCollectingHeader(byte b) } fullMsgId |= _internalBuffer[_config.HeaderSize - 1]; var info = _getMessageInfo(fullMsgId); + // Valid frames may carry anywhere from MinSize (variable messages / + // truncated extensions) up to Size bytes — only count outside that range. if (info.HasValue && - (payloadLen > info.Value.Size || payloadLen < info.Value.BaseSize)) + (payloadLen > info.Value.Size || payloadLen < info.Value.MinSize)) { _diagnostics.CntLenErrors++; } @@ -608,11 +626,62 @@ private FrameMsgInfo ValidateAndReturn() // or `length` if none is found. private int FindStartByteOffset(byte[] buf, int start, int length) { - if (length <= 0 || _config.NumStartBytes == 0) return length; + if (length <= 0) return length; + // No start bytes (e.g. IPC profile): resync one byte at a time so a + // single unknown msg_id doesn't discard the rest of the buffer. + if (_config.NumStartBytes == 0) return 0; int found = Array.IndexOf(buf, _config.ComputedStartByte1, start, length); return found >= 0 ? found - start : length; } + // Record per-frame diagnostics for a complete frame consumed in buffer mode, + // matching the stream-mode (PushByte) counter semantics: + // - CntLenErrors when the header length is outside [MinSize, Size] + // - CntSeqGaps on valid frames for profiles that carry a sequence number + // `frame`/`frameOffset` locate the first byte of the frame (start bytes included). + private void RecordFrameDiagnostics(byte[]? frame, int frameOffset, bool valid) + { + if (frame == null) return; + if (_config.HasLength && _getMessageInfo != null) + { + int lenOffset = frameOffset + _config.NumStartBytes; + if (_config.HasSeq) lenOffset++; + if (_config.HasSysId) lenOffset++; + if (_config.HasCompId) lenOffset++; + + int payloadLen = _config.LengthBytes == 1 + ? frame[lenOffset] + : frame[lenOffset] | (frame[lenOffset + 1] << 8); + + int fullMsgId = 0; + if (_config.HasPkgId) + { + fullMsgId = frame[frameOffset + _config.HeaderSize - 2] << 8; + } + fullMsgId |= frame[frameOffset + _config.HeaderSize - 1]; + var info = _getMessageInfo(fullMsgId); + if (info.HasValue && + (payloadLen > info.Value.Size || payloadLen < info.Value.MinSize)) + { + _diagnostics.CntLenErrors++; + } + } + if (valid && _config.HasSeq) + { + byte seq = frame[frameOffset + _config.NumStartBytes]; + if (_lastSeqValid) + { + byte expectedSeq = (byte)((_lastSeq + 1) & 0xFF); + if (seq != expectedSeq) + { + _diagnostics.CntSeqGaps++; + } + } + _lastSeq = seq; + _lastSeqValid = true; + } + } + private FrameMsgInfo StatusResult(FrameMsgStatus status, int frameSize = 0) { var r = FrameMsgInfo.Invalid; diff --git a/src/struct_frame/boilerplate/csharp/Framework/Framing/BufferReader.cs b/src/struct_frame/boilerplate/csharp/Framework/Framing/BufferReader.cs index c2d0a39c..35aa2423 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Framing/BufferReader.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Framing/BufferReader.cs @@ -66,7 +66,13 @@ public FrameMsgInfo Next() int oldOffset = _offset; int searchFrom = _offset + 1; int searchLen = _size - searchFrom; - if (searchLen > 0 && _config.NumStartBytes > 0) + if (_config.NumStartBytes == 0) + { + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + _offset += 1; + } + else if (searchLen > 0) { int found = Array.IndexOf(_buffer, _config.ComputedStartByte1, searchFrom, searchLen); _offset = found >= 0 ? found : _size; diff --git a/src/struct_frame/boilerplate/csharp/Framework/Types/MessageInfo.cs b/src/struct_frame/boilerplate/csharp/Framework/Types/MessageInfo.cs index 25bb699c..f1106699 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Types/MessageInfo.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Types/MessageInfo.cs @@ -11,6 +11,8 @@ public readonly struct MessageInfo public byte Magic1 { get; } public byte Magic2 { get; } public int BaseSize { get; } // Non-extension portion size (== Size when no extensions) + public int MinSize { get; } // Minimum valid payload size: BaseSize for fixed messages, + // MinSize for variable messages public MessageInfo(int size, byte magic1 = 0, byte magic2 = 0) { @@ -18,6 +20,7 @@ public MessageInfo(int size, byte magic1 = 0, byte magic2 = 0) Magic1 = magic1; Magic2 = magic2; BaseSize = size; + MinSize = size; } public MessageInfo(int size, byte magic1, byte magic2, int baseSize) @@ -26,6 +29,16 @@ public MessageInfo(int size, byte magic1, byte magic2, int baseSize) Magic1 = magic1; Magic2 = magic2; BaseSize = baseSize; + MinSize = baseSize; + } + + public MessageInfo(int size, byte magic1, byte magic2, int baseSize, int minSize) + { + Size = size; + Magic1 = magic1; + Magic2 = magic2; + BaseSize = baseSize; + MinSize = minSize; } } } diff --git a/src/struct_frame/boilerplate/js/frame-profiles.js b/src/struct_frame/boilerplate/js/frame-profiles.js index cfb0697e..48447d46 100644 --- a/src/struct_frame/boilerplate/js/frame-profiles.js +++ b/src/struct_frame/boilerplate/js/frame-profiles.js @@ -211,17 +211,22 @@ function parseFrameWithCrc(config, buffer, getMessageInfo) { const headerSize = profileHeaderSize(config); const footerSize = profileFooterSize(config); if (length < headerSize + footerSize) { + // Not enough bytes yet to even hold the header+footer + result.status = frame_base_1.FrameMsgStatus.Collecting; return result; } let idx = 0; - // Verify start bytes + // Verify start bytes. A mismatch means this position is not a frame start, + // so report WaitingForStart to let buffer readers scan forward for a start byte. if (header.numStartBytes >= 1) { if (buffer[idx++] !== config.startByte1) { + result.status = frame_base_1.FrameMsgStatus.WaitingForStart; return result; } } if (header.numStartBytes >= 2) { if (buffer[idx++] !== config.startByte2) { + result.status = frame_base_1.FrameMsgStatus.WaitingForStart; return result; } } @@ -253,6 +258,8 @@ function parseFrameWithCrc(config, buffer, getMessageInfo) { // Verify total size const totalSize = headerSize + msgLen + footerSize; if (length < totalSize) { + // Header parsed but payload/footer not fully received yet + result.status = frame_base_1.FrameMsgStatus.Collecting; return result; } // Verify CRC (extension-aware) @@ -302,30 +309,39 @@ function parseFrameMinimal(config, buffer, getMessageInfo) { const header = config.header; const headerSize = profileHeaderSize(config); if (buffer.length < headerSize) { + // Not enough bytes yet to even hold the header + result.status = frame_base_1.FrameMsgStatus.Collecting; return result; } let idx = 0; - // Verify start bytes + // Verify start bytes. A mismatch means this position is not a frame start, + // so report WaitingForStart to let buffer readers scan forward for a start byte. if (header.numStartBytes >= 1) { if (buffer[idx++] !== config.startByte1) { + result.status = frame_base_1.FrameMsgStatus.WaitingForStart; return result; } } if (header.numStartBytes >= 2) { if (buffer[idx++] !== config.startByte2) { + result.status = frame_base_1.FrameMsgStatus.WaitingForStart; return result; } } // Read message ID const msgId = buffer[idx]; - // Get message length from callback + // Get message length from callback. An unknown msg_id means this position + // is not a frame start — report WaitingForStart so readers resync. const info = getMessageInfo(msgId); if (!info) { + result.status = frame_base_1.FrameMsgStatus.WaitingForStart; return result; } const msgLen = info.size; const totalSize = headerSize + msgLen; if (buffer.length < totalSize) { + // Header parsed but payload not fully received yet + result.status = frame_base_1.FrameMsgStatus.Collecting; return result; } // Extract message data @@ -384,16 +400,27 @@ class BufferReader { const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); this._offset += frameSize; } - else { - // If a complete frame failed CRC, skip it and continue on next call. - if (result.status === frame_base_1.FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { - this._offset += result.frameSize; + else if (result.status === frame_base_1.FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { + // A complete frame failed CRC — skip it and continue on next call. + this._offset += result.frameSize; + } + else if (result.status === frame_base_1.FrameMsgStatus.WaitingForStart) { + // Head byte is not a frame start — scan forward to the next start byte, + // reporting SyncRecovery so tryNext() keeps advancing. + const oldOffset = this._offset; + if (this.config.header.numStartBytes >= 1) { + const nxt = this.buffer.indexOf(this.config.startByte1, this._offset + 1); + this._offset = (nxt !== -1 && nxt < this.size) ? nxt : this.size; } else { - // No more valid frames - stop parsing - this._offset = this.size; + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + this._offset += 1; } + result.status = frame_base_1.FrameMsgStatus.SyncRecovery; + result.frameSize = this._offset - oldOffset; } + // Collecting: leave offset unchanged — trailing partial frame (normal end-of-buffer) return result; } /** Reset the reader to the beginning of the buffer. */ @@ -583,6 +610,7 @@ class AccumulatingReader { const result = this.parseBuffer(internalBytes); if (result.valid) { result.msgData = result.msgData.slice(); // own copy — internalBuffer is reused + this.recordFrameDiagnostics(this.internalBuffer, true); const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; const bytesFromCurrent = frameSize > partialLen ? frameSize - partialLen : 0; @@ -599,6 +627,7 @@ class AccumulatingReader { this._diagnostics.cntCrcFailures++; this._diagnostics.cntFailedBytes += result.frameSize; this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(this.internalBuffer, false); const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; const bytesFromCurrent = result.frameSize > partialLen ? result.frameSize - partialLen : 0; this.currentOffset = bytesFromCurrent; @@ -616,6 +645,7 @@ class AccumulatingReader { const remaining = this.currentBuffer.subarray(this.currentOffset); const result = this.parseBuffer(remaining); if (result.valid) { + this.recordFrameDiagnostics(remaining, true); const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); this.currentOffset += frameSize; return this.withDiagnostics(result); @@ -625,12 +655,34 @@ class AccumulatingReader { this._diagnostics.cntCrcFailures++; this._diagnostics.cntFailedBytes += result.frameSize; this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(remaining, false); this.currentOffset += result.frameSize; return this.withDiagnostics(result); } - // Parse failed - might be partial message at end of buffer + if (result.status === frame_base_1.FrameMsgStatus.WaitingForStart) { + // Head byte is not a frame start — scan forward to the next start byte, + // reporting SyncRecovery with frameSize=bytes skipped so tryNext() keeps draining. + const oldOffset = this.currentOffset; + if (this.config.header.numStartBytes >= 1) { + const nxt = this.currentBuffer.indexOf(this.config.startByte1, this.currentOffset + 1); + this.currentOffset = (nxt !== -1 && nxt < this.currentSize) ? nxt : this.currentSize; + } + else { + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + this.currentOffset += 1; + } + const skipped = this.currentOffset - oldOffset; + this._diagnostics.cntFailedBytes += skipped; + this._diagnostics.cntSyncRecoveries++; + const r = (0, frame_base_1.createFrameMsgInfo)(); + r.status = frame_base_1.FrameMsgStatus.SyncRecovery; + r.frameSize = skipped; + return this.withDiagnostics(r); + } + // Collecting — save the trailing partial frame for the next addData() call const remainingLen = this.currentSize - this.currentOffset; - if (remainingLen > 0 && remainingLen < this.bufferSize) { + if (remainingLen > 0 && remainingLen < this.bufferSize && result.status === frame_base_1.FrameMsgStatus.Collecting) { this.internalBuffer.set(remaining, 0); this.internalDataLen = remainingLen; this.currentOffset = this.currentSize; @@ -1004,5 +1056,54 @@ class AccumulatingReader { result.diagnostics = { ...this._diagnostics }; return result; } + /** + * Record per-frame diagnostics for a complete frame consumed in buffer mode, + * matching the stream-mode (pushByte) counter semantics: + * - cntLenErrors when the header length is outside [minSize, size] + * - cntSeqGaps on valid frames for profiles that carry a sequence number + * `frame` is the raw frame bytes (start bytes included). + */ + recordFrameDiagnostics(frame, valid) { + const config = this.config; + if (config.payload.hasLength && this.getMessageInfo) { + let lenOffset = config.header.numStartBytes; + if (config.payload.hasSeq) + lenOffset++; + if (config.payload.hasSysId) + lenOffset++; + if (config.payload.hasCompId) + lenOffset++; + let payloadLen; + if (config.payload.lengthBytes === 1) { + payloadLen = frame[lenOffset]; + } + else { + payloadLen = frame[lenOffset] | (frame[lenOffset + 1] << 8); + } + const headerSize = profileHeaderSize(config); + let fullMsgId = 0; + if (config.payload.hasPkgId) { + fullMsgId = frame[headerSize - 2] << 8; + } + fullMsgId |= frame[headerSize - 1]; + const info = this.getMessageInfo(fullMsgId); + if (info !== undefined) { + const minSize = info.minSize ?? (info.isVariable ? 0 : info.size); + if (payloadLen > info.size || payloadLen < minSize) { + this._diagnostics.cntLenErrors++; + } + } + } + if (valid && config.payload.hasSeq) { + const seq = frame[config.header.numStartBytes]; + if (this._lastSeq !== null) { + const expectedSeq = (this._lastSeq + 1) & 0xFF; + if (seq !== expectedSeq) { + this._diagnostics.cntSeqGaps++; + } + } + this._lastSeq = seq; + } + } } exports.AccumulatingReader = AccumulatingReader; diff --git a/src/struct_frame/boilerplate/js/struct-frame-sdk/struct-frame-sdk.js b/src/struct_frame/boilerplate/js/struct-frame-sdk/struct-frame-sdk.js index f29a3513..f94a5f09 100644 --- a/src/struct_frame/boilerplate/js/struct-frame-sdk/struct-frame-sdk.js +++ b/src/struct_frame/boilerplate/js/struct-frame-sdk/struct-frame-sdk.js @@ -163,9 +163,13 @@ class StructFrameSdk { this.log(`Failed to deserialize message ID ${result.msgId}: ${error}`); } } - for (let i = 0; i < handlers.length; i++) { + // Iterate a snapshot: a handler that unsubscribes during dispatch (e.g. the + // one-shot handler registered by request()) splices the live array, which + // would otherwise skip the next handler for this same message. + const snapshot = handlers.slice(); + for (let i = 0; i < snapshot.length; i++) { try { - handlers[i](message, result.msgId); + snapshot[i](message, result.msgId); } catch (error) { this.log(`Handler error for message ID ${result.msgId}: ${error}`); diff --git a/src/struct_frame/boilerplate/py/frame_profiles.py b/src/struct_frame/boilerplate/py/frame_profiles.py index 01071d57..1a5084f2 100644 --- a/src/struct_frame/boilerplate/py/frame_profiles.py +++ b/src/struct_frame/boilerplate/py/frame_profiles.py @@ -65,11 +65,15 @@ class MessageInfo(NamedTuple): magic1: First magic number for CRC calculation magic2: Second magic number for CRC calculation base_size: Non-extension portion size (== size when no extensions) + min_size: Minimum valid payload size (base_size for fixed messages, + MIN_SIZE for variable messages; 0 means "unset" — parsers fall + back to base_size) """ size: int magic1: int = 0 magic2: int = 0 base_size: int = 0 + min_size: int = 0 # ============================================================================= @@ -576,33 +580,41 @@ def _frame_format_parse_minimal( FrameMsgInfo with valid=True if frame is valid """ result = FrameMsgInfo() - + if len(buffer) < config.header_size: + # Not enough bytes yet to even hold the header + result.status = FrameMsgStatus.COLLECTING return result - + idx = 0 - - # Verify start bytes + + # Verify start bytes. A mismatch means this position is not a frame start, + # so report WAITING_FOR_START to let buffer readers scan forward for a start byte. if config.num_start_bytes >= 1: if buffer[idx] != config.computed_start_byte1(): + result.status = FrameMsgStatus.WAITING_FOR_START return result idx += 1 if config.num_start_bytes >= 2: if buffer[idx] != config.computed_start_byte2(): + result.status = FrameMsgStatus.WAITING_FOR_START return result idx += 1 - + # Read message ID msg_id = buffer[idx] - - # Get message info from callback + + # Get message info from callback. An unknown msg_id means this position is + # not a frame start — report WAITING_FOR_START so readers resync. msg_info = get_message_info(msg_id) if msg_info is None: + result.status = FrameMsgStatus.WAITING_FOR_START return result msg_len = msg_info.size - + total_size = config.header_size + msg_len if len(buffer) < total_size: + result.status = FrameMsgStatus.COLLECTING return result # Extract message data @@ -757,18 +769,21 @@ def next(self) -> FrameMsgInfo: if result.frame_size > 0: # Advance past complete frames (valid, or CRC-failed but structurally known) self._offset += result.frame_size - elif result.status == FrameMsgStatus.WAITING_FOR_START and self._config.num_start_bytes >= 1: + elif result.status == FrameMsgStatus.WAITING_FOR_START: # Head byte is not a frame start — scan forward to the next start byte, # reporting SyncRecovery so try_next() keeps advancing. old_offset = self._offset - start1 = bytes([self._config.computed_start_byte1()]) - nxt = self._buffer.find(start1, self._offset + 1, self._size) - self._offset = nxt if nxt != -1 else self._size + if self._config.num_start_bytes >= 1: + start1 = bytes([self._config.computed_start_byte1()]) + nxt = self._buffer.find(start1, self._offset + 1, self._size) + self._offset = nxt if nxt != -1 else self._size + else: + # No start bytes (e.g. IPC profile): advance one byte and retry so a + # single unknown msg_id doesn't discard the rest of the buffer. + self._offset += 1 result.status = FrameMsgStatus.SYNC_RECOVERY result.frame_size = self._offset - old_offset - else: - # Incomplete/garbage with no recoverable start — consume the rest - self._offset = self._size + # COLLECTING: leave offset unchanged — trailing partial frame (normal end-of-buffer) return result @@ -1010,6 +1025,7 @@ def next(self) -> FrameMsgInfo: partial_len = self._internal_data_len - self._bytes_appended_to_internal if result.valid: + self._record_frame_diagnostics(internal_bytes, True) # How many bytes from the current buffer were consumed to complete the frame bytes_from_current = result.frame_size - partial_len if result.frame_size > partial_len else 0 self._current_offset = bytes_from_current @@ -1024,6 +1040,7 @@ def next(self) -> FrameMsgInfo: self._diag.cnt_crc_failures += 1 self._diag.cnt_failed_bytes += result.frame_size self._diag.cnt_sync_recoveries += 1 + self._record_frame_diagnostics(internal_bytes, False) bytes_from_current = result.frame_size - partial_len if result.frame_size > partial_len else 0 self._current_offset = bytes_from_current self._internal_data_len = 0 @@ -1042,6 +1059,7 @@ def next(self) -> FrameMsgInfo: result = self._parse_buffer(remaining) if result.valid: + self._record_frame_diagnostics(remaining, True) self._current_offset += result.frame_size return result @@ -1051,16 +1069,22 @@ def next(self) -> FrameMsgInfo: self._diag.cnt_crc_failures += 1 self._diag.cnt_failed_bytes += result.frame_size self._diag.cnt_sync_recoveries += 1 + self._record_frame_diagnostics(remaining, False) self._current_offset += result.frame_size return self._with_diag(result) - if result.status == FrameMsgStatus.WAITING_FOR_START and self._config.num_start_bytes >= 1: + if result.status == FrameMsgStatus.WAITING_FOR_START: # Head byte is not a frame start — scan forward to the next start byte, # reporting SyncRecovery with frame_size=bytes_skipped so try_next() keeps draining. old_offset = self._current_offset - start1 = bytes([self._config.computed_start_byte1()]) - nxt = self._current_buffer.find(start1, self._current_offset + 1, self._current_size) - self._current_offset = nxt if nxt != -1 else self._current_size + if self._config.num_start_bytes >= 1: + start1 = bytes([self._config.computed_start_byte1()]) + nxt = self._current_buffer.find(start1, self._current_offset + 1, self._current_size) + self._current_offset = nxt if nxt != -1 else self._current_size + else: + # No start bytes (e.g. IPC profile): advance one byte and retry so a + # single unknown msg_id doesn't discard the rest of the buffer. + self._current_offset += 1 skipped = self._current_offset - old_offset self._diag.cnt_failed_bytes += skipped self._diag.cnt_sync_recoveries += 1 @@ -1114,6 +1138,42 @@ def _with_diag(self, result: FrameMsgInfo) -> FrameMsgInfo: def _status_result(self, status: FrameMsgStatus) -> FrameMsgInfo: return self._with_diag(FrameMsgInfo(status=status)) + + def _record_frame_diagnostics(self, frame, valid: bool) -> None: + """Record per-frame diagnostics for a complete frame consumed in buffer + mode, matching the stream-mode (push_byte) counter semantics: + cnt_len_errors when the header length is outside [min_size, size], and + cnt_seq_gaps on valid frames for profiles that carry a sequence number. + `frame` is the raw frame bytes (start bytes included).""" + config = self._config + if config.has_length and self._get_message_info is not None: + len_offset = config.num_start_bytes + if config.has_sequence: + len_offset += 1 + if config.has_system_id: + len_offset += 1 + if config.has_component_id: + len_offset += 1 + if config.length_bytes == 1: + payload_len = frame[len_offset] + else: + payload_len = frame[len_offset] | (frame[len_offset + 1] << 8) + if config.has_package_id: + full_msg_id = (frame[config.header_size - 2] << 8) | frame[config.header_size - 1] + else: + full_msg_id = frame[config.header_size - 1] + msg_info = self._get_message_info(full_msg_id) + if msg_info is not None: + min_size = getattr(msg_info, 'min_size', 0) or getattr(msg_info, 'base_size', msg_info.size) + if payload_len > msg_info.size or payload_len < min_size: + self._diag.cnt_len_errors += 1 + if valid and config.has_sequence: + seq = frame[config.num_start_bytes] + if self._last_seq is not None: + expected_seq = (self._last_seq + 1) & 0xFF + if seq != expected_seq: + self._diag.cnt_seq_gaps += 1 + self._last_seq = seq def _handle_looking_for_start1(self, byte: int) -> FrameMsgInfo: """Handle LOOKING_FOR_START1 state""" @@ -1247,12 +1307,12 @@ def _handle_collecting_header(self, byte: int) -> FrameMsgInfo: full_msg_id = self._internal_buffer[self._config.header_size - 1] msg_info = self._get_message_info(full_msg_id) if msg_info is not None: - # A valid frame may carry anywhere from base_size (extensions - # truncated) up to size (full extensions) bytes. Only count a - # length error when payload_len falls outside that range — - # matches the C++ parser semantics. - base_size = getattr(msg_info, 'base_size', msg_info.size) - if payload_len > msg_info.size or payload_len < base_size: + # A valid frame may carry anywhere from min_size (variable + # messages / truncated extensions) up to size (full + # extensions) bytes. Only count a length error when + # payload_len falls outside that range. + min_size = getattr(msg_info, 'min_size', 0) or getattr(msg_info, 'base_size', msg_info.size) + if payload_len > msg_info.size or payload_len < min_size: self._diag.cnt_len_errors += 1 self._expected_frame_size = self._config.overhead + payload_len diff --git a/src/struct_frame/boilerplate/rust/frame_profiles.rs b/src/struct_frame/boilerplate/rust/frame_profiles.rs index f05e17a9..54a151e5 100644 --- a/src/struct_frame/boilerplate/rust/frame_profiles.rs +++ b/src/struct_frame/boilerplate/rust/frame_profiles.rs @@ -774,12 +774,18 @@ impl BufferReader { BufferReader { config, data, offset: 0 } } - /// Get the next frame from the buffer. + /// Get the next frame event from the buffer. /// - /// B1: on CRC failure or bad start bytes, advances the internal offset to the next - /// candidate start-byte position and returns None, so the *next* call can attempt - /// the following frame. Only stalls (returns None without advancing) when the - /// buffer has too little data to make a decision (Collecting status). + /// Matches the cross-language BufferReader drain contract: returns Some(...) + /// while forward progress is made — a valid frame, a complete-but-CRC-failed + /// frame (status CrcFailure, skipped in full), or a SyncRecovery event for + /// skipped garbage bytes. Returns None only when the buffer is drained or a + /// trailing partial frame remains (Collecting). + /// + /// Canonical drain loop: + /// while let Some(f) = reader.next(&get_message_info) { + /// if f.valid { handle(f); } // else CrcFailure or SyncRecovery + /// } pub fn next(&mut self, get_message_info: &dyn Fn(u16) -> Option) -> Option { if self.offset >= self.data.len() { return None; @@ -796,12 +802,31 @@ impl BufferReader { return Some(result); } - // B1: advance past the bad position so the next call starts from the next candidate. - if result.status != FrameMsgStatus::Collecting { - let advance = self.find_next_start_byte_offset(remaining).max(1); - self.offset += advance; + // Complete-but-invalid frame (CRC failure): surface it and skip past it, + // so frames after a corrupt one are still delivered. + if result.frame_size > 0 { + self.offset += result.frame_size; + return Some(result); + } + + if result.status == FrameMsgStatus::Collecting { + // Trailing partial frame — leave offset unchanged. + return None; } - None + + // Bad start byte / unknown msg_id: skip to the next candidate start position + // and surface a SyncRecovery event so drain loops keep advancing. + let advance = self.find_next_start_byte_offset(remaining).max(1); + self.offset += advance; + let mut sync = FrameMsgInfo::invalid(); + sync.status = FrameMsgStatus::SyncRecovery; + sync.frame_size = advance; + Some(sync) + } + + /// Drain-loop alias matching the AccumulatingReader / cross-language naming. + pub fn try_next(&mut self, get_message_info: &dyn Fn(u16) -> Option) -> Option { + self.next(get_message_info) } /// Find the offset of the next possible start-byte sequence within `buf` starting at index 1. @@ -946,6 +971,50 @@ impl AccumulatingReader { self.maybe_compact(); } + /// Increment cnt_len_errors when the frame at the buffer head carries a length + /// field outside the [min_size, size] range for its message — matching the + /// stream-mode counter semantics of the other language parsers. + /// A8: mirrors the C++ / TS check — only errors on out-of-range lengths. + fn record_len_error_if_any(&mut self, get_message_info: &dyn Fn(u16) -> Option) { + if !self.starts_with_possible_frame() || !self.config.payload.has_length { + return; + } + let buf = self.buf(); + let header_size = self.config.header_size(); + if buf.len() < header_size { + return; + } + let mut len_offset = self.config.header.num_start_bytes as usize; + if self.config.payload.has_seq { + len_offset += 1; + } + if self.config.payload.has_sys_id { + len_offset += 1; + } + if self.config.payload.has_comp_id { + len_offset += 1; + } + let msg_len = if self.config.payload.length_bytes == 1 { + buf[len_offset] as usize + } else { + (buf[len_offset] as usize) | ((buf[len_offset + 1] as usize) << 8) + }; + let mut id_offset = len_offset + self.config.payload.length_bytes as usize; + let mut full_msg_id: u16 = 0; + if self.config.payload.has_pkg_id && buf.len() > id_offset { + full_msg_id = (buf[id_offset] as u16) << 8; + id_offset += 1; + } + if buf.len() > id_offset { + full_msg_id |= buf[id_offset] as u16; + if let Some(info) = get_message_info(full_msg_id) { + if msg_len > info.size || msg_len < info.min_size { + self.diagnostics.cnt_len_errors += 1; + } + } + } + } + /// Returns true if the buffer front starts with the expected start-byte sequence fn starts_with_possible_frame(&self) -> bool { let buf = self.buf(); @@ -1184,6 +1253,7 @@ impl AccumulatingReader { self.last_seq = Some(seq); } + self.record_len_error_if_any(get_message_info); self.partial_pending = false; self.drain_head(frame_size); return Some(result); @@ -1196,6 +1266,7 @@ impl AccumulatingReader { } self.diagnostics.cnt_failed_bytes += result.frame_size as u32; self.diagnostics.cnt_sync_recoveries += 1; + self.record_len_error_if_any(get_message_info); self.partial_pending = false; self.drain_head(result.frame_size); return Some(result); @@ -1213,42 +1284,7 @@ impl AccumulatingReader { } // Record diagnostics before draining - if self.starts_with_possible_frame() && self.config.payload.has_length { - let buf = self.buf(); - let header_size = self.config.header_size(); - if buf.len() >= header_size { - let mut len_offset = self.config.header.num_start_bytes as usize; - if self.config.payload.has_seq { - len_offset += 1; - } - if self.config.payload.has_sys_id { - len_offset += 1; - } - if self.config.payload.has_comp_id { - len_offset += 1; - } - let msg_len = if self.config.payload.length_bytes == 1 { - buf[len_offset] as usize - } else { - (buf[len_offset] as usize) | ((buf[len_offset + 1] as usize) << 8) - }; - let mut id_offset = len_offset + self.config.payload.length_bytes as usize; - let mut full_msg_id: u16 = 0; - if self.config.payload.has_pkg_id && buf.len() > id_offset { - full_msg_id = (buf[id_offset] as u16) << 8; - id_offset += 1; - } - if buf.len() > id_offset { - full_msg_id |= buf[id_offset] as u16; - if let Some(info) = get_message_info(full_msg_id) { - // A8: mirrors C++ / TS fix — only error on out-of-range - if msg_len > info.size || msg_len < info.min_size { - self.diagnostics.cnt_len_errors += 1; - } - } - } - } - } + self.record_len_error_if_any(get_message_info); self.diagnostics.cnt_failed_bytes += bytes_to_drain as u32; self.diagnostics.cnt_sync_recoveries += 1; diff --git a/src/struct_frame/boilerplate/ts/frame-profiles.ts b/src/struct_frame/boilerplate/ts/frame-profiles.ts index bd40e92f..3bd8047d 100644 --- a/src/struct_frame/boilerplate/ts/frame-profiles.ts +++ b/src/struct_frame/boilerplate/ts/frame-profiles.ts @@ -343,19 +343,24 @@ export function parseFrameWithCrc( const footerSize = profileFooterSize(config); if (length < headerSize + footerSize) { + // Not enough bytes yet to even hold the header+footer + result.status = FrameMsgStatus.Collecting; return result; } let idx = 0; - // Verify start bytes + // Verify start bytes. A mismatch means this position is not a frame start, + // so report WaitingForStart to let buffer readers scan forward for a start byte. if (header.numStartBytes >= 1) { if (buffer[idx++] !== config.startByte1) { + result.status = FrameMsgStatus.WaitingForStart; return result; } } if (header.numStartBytes >= 2) { if (buffer[idx++] !== config.startByte2) { + result.status = FrameMsgStatus.WaitingForStart; return result; } } @@ -388,6 +393,8 @@ export function parseFrameWithCrc( // Verify total size const totalSize = headerSize + msgLen + footerSize; if (length < totalSize) { + // Header parsed but payload/footer not fully received yet + result.status = FrameMsgStatus.Collecting; return result; } @@ -447,19 +454,24 @@ export function parseFrameMinimal( const headerSize = profileHeaderSize(config); if (buffer.length < headerSize) { + // Not enough bytes yet to even hold the header + result.status = FrameMsgStatus.Collecting; return result; } let idx = 0; - // Verify start bytes + // Verify start bytes. A mismatch means this position is not a frame start, + // so report WaitingForStart to let buffer readers scan forward for a start byte. if (header.numStartBytes >= 1) { if (buffer[idx++] !== config.startByte1) { + result.status = FrameMsgStatus.WaitingForStart; return result; } } if (header.numStartBytes >= 2) { if (buffer[idx++] !== config.startByte2) { + result.status = FrameMsgStatus.WaitingForStart; return result; } } @@ -467,15 +479,19 @@ export function parseFrameMinimal( // Read message ID const msgId = buffer[idx]; - // Get message length from callback + // Get message length from callback. An unknown msg_id means this position + // is not a frame start — report WaitingForStart so readers resync. const info = getMessageInfo(msgId); if (!info) { + result.status = FrameMsgStatus.WaitingForStart; return result; } const msgLen = info.size; const totalSize = headerSize + msgLen; if (buffer.length < totalSize) { + // Header parsed but payload not fully received yet + result.status = FrameMsgStatus.Collecting; return result; } @@ -550,15 +566,25 @@ export class BufferReader { if (result.valid) { const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); this._offset += frameSize; - } else { - // If a complete frame failed CRC, skip it and continue on next call. - if (result.status === FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { - this._offset += result.frameSize!; + } else if (result.status === FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { + // A complete frame failed CRC — skip it and continue on next call. + this._offset += result.frameSize!; + } else if (result.status === FrameMsgStatus.WaitingForStart) { + // Head byte is not a frame start — scan forward to the next start byte, + // reporting SyncRecovery so tryNext() keeps advancing. + const oldOffset = this._offset; + if (this.config.header.numStartBytes >= 1) { + const nxt = this.buffer.indexOf(this.config.startByte1, this._offset + 1); + this._offset = (nxt !== -1 && nxt < this.size) ? nxt : this.size; } else { - // No more valid frames - stop parsing - this._offset = this.size; + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + this._offset += 1; } + result.status = FrameMsgStatus.SyncRecovery; + result.frameSize = this._offset - oldOffset; } + // Collecting: leave offset unchanged — trailing partial frame (normal end-of-buffer) return result; } @@ -821,6 +847,7 @@ export class AccumulatingReader { if (result.valid) { result.msgData = result.msgData.slice(); // own copy — internalBuffer is reused + this.recordFrameDiagnostics(this.internalBuffer, true); const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; const bytesFromCurrent = frameSize > partialLen ? frameSize - partialLen : 0; @@ -838,6 +865,7 @@ export class AccumulatingReader { this._diagnostics.cntCrcFailures++; this._diagnostics.cntFailedBytes += result.frameSize!; this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(this.internalBuffer, false); const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; const bytesFromCurrent = result.frameSize! > partialLen ? result.frameSize! - partialLen : 0; this.currentOffset = bytesFromCurrent; @@ -858,6 +886,7 @@ export class AccumulatingReader { const result = this.parseBuffer(remaining); if (result.valid) { + this.recordFrameDiagnostics(remaining, true); const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); this.currentOffset += frameSize; return this.withDiagnostics(result); @@ -868,13 +897,35 @@ export class AccumulatingReader { this._diagnostics.cntCrcFailures++; this._diagnostics.cntFailedBytes += result.frameSize!; this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(remaining, false); this.currentOffset += result.frameSize!; return this.withDiagnostics(result); } - // Parse failed - might be partial message at end of buffer + if (result.status === FrameMsgStatus.WaitingForStart) { + // Head byte is not a frame start — scan forward to the next start byte, + // reporting SyncRecovery with frameSize=bytes skipped so tryNext() keeps draining. + const oldOffset = this.currentOffset; + if (this.config.header.numStartBytes >= 1) { + const nxt = this.currentBuffer.indexOf(this.config.startByte1, this.currentOffset + 1); + this.currentOffset = (nxt !== -1 && nxt < this.currentSize) ? nxt : this.currentSize; + } else { + // No start bytes (e.g. IPC profile): advance one byte and retry so a + // single unknown msg_id doesn't discard the rest of the buffer. + this.currentOffset += 1; + } + const skipped = this.currentOffset - oldOffset; + this._diagnostics.cntFailedBytes += skipped; + this._diagnostics.cntSyncRecoveries++; + const r = createFrameMsgInfo(); + r.status = FrameMsgStatus.SyncRecovery; + r.frameSize = skipped; + return this.withDiagnostics(r); + } + + // Collecting — save the trailing partial frame for the next addData() call const remainingLen = this.currentSize - this.currentOffset; - if (remainingLen > 0 && remainingLen < this.bufferSize) { + if (remainingLen > 0 && remainingLen < this.bufferSize && result.status === FrameMsgStatus.Collecting) { this.internalBuffer.set(remaining, 0); this.internalDataLen = remainingLen; this.currentOffset = this.currentSize; @@ -1256,5 +1307,53 @@ export class AccumulatingReader { result.diagnostics = { ...this._diagnostics }; return result; } + + /** + * Record per-frame diagnostics for a complete frame consumed in buffer mode, + * matching the stream-mode (pushByte) counter semantics: + * - cntLenErrors when the header length is outside [minSize, size] + * - cntSeqGaps on valid frames for profiles that carry a sequence number + * `frame` is the raw frame bytes (start bytes included). + */ + private recordFrameDiagnostics(frame: Uint8Array, valid: boolean): void { + const config = this.config; + if (config.payload.hasLength && this.getMessageInfo) { + let lenOffset = config.header.numStartBytes; + if (config.payload.hasSeq) lenOffset++; + if (config.payload.hasSysId) lenOffset++; + if (config.payload.hasCompId) lenOffset++; + + let payloadLen: number; + if (config.payload.lengthBytes === 1) { + payloadLen = frame[lenOffset]; + } else { + payloadLen = frame[lenOffset] | (frame[lenOffset + 1] << 8); + } + + const headerSize = profileHeaderSize(config); + let fullMsgId = 0; + if (config.payload.hasPkgId) { + fullMsgId = frame[headerSize - 2] << 8; + } + fullMsgId |= frame[headerSize - 1]; + const info = this.getMessageInfo(fullMsgId); + if (info !== undefined) { + const minSize = info.minSize ?? (info.isVariable ? 0 : info.size); + if (payloadLen > info.size || payloadLen < minSize) { + this._diagnostics.cntLenErrors++; + } + } + } + if (valid && config.payload.hasSeq) { + const seq = frame[config.header.numStartBytes]; + if (this._lastSeq !== null) { + const expectedSeq = (this._lastSeq + 1) & 0xFF; + if (seq !== expectedSeq) { + this._diagnostics.cntSeqGaps++; + } + } + this._lastSeq = seq; + } + } } diff --git a/src/struct_frame/boilerplate/ts/struct-frame-sdk/struct-frame-sdk.ts b/src/struct_frame/boilerplate/ts/struct-frame-sdk/struct-frame-sdk.ts index 9bb89644..3f539064 100644 --- a/src/struct_frame/boilerplate/ts/struct-frame-sdk/struct-frame-sdk.ts +++ b/src/struct_frame/boilerplate/ts/struct-frame-sdk/struct-frame-sdk.ts @@ -229,9 +229,13 @@ export class StructFrameSdk { this.log(`Failed to deserialize message ID ${result.msgId}: ${error}`); } } - for (let i = 0; i < handlers.length; i++) { + // Iterate a snapshot: a handler that unsubscribes during dispatch (e.g. the + // one-shot handler registered by request()) splices the live array, which + // would otherwise skip the next handler for this same message. + const snapshot = handlers.slice(); + for (let i = 0; i < snapshot.length; i++) { try { - handlers[i](message, result.msgId); + snapshot[i](message, result.msgId); } catch (error) { this.log(`Handler error for message ID ${result.msgId}: ${error}`); } diff --git a/src/struct_frame/cpp_gen.py b/src/struct_frame/cpp_gen.py index 92d3f6fe..f14185b9 100644 --- a/src/struct_frame/cpp_gen.py +++ b/src/struct_frame/cpp_gen.py @@ -1012,14 +1012,17 @@ def generate(package, imported_packages=None, equality=False): for key, msg in package.sortedMessages().items(): qualName = '%s::%s' % (namespace_name, msg.name) if msg.id is not None: + # min_size for the cnt_len_errors range check: MIN_SIZE for variable + # messages, BASE_SIZE otherwise (extensions may be truncated) — matches c_gen + effective_min = msg.min_size if msg.variable else msg.base_size if package.package_id is not None: # When using package ID, compare against local message ID - yield ' case %d: return MessageInfo{%s::MAX_SIZE, %s::MAGIC1, %s::MAGIC2, %s::BASE_SIZE};\n' % ( - msg.id, qualName, qualName, qualName, qualName) + yield ' case %d: return MessageInfo{%s::MAX_SIZE, %s::MAGIC1, %s::MAGIC2, %s::BASE_SIZE, %d};\n' % ( + msg.id, qualName, qualName, qualName, qualName, effective_min) else: # No package ID, compare against MSG_ID from MessageBase - yield ' case %s::MSG_ID: return MessageInfo{%s::MAX_SIZE, %s::MAGIC1, %s::MAGIC2, %s::BASE_SIZE};\n' % ( - qualName, qualName, qualName, qualName, qualName) + yield ' case %s::MSG_ID: return MessageInfo{%s::MAX_SIZE, %s::MAGIC1, %s::MAGIC2, %s::BASE_SIZE, %d};\n' % ( + qualName, qualName, qualName, qualName, qualName, effective_min) yield ' default: break;\n' yield ' }\n' diff --git a/src/struct_frame/csharp_gen.py b/src/struct_frame/csharp_gen.py index ca27e00f..3f12925e 100644 --- a/src/struct_frame/csharp_gen.py +++ b/src/struct_frame/csharp_gen.py @@ -1709,10 +1709,13 @@ def _generate_message_definitions(package): if msg.magic_bytes: magic1 = f'{structName}.Magic1' magic2 = f'{structName}.Magic2' + # MinSize for the CntLenErrors range check: MIN_SIZE for variable + # messages, BASE_SIZE otherwise (extensions may be truncated) — matches c_gen + effective_min = msg.min_size if msg.variable else msg.base_size if package.package_id is not None: - result += ' case %d: return new MessageInfo(%s.MaxSize, %s, %s, %s.BaseSize);\n' % (msg.id, structName, magic1, magic2, structName) + result += ' case %d: return new MessageInfo(%s.MaxSize, %s, %s, %s.BaseSize, %d);\n' % (msg.id, structName, magic1, magic2, structName, effective_min) else: - result += ' case %s.MsgId: return new MessageInfo(%s.MaxSize, %s, %s, %s.BaseSize);\n' % (structName, structName, magic1, magic2, structName) + result += ' case %s.MsgId: return new MessageInfo(%s.MaxSize, %s, %s, %s.BaseSize, %d);\n' % (structName, structName, magic1, magic2, structName, effective_min) result += ' default: return null;\n' result += ' }\n' result += ' }\n\n' diff --git a/src/struct_frame/js_gen.py b/src/struct_frame/js_gen.py index 5c0406d6..3a89e596 100644 --- a/src/struct_frame/js_gen.py +++ b/src/struct_frame/js_gen.py @@ -1039,7 +1039,9 @@ def generate(package, use_class_based=False, packages=None, equality=False): for msg in messages_with_id: package_msg_name = msg.name - min_size = msg.min_size if msg.variable else msg.size + # minSize for the cntLenErrors range check: MIN_SIZE for variable + # messages, BASE_SIZE otherwise (extensions may be truncated) — matches c_gen + min_size = msg.min_size if msg.variable else msg.base_size is_variable = 'true' if msg.variable else 'false' yield ' case %s._msgid: return { size: %s._size, minSize: %d, isVariable: %s, magic1: %s._magic1, magic2: %s._magic2, baseSize: %s._baseSize };\n' % ( package_msg_name, package_msg_name, min_size, is_variable, package_msg_name, package_msg_name, package_msg_name) diff --git a/src/struct_frame/py_gen.py b/src/struct_frame/py_gen.py index fcd9cc55..d392f829 100644 --- a/src/struct_frame/py_gen.py +++ b/src/struct_frame/py_gen.py @@ -1467,7 +1467,8 @@ def generate(package, imported_packages=None, imported_package_objects=None, equ yield f' magic1 = getattr(msg_class, "MAGIC1", 0)\n' yield f' magic2 = getattr(msg_class, "MAGIC2", 0)\n' yield f' base_size = getattr(msg_class, "BASE_SIZE", msg_class.msg_size)\n' - yield f' return MessageInfo(size=msg_class.msg_size, magic1=magic1, magic2=magic2, base_size=base_size)\n' + yield f' min_size = getattr(msg_class, "MIN_SIZE", base_size)\n' + yield f' return MessageInfo(size=msg_class.msg_size, magic1=magic1, magic2=magic2, base_size=base_size, min_size=min_size)\n' else: # Flat namespace mode: 8-bit message ID yield '%s_definitions = {\n' % package.name @@ -1508,7 +1509,8 @@ def generate(package, imported_packages=None, imported_package_objects=None, equ yield f' magic1 = getattr(msg_class, "MAGIC1", 0)\n' yield f' magic2 = getattr(msg_class, "MAGIC2", 0)\n' yield f' base_size = getattr(msg_class, "BASE_SIZE", msg_class.msg_size)\n' - yield f' return MessageInfo(size=msg_class.msg_size, magic1=magic1, magic2=magic2, base_size=base_size)\n' + yield f' min_size = getattr(msg_class, "MIN_SIZE", base_size)\n' + yield f' return MessageInfo(size=msg_class.msg_size, magic1=magic1, magic2=magic2, base_size=base_size, min_size=min_size)\n' class TestPyGen(): diff --git a/src/struct_frame/ts_gen.py b/src/struct_frame/ts_gen.py index b85bd160..c75c41ef 100644 --- a/src/struct_frame/ts_gen.py +++ b/src/struct_frame/ts_gen.py @@ -1166,7 +1166,9 @@ def generate(package, use_class_based=False, packages=None, equality=False): package_msg_name = msg.name magic1 = msg.magic_bytes[0] if msg.magic_bytes else 0 magic2 = msg.magic_bytes[1] if msg.magic_bytes else 0 - min_size = msg.min_size if msg.variable else msg.size + # minSize for the cntLenErrors range check: MIN_SIZE for variable + # messages, BASE_SIZE otherwise (extensions may be truncated) — matches c_gen + min_size = msg.min_size if msg.variable else msg.base_size is_variable = 'true' if msg.variable else 'false' if use_class_based: yield ' case %s._msgid: return { size: %s._size, minSize: %d, isVariable: %s, magic1: %s._magic1, magic2: %s._magic2, baseSize: %s._baseSize };\n' % ( diff --git a/tests/NEGATIVE_TESTS.md b/tests/NEGATIVE_TESTS.md index 9795d8fc..355d6720 100644 --- a/tests/NEGATIVE_TESTS.md +++ b/tests/NEGATIVE_TESTS.md @@ -16,49 +16,51 @@ Negative tests are critical for ensuring robust error handling. They verify that ## Test Files -All seven language implementations now share **20 identical test scenarios** (same names, -same behaviour), including a common `tryNext` drain contract and partial-pending checks. -Individual languages then add language-specific scenarios on top, so per-language totals -differ (see counts below): +All seven language implementations now share **31 identical test scenarios** (same names, +same behaviour), including a common `tryNext` drain contract, partial-pending checks, +diagnostic-counter assertions, minimal-profile resync scenarios, and a chunk-boundary +split sweep. All languages also carry the four package-corruption scenarios (bulk +`pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption), so +every language runs **35 scenarios**; Python adds 5 status-machine/diagnostic extras (40). ### C Tests (`tests/c/test_negative.c`) -- **24 test cases**: the 20 uniform scenarios + 4 C-specific (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) +- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios - Tests buffer reader and accumulating reader (buffer mode) APIs - Uses ProfileStandard, ProfileSensor, ProfileBulk, and ProfileNetwork configurations ### C++ Tests (`tests/cpp/test_negative.cpp`) -- **24 test cases**: the 20 uniform scenarios + 4 C++-specific (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) +- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios - Tests both BufferReader and AccumulatingReader APIs - Tests multiple frame profiles (Standard, Sensor, Bulk, Network) ### Python Tests (`tests/py/test_negative.py`) -- **34 test cases**: the 20 uniform scenarios + Python-specific extras (bulk/cross-package/network corruption, diagnostic-counter, and status-machine tests) +- **40 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios + 5 Python-specific status-machine/diagnostic extras - Tests both buffer and streaming modes - Uses ProfileStandardReader, ProfileSensorReader, and ProfileNetworkReader ### TypeScript Tests (`tests/ts/test_negative.ts`) -- **24 test cases**: the 20 uniform scenarios + 4 TS-specific (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) +- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### JavaScript Tests (`tests/js/test_negative.js`) -- **24 test cases** identical to TypeScript +- **35 test cases** identical to TypeScript - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### C# Tests (`tests/csharp/TestNegative.cs`) -- **24 test cases**: the 20 uniform scenarios + 4 C#-specific (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) +- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### Rust Tests (`tests/rust/src/test_negative.rs`) -- **20 test cases**: the full uniform scenario set +- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios - Tests BufferReader and AccumulatingReader APIs - Tests multiple profiles (Standard, Sensor, Bulk, Network) ## Uniform Test Scenarios -All seven languages implement the following 20 scenarios with identical names: +All seven languages implement the following 31 scenarios with identical names: 1. **Buffer mode: recovers after CRC failure** – buffer-mode accumulating reader resyncs and returns the next valid frame after a CRC-failed frame 2. **Buffer reader: skips CRC-failed frame** – `BufferReader` advances past a CRC-failed frame instead of stalling on it @@ -80,6 +82,17 @@ All seven languages implement the following 20 scenarios with identical names: 18. **TryNext drain: CRC/resync + valid** – `tryNext` loop keeps making forward progress through CRC/resync events and still delivers valid frames 19. **TryNext partial pending contract** – when data is partial, `tryNext` reports no progress while exposing partial state; after completion it drains and clears partial state 20. **Zero-length buffer handling** – empty input edge case +21. **Buffer mode: CRC failure counters** – a CRC-failed frame consumed via `add_data` increments `cnt_crc_failures`, `cnt_failed_bytes` and `cnt_sync_recoveries` (same counter semantics as stream mode) +22. **Buffer mode: Sequence gap counted** – sequence gaps are detected on frames consumed via `add_data`, matching stream-mode behaviour +23. **Diagnostics: CRC failure counter** – `cnt_crc_failures` increments on a stream-mode CRC failure +24. **Diagnostics: Length error counter** – `cnt_len_errors` increments when the header length is outside the `[min_size, size]` range for the message +25. **Diagnostics: Reset diagnostics** – `reset_diagnostics()` clears all counters +26. **Diagnostics: Sequence gap counter** – `cnt_seq_gaps` increments when a sequence number is skipped (Network profile) +27. **Diagnostics: Sync recovery counter** – `cnt_sync_recoveries` increments when garbage bytes force a resync +28. **IPC buffer: unknown msg_id advances one byte** – on the None-header (IPC) profile an unknown msg_id advances exactly one byte (SyncRecovery) and the following valid frame is still delivered +29. **Sensor buffer: unknown msg_id resync** – on the Tiny-header (Sensor) profile an unknown msg_id triggers a scan to the next start byte instead of discarding the rest of the buffer +30. **Split sweep: two frames at every boundary** – two back-to-back frames are delivered intact when the stream is split into two `add_data` chunks at *every* possible offset +31. **Streaming: two frames byte-by-byte** – two back-to-back frames are both decoded in byte-at-a-time mode ### `tryNext` Contract (Unified) diff --git a/tests/api_stability/nuget/PublicAPI.Shipped.txt b/tests/api_stability/nuget/PublicAPI.Shipped.txt index fbeb3b31..db78b384 100644 --- a/tests/api_stability/nuget/PublicAPI.Shipped.txt +++ b/tests/api_stability/nuget/PublicAPI.Shipped.txt @@ -430,6 +430,7 @@ Framework/Framing/BufferReader.cs:Remaining Framework/Framing/BufferReader.cs:Reset Framework/Framing/BufferReader.cs:SetBuffer Framework/Framing/BufferReader.cs:SetBuffer +Framework/Framing/BufferReader.cs:TryNext Framework/Framing/BufferWriter.cs:BufferWriter Framework/Framing/BufferWriter.cs:BufferWriter Framework/Framing/BufferWriter.cs:Remaining @@ -601,6 +602,7 @@ Framework/Types/IStructFrameMessage.cs:IStructFrameMessage Framework/Types/MessageInfo.cs:BaseSize Framework/Types/MessageInfo.cs:Magic1 Framework/Types/MessageInfo.cs:Magic2 +Framework/Types/MessageInfo.cs:MinSize Framework/Types/MessageInfo.cs:Size Framework/Types/MessageInfo.cs:struct Framework/Types/ParserDiagnostics.cs:CntCrcFailures diff --git a/tests/c/test_negative.c b/tests/c/test_negative.c index 012e19eb..756d3bf4 100644 --- a/tests/c/test_negative.c +++ b/tests/c/test_negative.c @@ -1060,6 +1060,307 @@ bool test_try_next_partial_pending_contract(void) { return !accumulating_reader_has_more(&reader); } +/* Helper: encode one Standard-profile frame; returns frame size. */ +static size_t encode_standard_frame(buffer_writer_t* writer) { + SerializationTestBasicTypesMessage msg; + create_test_message(&msg); + uint8_t payload[256]; + size_t payload_size = SerializationTestBasicTypesMessage_serialize(&msg, payload); + return buffer_writer_write(writer, SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID, + payload, payload_size, 0, 0, 0, 0, + SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MAGIC1, + SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MAGIC2); +} + +/* Helper: encode one Network-profile frame with the given sequence number. */ +static size_t encode_network_frame(buffer_writer_t* writer, uint8_t seq) { + SerializationTestBasicTypesMessage msg; + create_test_message(&msg); + uint8_t payload[256]; + size_t payload_size = SerializationTestBasicTypesMessage_serialize(&msg, payload); + return buffer_writer_write(writer, SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID, + payload, payload_size, seq, 1, 1, 0, + SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MAGIC1, + SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MAGIC2); +} + +/** + * Diagnostics: cnt_crc_failures increments on a stream-mode CRC failure. + */ +bool test_diagnostic_crc_failure(void) { + uint8_t buffer[1024]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + if (frame_size < 4) return false; + + buffer[frame_size - 1] ^= 0xFF; + buffer[frame_size - 2] ^= 0xFF; + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + for (size_t i = 0; i < frame_size; i++) { + accumulating_reader_push_byte(&reader, buffer[i]); + } + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return diag.cnt_crc_failures == 1 && diag.cnt_sync_recoveries >= 1; +} + +/** + * Diagnostics: cnt_sync_recoveries increments when garbage bytes are fed. + */ +bool test_diagnostic_sync_recovery(void) { + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + accumulating_reader_push_byte(&reader, 0x90); /* valid start1 */ + accumulating_reader_push_byte(&reader, 0xAB); /* invalid start2 -> sync recovery */ + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return diag.cnt_sync_recoveries >= 1; +} + +/** + * Diagnostics: cnt_len_errors increments when the header length field does not + * match the expected message size range. + */ +bool test_diagnostic_len_error(void) { + uint8_t buffer[1024]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + if (frame_size < 5) return false; + + /* ProfileStandard header: [0x90][0x71][LEN][MSG_ID]... Set LEN one too large. */ + buffer[2] = (uint8_t)(buffer[2] + 1); + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + for (size_t i = 0; i < frame_size; i++) { + accumulating_reader_push_byte(&reader, buffer[i]); + } + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return diag.cnt_len_errors == 1; +} + +/** + * Diagnostics: cnt_seq_gaps increments when a sequence number is skipped. + */ +bool test_diagnostic_seq_gap(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_NETWORK_CONFIG, buffer, sizeof(buffer)); + size_t frame0_size = encode_network_frame(&writer, 0); + size_t total = frame0_size + encode_network_frame(&writer, 5); /* skips seq 1-4 */ + if (total <= frame0_size) return false; + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_NETWORK_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + for (size_t i = 0; i < total; i++) { + accumulating_reader_push_byte(&reader, buffer[i]); + } + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0; +} + +/** + * Diagnostics: accumulating_reader_reset_diagnostics clears all counters. + */ +bool test_diagnostic_reset(void) { + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + accumulating_reader_push_byte(&reader, 0x90); + accumulating_reader_push_byte(&reader, 0xAB); + if (accumulating_reader_diagnostics(&reader).cnt_sync_recoveries < 1) return false; + + accumulating_reader_reset_diagnostics(&reader); + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return diag.cnt_crc_failures == 0 && diag.cnt_sync_recoveries == 0 && + diag.cnt_failed_bytes == 0 && diag.cnt_len_errors == 0 && diag.cnt_seq_gaps == 0; +} + +/** + * Buffer mode: a CRC-failed frame increments cnt_crc_failures, cnt_failed_bytes + * and cnt_sync_recoveries — same counter semantics as stream mode. + */ +bool test_buffer_mode_crc_counters(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + size_t total = frame_size + encode_standard_frame(&writer); + if (total <= frame_size) return false; + + buffer[frame_size - 1] ^= 0xFF; /* corrupt frame 1's CRC */ + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + accumulating_reader_add_data(&reader, buffer, total); + + int valid_count = 0; + frame_msg_info_t f; + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + } + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return valid_count == 1 && diag.cnt_crc_failures == 1 && + diag.cnt_sync_recoveries == 1 && diag.cnt_failed_bytes == (uint32_t)frame_size; +} + +/** + * Buffer mode: sequence gaps are detected on frames consumed via add_data. + */ +bool test_buffer_mode_seq_gap(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_NETWORK_CONFIG, buffer, sizeof(buffer)); + size_t frame0_size = encode_network_frame(&writer, 0); + size_t total = frame0_size + encode_network_frame(&writer, 5); /* skips seq 1-4 */ + if (total <= frame0_size) return false; + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_NETWORK_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + accumulating_reader_add_data(&reader, buffer, total); + + int valid_count = 0; + frame_msg_info_t f; + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + } + + frame_parser_diagnostics_t diag = accumulating_reader_diagnostics(&reader); + return valid_count == 2 && diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0; +} + +/** + * Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + * triggers a resync scan to the next start byte instead of discarding the buffer. + */ +bool test_sensor_buffer_unknown_msg_id_resync(void) { + message_info_t info; + if (!get_message_info(SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID, &info)) return false; + + uint8_t data[600]; + memset(data, 0, sizeof(data)); + /* [0x70][0xFF (unknown)] then a valid sensor frame [0x70][msg_id][payload@size] */ + data[0] = 0x70; + data[1] = 0xFF; + data[2] = 0x70; + data[3] = SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID & 0xFF; + size_t total = 4 + info.size; /* zeroed payload is fine for framing validity */ + if (total > sizeof(data)) return false; + + buffer_reader_t reader; + buffer_reader_init(&reader, &PROFILE_SENSOR_CONFIG, data, total, get_message_info); + + bool saw_sync = false; + int valid_count = 0; + frame_msg_info_t f; + while (buffer_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + else if (f.status == FRAME_MSG_STATUS_SYNC_RECOVERY) saw_sync = true; + } + return saw_sync && valid_count == 1; +} + +/** + * IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + * following valid frame is still delivered. + */ +bool test_ipc_buffer_unknown_msg_id(void) { + message_info_t info; + if (!get_message_info(SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID, &info)) return false; + + uint8_t data[600]; + memset(data, 0, sizeof(data)); + data[0] = 0xFF; /* unknown msg_id */ + data[1] = SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID & 0xFF; + size_t total = 2 + info.size; + if (total > sizeof(data)) return false; + + buffer_reader_t reader; + buffer_reader_init(&reader, &PROFILE_IPC_CONFIG, data, total, get_message_info); + + bool saw_sync = false; + int valid_count = 0; + frame_msg_info_t f; + while (buffer_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + else if (f.status == FRAME_MSG_STATUS_SYNC_RECOVERY) saw_sync = true; + } + return saw_sync && valid_count == 1; +} + +/** + * Split sweep: two back-to-back frames delivered intact when the byte stream is + * split into two add_data chunks at every possible offset. + */ +bool test_split_sweep_all_boundaries(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + size_t total = frame_size + encode_standard_frame(&writer); + if (total <= frame_size) return false; + + for (size_t split = 1; split < total; split++) { + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + int valid_count = 0; + frame_msg_info_t f; + + accumulating_reader_add_data(&reader, buffer, split); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + } + accumulating_reader_add_data(&reader, buffer + split, total - split); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + } + + if (valid_count != 2) return false; + if (accumulating_reader_has_partial(&reader)) return false; + } + return true; +} + +/** + * Streaming: two back-to-back frames are both decoded byte-by-byte. + */ +bool test_streaming_two_frames(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + size_t total = frame_size + encode_standard_frame(&writer); + if (total <= frame_size) return false; + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + int valid_count = 0; + for (size_t i = 0; i < total; i++) { + frame_msg_info_t r = accumulating_reader_push_byte(&reader, buffer[i]); + if (r.valid) valid_count++; + } + return valid_count == 2; +} + // Test function pointer type typedef bool (*TestFunc)(void); @@ -1076,6 +1377,8 @@ int main(void) { // Define test matrix TestCase tests[] = { + {"Buffer mode: CRC failure counters", test_buffer_mode_crc_counters}, + {"Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap}, {"Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure}, {"Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure}, {"Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc}, @@ -1084,20 +1387,29 @@ int main(void) { {"Corrupted CRC detection", test_corrupted_crc}, {"Corrupted length field detection", test_corrupted_length}, {"Cross-package message rejection", test_cross_package_rejection}, + {"Diagnostics: CRC failure counter", test_diagnostic_crc_failure}, + {"Diagnostics: Length error counter", test_diagnostic_len_error}, + {"Diagnostics: Reset diagnostics", test_diagnostic_reset}, + {"Diagnostics: Sequence gap counter", test_diagnostic_seq_gap}, + {"Diagnostics: Sync recovery counter", test_diagnostic_sync_recovery}, {"Invalid message ID rejection", test_invalid_msg_id}, {"Invalid start bytes detection", test_invalid_start_bytes}, + {"IPC buffer: unknown msg_id advances one byte", test_ipc_buffer_unknown_msg_id}, {"Minimal profile: Truncated frame", test_minimal_profile_truncated_frame}, {"Multiple frames: CRC error then valid frame", test_crc_error_then_valid_frame}, {"Multiple frames: Corrupted middle frame", test_multiple_corrupted_frames}, {"Network profile: Corrupted pkg_id", test_network_corrupted_pkg_id}, {"Network profile: SysId/CompId corruption", test_network_sysid_compid}, {"Partial frame across buffer boundary", test_partial_frame_boundary}, + {"Sensor buffer: unknown msg_id resync", test_sensor_buffer_unknown_msg_id_resync}, + {"Split sweep: two frames at every boundary", test_split_sweep_all_boundaries}, {"Split-buffer: CRC error status preserved", test_split_buffer_crc_error_status}, {"TryNext drain: CRC/resync + valid", test_try_next_drain_contract}, {"TryNext partial pending contract", test_try_next_partial_pending_contract}, {"Stream mode: recovers after garbage prefix", test_stream_recovers_after_garbage}, {"Streaming: Corrupted CRC detection", test_streaming_corrupted_crc}, {"Streaming: Garbage data handling", test_streaming_garbage}, + {"Streaming: two frames byte-by-byte", test_streaming_two_frames}, {"Truncated frame detection", test_truncated_frame}, {"Zero-length buffer handling", test_zero_length_buffer} }; diff --git a/tests/coverage_spec.py b/tests/coverage_spec.py index 9a85583c..f59a7178 100644 --- a/tests/coverage_spec.py +++ b/tests/coverage_spec.py @@ -404,13 +404,15 @@ def _full(label, symbol, notes=""): "intro": ( "Test files: `tests/{c,cpp,py,ts,js,csharp,rust}/test_negative.*`\n\n" "See `tests/NEGATIVE_TESTS.md` for full scenario descriptions.\n\n" - "The 20 scenarios in the table below are registered in every " - "language's `test_negative.*` file. Individual languages carry " - "additional language-specific scenarios: C/C++/TS/JS/C# (24 each) " - "add bulk `pkg_id`/`msg_id` corruption, cross-package rejection, and " - "network `pkg_id` corruption; Python (34) adds those plus " - "diagnostic-counter and status-machine tests; Rust (20) currently " - "matches the uniform set." + "The 31 scenarios in the table below are registered in every " + "language's `test_negative.*` file, covering corruption handling, " + "the `tryNext` drain contract, diagnostic counters (unified " + "semantics in buffer and stream mode), minimal-profile resync, and " + "a chunk-boundary split sweep. All seven languages additionally " + "carry the four package-corruption scenarios (bulk " + "`pkg_id`/`msg_id` corruption, cross-package rejection, network " + "`pkg_id` corruption) for 35 scenarios each; Python (40) adds " + "status-machine and buffer-mode diagnostic extras." ), "tables": [ { @@ -418,22 +420,33 @@ def _full(label, symbol, notes=""): "columns": LANGS, "lang_cols": LANGS, "rows": [_full(s, "✅") for s in ( + "Buffer mode: CRC failure counters", + "Buffer mode: Sequence gap counted", "Buffer mode: recovers after CRC failure", "Buffer reader: skips CRC-failed frame", "Bulk profile: Corrupted CRC", "Corrupted CRC detection", "Corrupted length field detection", + "Diagnostics: CRC failure counter", + "Diagnostics: Length error counter", + "Diagnostics: Reset diagnostics", + "Diagnostics: Sequence gap counter", + "Diagnostics: Sync recovery counter", "Invalid message ID rejection", "Invalid start bytes detection", + "IPC buffer: unknown msg_id advances one byte", "Minimal profile: Truncated frame", "Multiple frames: CRC error then valid frame", "Multiple frames: Corrupted middle frame", "Network profile: SysId/CompId corruption", "Partial frame across buffer boundary", + "Sensor buffer: unknown msg_id resync", + "Split sweep: two frames at every boundary", "Split-buffer: CRC error status preserved", "Stream mode: recovers after garbage prefix", "Streaming: Corrupted CRC detection", "Streaming: Garbage data handling", + "Streaming: two frames byte-by-byte", "TryNext drain: CRC/resync + valid", "TryNext partial pending contract", "Truncated frame detection", diff --git a/tests/cpp/test_negative.cpp b/tests/cpp/test_negative.cpp index cede5b70..5cabafc6 100644 --- a/tests/cpp/test_negative.cpp +++ b/tests/cpp/test_negative.cpp @@ -776,6 +776,275 @@ bool test_split_buffer_crc_error_status() { return true; } +// Helper: encode one Standard-profile frame (message index 0); returns bytes written. +static size_t encode_standard_frame(BufferWriter& writer) { + auto msg = StandardMessages::get_message(0); + size_t written = 0; + std::visit([&writer, &written](auto&& m) { written = writer.write(m); }, msg); + return written; +} + +// Helper: encode one Network-profile frame with the given sequence number. +static size_t encode_network_frame(BufferWriter& writer, uint8_t seq) { + auto msg = StandardMessages::get_message(0); + size_t written = 0; + std::visit([&writer, &written, seq](auto&& m) { written = writer.write(m, seq, 1, 1); }, msg); + return written; +} + +// Helper: MSG_ID of the message used by the minimal-profile tests. +static uint16_t standard_message0_id() { + auto msg = StandardMessages::get_message(0); + uint16_t id = 0; + std::visit([&id](auto&& m) { id = static_cast(std::decay_t::MSG_ID); }, msg); + return id; +} + +/** + * Diagnostics: cnt_crc_failures increments on a stream-mode CRC failure. + */ +bool test_diagnostic_crc_failure() { + std::vector buffer(1024); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + if (frame_size < 4) return false; + + buffer[frame_size - 1] ^= 0xFF; + buffer[frame_size - 2] ^= 0xFF; + + AccumulatingReader reader(get_message_info); + for (size_t i = 0; i < frame_size; i++) { + reader.push_byte(buffer[i]); + } + + auto diag = reader.diagnostics(); + return diag.cnt_crc_failures == 1 && diag.cnt_sync_recoveries >= 1; +} + +/** + * Diagnostics: cnt_sync_recoveries increments when garbage bytes are fed. + */ +bool test_diagnostic_sync_recovery() { + AccumulatingReader reader(get_message_info); + + reader.push_byte(0x90); // valid start1 + reader.push_byte(0xAB); // invalid start2 -> sync recovery + + return reader.diagnostics().cnt_sync_recoveries >= 1; +} + +/** + * Diagnostics: cnt_len_errors increments when the header length field is out of + * the [min_size, size] range for the message. Feeding just the header is enough — + * the check fires at header completion. + */ +bool test_diagnostic_len_error() { + uint16_t msg_id = standard_message0_id(); + auto info = get_message_info(msg_id); + if (!info || info.size + 1 > 255) return false; + + AccumulatingReader reader(get_message_info); + reader.push_byte(0x90); + reader.push_byte(0x71); + reader.push_byte(static_cast(info.size + 1)); // out of range + reader.push_byte(static_cast(msg_id & 0xFF)); + + return reader.diagnostics().cnt_len_errors == 1; +} + +/** + * Diagnostics: cnt_seq_gaps increments when a sequence number is skipped. + */ +bool test_diagnostic_seq_gap() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame0_size = encode_network_frame(writer, 0); + size_t total = frame0_size + encode_network_frame(writer, 5); // skips seq 1-4 + if (total <= frame0_size) return false; + + AccumulatingReader reader(get_message_info); + for (size_t i = 0; i < total; i++) { + reader.push_byte(buffer[i]); + } + + auto diag = reader.diagnostics(); + return diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0; +} + +/** + * Diagnostics: reset_diagnostics() clears all counters. + */ +bool test_diagnostic_reset() { + AccumulatingReader reader(get_message_info); + + reader.push_byte(0x90); + reader.push_byte(0xAB); + if (reader.diagnostics().cnt_sync_recoveries < 1) return false; + + reader.reset_diagnostics(); + auto diag = reader.diagnostics(); + return diag.cnt_crc_failures == 0 && diag.cnt_sync_recoveries == 0 && + diag.cnt_failed_bytes == 0 && diag.cnt_len_errors == 0 && diag.cnt_seq_gaps == 0; +} + +/** + * Buffer mode: a CRC-failed frame increments cnt_crc_failures, cnt_failed_bytes + * and cnt_sync_recoveries — same counter semantics as stream mode. + */ +bool test_buffer_mode_crc_counters() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + size_t total = frame_size + encode_standard_frame(writer); + if (total <= frame_size) return false; + + buffer[frame_size - 1] ^= 0xFF; // corrupt frame 1's CRC + + AccumulatingReader reader(get_message_info); + reader.add_data(buffer.data(), total); + + int valid_count = 0; + FrameMsgInfo f; + while (reader.try_next(f)) { + if (f.valid) valid_count++; + } + + auto diag = reader.diagnostics(); + return valid_count == 1 && diag.cnt_crc_failures == 1 && + diag.cnt_sync_recoveries == 1 && diag.cnt_failed_bytes == static_cast(frame_size); +} + +/** + * Buffer mode: sequence gaps are detected on frames consumed via add_data. + */ +bool test_buffer_mode_seq_gap() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame0_size = encode_network_frame(writer, 0); + size_t total = frame0_size + encode_network_frame(writer, 5); // skips seq 1-4 + if (total <= frame0_size) return false; + + AccumulatingReader reader(get_message_info); + reader.add_data(buffer.data(), total); + + int valid_count = 0; + FrameMsgInfo f; + while (reader.try_next(f)) { + if (f.valid) valid_count++; + } + + auto diag = reader.diagnostics(); + return valid_count == 2 && diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0; +} + +/** + * Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + * triggers a resync scan to the next start byte instead of stalling or + * discarding the rest of the buffer. + */ +bool test_sensor_buffer_unknown_msg_id_resync() { + uint16_t msg_id = standard_message0_id(); + auto info = get_message_info(msg_id); + if (!info) return false; + + std::vector data(4 + info.size, 0); + data[0] = 0x70; + data[1] = 0xFF; // unknown msg_id + data[2] = 0x70; + data[3] = static_cast(msg_id & 0xFF); + // zeroed payload is fine for framing validity + + BufferReader reader( + data.data(), data.size(), get_message_info); + + bool saw_sync = false; + int valid_count = 0; + FrameMsgInfo f; + while (reader.try_next(f)) { + if (f.valid) valid_count++; + else if (f.status == FrameMsgStatus::SyncRecovery) saw_sync = true; + } + return saw_sync && valid_count == 1; +} + +/** + * IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + * following valid frame is still delivered. + */ +bool test_ipc_buffer_unknown_msg_id() { + uint16_t msg_id = standard_message0_id(); + auto info = get_message_info(msg_id); + if (!info) return false; + + std::vector data(2 + info.size, 0); + data[0] = 0xFF; // unknown msg_id + data[1] = static_cast(msg_id & 0xFF); + + BufferReader reader( + data.data(), data.size(), get_message_info); + + bool saw_sync = false; + int valid_count = 0; + FrameMsgInfo f; + while (reader.try_next(f)) { + if (f.valid) valid_count++; + else if (f.status == FrameMsgStatus::SyncRecovery) saw_sync = true; + } + return saw_sync && valid_count == 1; +} + +/** + * Split sweep: two back-to-back frames delivered intact when the byte stream is + * split into two add_data chunks at every possible offset. + */ +bool test_split_sweep_all_boundaries() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + size_t total = frame_size + encode_standard_frame(writer); + if (total <= frame_size) return false; + + for (size_t split = 1; split < total; split++) { + AccumulatingReader reader(get_message_info); + + int valid_count = 0; + FrameMsgInfo f; + + reader.add_data(buffer.data(), split); + while (reader.try_next(f)) { + if (f.valid) valid_count++; + } + reader.add_data(buffer.data() + split, total - split); + while (reader.try_next(f)) { + if (f.valid) valid_count++; + } + + if (valid_count != 2) return false; + if (reader.has_partial()) return false; + } + return true; +} + +/** + * Streaming: two back-to-back frames are both decoded byte-by-byte. + */ +bool test_streaming_two_frames() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + size_t total = frame_size + encode_standard_frame(writer); + if (total <= frame_size) return false; + + AccumulatingReader reader(get_message_info); + + int valid_count = 0; + for (size_t i = 0; i < total; i++) { + auto r = reader.push_byte(buffer[i]); + if (r.valid) valid_count++; + } + return valid_count == 2; +} + // Test function pointer type typedef bool (*TestFunc)(); @@ -792,6 +1061,8 @@ int main() { // Define test matrix TestCase tests[] = { + {"Buffer mode: CRC failure counters", test_buffer_mode_crc_counters}, + {"Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap}, {"Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure}, {"Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure}, {"Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc}, @@ -800,20 +1071,29 @@ int main() { {"Corrupted CRC detection", test_corrupted_crc}, {"Corrupted length field detection", test_corrupted_length}, {"Cross-package message rejection", test_cross_package_rejection}, + {"Diagnostics: CRC failure counter", test_diagnostic_crc_failure}, + {"Diagnostics: Length error counter", test_diagnostic_len_error}, + {"Diagnostics: Reset diagnostics", test_diagnostic_reset}, + {"Diagnostics: Sequence gap counter", test_diagnostic_seq_gap}, + {"Diagnostics: Sync recovery counter", test_diagnostic_sync_recovery}, {"Invalid message ID rejection", test_invalid_msg_id}, {"Invalid start bytes detection", test_invalid_start_bytes}, + {"IPC buffer: unknown msg_id advances one byte", test_ipc_buffer_unknown_msg_id}, {"Minimal profile: Truncated frame", test_minimal_profile_truncated_frame}, {"Multiple frames: CRC error then valid frame", test_crc_error_then_valid_frame}, {"Multiple frames: Corrupted middle frame", test_multiple_corrupted_frames}, {"Network profile: Corrupted pkg_id", test_network_corrupted_pkg_id}, {"Network profile: SysId/CompId corruption", test_network_sysid_compid}, {"Partial frame across buffer boundary", test_partial_frame_boundary}, + {"Sensor buffer: unknown msg_id resync", test_sensor_buffer_unknown_msg_id_resync}, + {"Split sweep: two frames at every boundary", test_split_sweep_all_boundaries}, {"Split-buffer: CRC error status preserved", test_split_buffer_crc_error_status}, {"TryNext drain: CRC/resync + valid", test_try_next_drain_contract}, {"TryNext partial pending contract", test_try_next_partial_pending_contract}, {"Stream mode: recovers after garbage prefix", test_stream_recovers_after_garbage}, {"Streaming: Corrupted CRC detection", test_streaming_corrupted_crc}, {"Streaming: Garbage data handling", test_streaming_garbage_data}, + {"Streaming: two frames byte-by-byte", test_streaming_two_frames}, {"Truncated frame detection", test_truncated_frame}, {"Zero-length buffer handling", test_zero_length_buffer} }; diff --git a/tests/cpp/test_sdk_headers_compile.cpp b/tests/cpp/test_sdk_headers_compile.cpp new file mode 100644 index 00000000..b5a49e25 --- /dev/null +++ b/tests/cpp/test_sdk_headers_compile.cpp @@ -0,0 +1,23 @@ +/** + * Compile-only test: the public C++ SDK umbrella header must compile standalone. + * + * The umbrella headers (sdk.hpp / sdk_embedded.hpp) are the documented entry + * points for SDK users, but no runtime test includes them — which once let + * serial_transport.hpp ship in an uncompilable state. This TU instantiates the + * embedded umbrella (transport + serial_transport + observer + struct_frame_sdk; + * the full sdk.hpp additionally needs the vendored ASIO include path, so it is + * exercised separately by user builds). + */ + +#include "struct_frame_sdk/sdk_embedded.hpp" + +int main() { + // Touch a couple of symbols so the headers are actually instantiated. + structframe::sdk::TransportConfig config; + (void)config; + + structframe::sdk::BaseTransport transport; + (void)transport.IsConnected(); + + return 0; +} diff --git a/tests/csharp/TestNegative.cs b/tests/csharp/TestNegative.cs index ae5af4b6..71c98275 100644 --- a/tests/csharp/TestNegative.cs +++ b/tests/csharp/TestNegative.cs @@ -846,6 +846,266 @@ private static bool TestTryNextPartialPendingContract() return !reader.HasMore; } + + /** Helper: encode `count` Standard-profile frames into buffer; returns per-frame sizes. */ + private static int[] EncodeStandardFrames(byte[] buffer, int count) + { + var writer = new BufferWriter(); + writer.SetBuffer(buffer); + var sizes = new int[count + 1]; // last entry = total + int prev = 0; + for (int i = 0; i < count; i++) + { + writer.Write(CreateTestMessage()); + sizes[i] = writer.Size - prev; + prev = writer.Size; + } + sizes[count] = writer.Size; + return sizes; + } + + /** Helper: encode one Network-profile frame with the given sequence number. */ + private static byte[] EncodeNetworkFrame(byte seq) + { + byte[] buffer = new byte[1024]; + var writer = new BufferWriter(); + writer.SetBuffer(buffer); + writer.Write(CreateTestMessage(), seq: seq, sysId: 1, compId: 1); + var frame = new byte[writer.Size]; + Array.Copy(buffer, frame, writer.Size); + return frame; + } + + /** Diagnostics: CntCrcFailures increments on a stream-mode CRC failure. */ + private static bool TestDiagnosticCrcFailure() + { + byte[] buffer = new byte[1024]; + var sizes = EncodeStandardFrames(buffer, 1); + int frameSize = sizes[0]; + if (frameSize < 4) return false; + + buffer[frameSize - 1] ^= 0xFF; + buffer[frameSize - 2] ^= 0xFF; + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + for (int i = 0; i < frameSize; i++) + { + reader.PushByte(buffer[i]); + } + + var diag = reader.Diagnostics; + return diag.CntCrcFailures == 1 && diag.CntSyncRecoveries >= 1; + } + + /** Diagnostics: CntSyncRecoveries increments when garbage bytes are fed. */ + private static bool TestDiagnosticSyncRecovery() + { + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + reader.PushByte(0x90); // valid start1 + reader.PushByte(0xAB); // invalid start2 -> sync recovery + return reader.Diagnostics.CntSyncRecoveries >= 1; + } + + /** + * Diagnostics: CntLenErrors increments when the header length field is out of + * the [MinSize, Size] range. Feeding just the header is enough — the check + * fires at header completion. + */ + private static bool TestDiagnosticLenError() + { + var info = SerializationTestMD.GetMessageInfo(BasicTypesMessage.MsgId); + if (!info.HasValue || info.Value.Size + 1 > 255) return false; + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + reader.PushByte(0x90); + reader.PushByte(0x71); + reader.PushByte((byte)(info.Value.Size + 1)); // out of range + reader.PushByte((byte)(BasicTypesMessage.MsgId & 0xFF)); + + return reader.Diagnostics.CntLenErrors == 1; + } + + /** Diagnostics: CntSeqGaps increments when a sequence number is skipped. */ + private static bool TestDiagnosticSeqGap() + { + var frame0 = EncodeNetworkFrame(0); + var frame5 = EncodeNetworkFrame(5); // skips seq 1-4 + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + foreach (var b in frame0) reader.PushByte(b); + foreach (var b in frame5) reader.PushByte(b); + + var diag = reader.Diagnostics; + return diag.CntSeqGaps == 1 && diag.CntCrcFailures == 0; + } + + /** Diagnostics: ResetDiagnostics() clears all counters. */ + private static bool TestDiagnosticReset() + { + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + reader.PushByte(0x90); + reader.PushByte(0xAB); + if (reader.Diagnostics.CntSyncRecoveries < 1) return false; + + reader.ResetDiagnostics(); + var diag = reader.Diagnostics; + return diag.CntCrcFailures == 0 && diag.CntSyncRecoveries == 0 && + diag.CntFailedBytes == 0 && diag.CntLenErrors == 0 && diag.CntSeqGaps == 0; + } + + /** + * Buffer mode: a CRC-failed frame increments CntCrcFailures, CntFailedBytes + * and CntSyncRecoveries — same counter semantics as stream mode. + */ + private static bool TestBufferModeCrcCounters() + { + byte[] buffer = new byte[2048]; + var sizes = EncodeStandardFrames(buffer, 2); + int frameSize = sizes[0]; + int total = sizes[2]; + + buffer[frameSize - 1] ^= 0xFF; // corrupt frame 1's CRC + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + reader.AddData(buffer, 0, total); + + int validCount = 0; + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + } + + var diag = reader.Diagnostics; + return validCount == 1 && diag.CntCrcFailures == 1 && + diag.CntSyncRecoveries == 1 && diag.CntFailedBytes == frameSize; + } + + /** Buffer mode: sequence gaps are detected on frames consumed via AddData. */ + private static bool TestBufferModeSeqGap() + { + var frame0 = EncodeNetworkFrame(0); + var frame5 = EncodeNetworkFrame(5); // skips seq 1-4 + var data = new byte[frame0.Length + frame5.Length]; + Array.Copy(frame0, 0, data, 0, frame0.Length); + Array.Copy(frame5, 0, data, frame0.Length, frame5.Length); + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + reader.AddData(data); + + int validCount = 0; + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + } + + var diag = reader.Diagnostics; + return validCount == 2 && diag.CntSeqGaps == 1 && diag.CntCrcFailures == 0; + } + + /** + * Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + * triggers a resync scan to the next start byte instead of discarding the buffer. + */ + private static bool TestSensorBufferUnknownMsgIdResync() + { + var info = SerializationTestMD.GetMessageInfo(BasicTypesMessage.MsgId); + if (!info.HasValue) return false; + + var data = new byte[4 + info.Value.Size]; // zeroed payload is fine for framing + data[0] = 0x70; + data[1] = 0xFF; // unknown msg_id + data[2] = 0x70; + data[3] = (byte)(BasicTypesMessage.MsgId & 0xFF); + + var reader = new BufferReader(SerializationTestMD.GetMessageInfo); + reader.SetBuffer(data, 0, data.Length); + + bool sawSync = false; + int validCount = 0; + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + else if (f.Status == FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount == 1; + } + + /** + * IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + * following valid frame is still delivered. + */ + private static bool TestIpcBufferUnknownMsgId() + { + var info = SerializationTestMD.GetMessageInfo(BasicTypesMessage.MsgId); + if (!info.HasValue) return false; + + var data = new byte[2 + info.Value.Size]; + data[0] = 0xFF; // unknown msg_id + data[1] = (byte)(BasicTypesMessage.MsgId & 0xFF); + + var reader = new BufferReader(SerializationTestMD.GetMessageInfo); + reader.SetBuffer(data, 0, data.Length); + + bool sawSync = false; + int validCount = 0; + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + else if (f.Status == FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount == 1; + } + + /** + * Split sweep: two back-to-back frames delivered intact when the byte stream + * is split into two AddData chunks at every possible offset. + */ + private static bool TestSplitSweepAllBoundaries() + { + byte[] buffer = new byte[2048]; + var sizes = EncodeStandardFrames(buffer, 2); + int total = sizes[2]; + + for (int split = 1; split < total; split++) + { + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + int validCount = 0; + + reader.AddData(buffer, 0, split); + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + } + reader.AddData(buffer, split, total - split); + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + } + + if (validCount != 2) return false; + if (reader.HasPartial) return false; + } + return true; + } + + /** Streaming: two back-to-back frames are both decoded byte-by-byte. */ + private static bool TestStreamingTwoFrames() + { + byte[] buffer = new byte[2048]; + var sizes = EncodeStandardFrames(buffer, 2); + int total = sizes[2]; + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + int validCount = 0; + for (int i = 0; i < total; i++) + { + var r = reader.PushByte(buffer[i]); + if (r.Valid) validCount++; + } + return validCount == 2; + } + + public static int Main(string[] args) { Console.WriteLine("\n========================================"); @@ -855,6 +1115,8 @@ public static int Main(string[] args) // Define test matrix var tests = new (string name, Func func)[] { + ("Buffer mode: CRC failure counters", TestBufferModeCrcCounters), + ("Buffer mode: Sequence gap counted", TestBufferModeSeqGap), ("Buffer mode: recovers after CRC failure", TestBufferModeRecoverAfterCrcFailure), ("Buffer reader: skips CRC-failed frame", TestBufferReaderSkipsCrcFailed), ("Bulk profile: Corrupted CRC", TestBulkProfileCorruptedCrc), @@ -863,20 +1125,29 @@ public static int Main(string[] args) ("Corrupted CRC detection", TestCorruptedCrc), ("Corrupted length field detection", TestCorruptedLength), ("Cross-package rejection (pkgid mismatch)", TestCrossPackageRejection), + ("Diagnostics: CRC failure counter", TestDiagnosticCrcFailure), + ("Diagnostics: Length error counter", TestDiagnosticLenError), + ("Diagnostics: Reset diagnostics", TestDiagnosticReset), + ("Diagnostics: Sequence gap counter", TestDiagnosticSeqGap), + ("Diagnostics: Sync recovery counter", TestDiagnosticSyncRecovery), ("Invalid message ID rejection", TestInvalidMsgId), ("Invalid start bytes detection", TestInvalidStartBytes), + ("IPC buffer: unknown msg_id advances one byte", TestIpcBufferUnknownMsgId), ("Minimal profile: Truncated frame", TestMinimalProfileTruncatedFrame), ("Multiple frames: CRC error then valid frame", TestCrcErrorThenValidFrame), ("Multiple frames: Corrupted middle frame", TestMultipleCorruptedFrames), ("Network profile: Corrupted pkg_id byte", TestNetworkCorruptedPkgId), ("Network profile: SysId/CompId corruption", TestNetworkSysIdCompId), ("Partial frame across buffer boundary", TestPartialFrameBoundary), + ("Sensor buffer: unknown msg_id resync", TestSensorBufferUnknownMsgIdResync), + ("Split sweep: two frames at every boundary", TestSplitSweepAllBoundaries), ("Split-buffer: CRC error status preserved", TestSplitBufferCrcErrorStatus), ("TryNext drain: CRC/resync + valid", TestTryNextDrainContract), ("TryNext partial pending contract", TestTryNextPartialPendingContract), ("Stream mode: recovers after garbage prefix", TestStreamRecoversAfterGarbage), ("Streaming: Corrupted CRC detection", TestStreamingCorruptedCrc), ("Streaming: Garbage data handling", TestStreamingGarbage), + ("Streaming: two frames byte-by-byte", TestStreamingTwoFrames), ("Truncated frame detection", TestTruncatedFrame), ("Zero-length buffer handling", TestZeroLengthBuffer), }; diff --git a/tests/js/test_negative.js b/tests/js/test_negative.js index 1564d589..44c6adc4 100644 --- a/tests/js/test_negative.js +++ b/tests/js/test_negative.js @@ -14,6 +14,7 @@ const { AccumulatingReader, ProfileStandardConfig, ProfileSensorConfig, + ProfileIPCConfig, ProfileBulkConfig, ProfileNetworkConfig, } = require('../generated/js/frame-profiles'); @@ -756,6 +757,234 @@ function testTryNextPartialPendingContract() { return !reader.hasMore(); } + +/** Helper: encode `count` Standard-profile frames; returns [buffer, per-frame sizes]. */ +function encodeStandardFrames(count) { + const writer = new BufferWriter(ProfileStandardConfig, 2048); + const sizes = []; + let prev = 0; + for (let i = 0; i < count; i++) { + writer.write(createTestMessage()); + sizes.push(writer.size - prev); + prev = writer.size; + } + return [writer.data().slice(0, writer.size), sizes]; +} + +/** Helper: encode one Network-profile frame with the given sequence number. */ +function encodeNetworkFrame(seq) { + const writer = new BufferWriter(ProfileNetworkConfig, 1024); + writer.write(createTestMessage(), { seq, sysId: 1, compId: 1 }); + return writer.data().slice(0, writer.size); +} + +/** Diagnostics: cntCrcFailures increments on a stream-mode CRC failure. */ +function testDiagnosticCrcFailure() { + const [buffer, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 4) return false; + + buffer[frameSize - 1] ^= 0xFF; + buffer[frameSize - 2] ^= 0xFF; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + for (let i = 0; i < frameSize; i++) { + reader.pushByte(buffer[i]); + } + + const diag = reader.diagnostics; + return diag.cntCrcFailures === 1 && diag.cntSyncRecoveries >= 1; +} + +/** Diagnostics: cntSyncRecoveries increments when garbage bytes are fed. */ +function testDiagnosticSyncRecovery() { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); // valid start1 + reader.pushByte(0xAB); // invalid start2 -> sync recovery + return reader.diagnostics.cntSyncRecoveries >= 1; +} + +/** + * Diagnostics: cntLenErrors increments when the header length field is out of the + * [minSize, size] range. Feeding just the header is enough — the check fires at + * header completion. + */ +function testDiagnosticLenError() { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info || info.size + 1 > 255) return false; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); + reader.pushByte(0x71); + reader.pushByte((info.size + 1) & 0xFF); // out of range + reader.pushByte(msgId & 0xFF); + + return reader.diagnostics.cntLenErrors === 1; +} + +/** Diagnostics: cntSeqGaps increments when a sequence number is skipped. */ +function testDiagnosticSeqGap() { + const frame0 = encodeNetworkFrame(0); + const frame5 = encodeNetworkFrame(5); // skips seq 1-4 + + const reader = new AccumulatingReader(ProfileNetworkConfig, getMessageInfo, 1024); + for (const b of frame0) reader.pushByte(b); + for (const b of frame5) reader.pushByte(b); + + const diag = reader.diagnostics; + return diag.cntSeqGaps === 1 && diag.cntCrcFailures === 0; +} + +/** Diagnostics: resetDiagnostics() clears all counters. */ +function testDiagnosticReset() { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); + reader.pushByte(0xAB); + if (reader.diagnostics.cntSyncRecoveries < 1) return false; + + reader.resetDiagnostics(); + const diag = reader.diagnostics; + return diag.cntCrcFailures === 0 && diag.cntSyncRecoveries === 0 && + diag.cntFailedBytes === 0 && diag.cntLenErrors === 0 && diag.cntSeqGaps === 0; +} + +/** + * Buffer mode: a CRC-failed frame increments cntCrcFailures, cntFailedBytes and + * cntSyncRecoveries — same counter semantics as stream mode. + */ +function testBufferModeCrcCounters() { + const [buffer, sizes] = encodeStandardFrames(2); + const frameSize = sizes[0]; + buffer[frameSize - 1] ^= 0xFF; // corrupt frame 1's CRC + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.addData(buffer); + + let validCount = 0; + let r; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + const diag = reader.diagnostics; + return validCount === 1 && diag.cntCrcFailures === 1 && + diag.cntSyncRecoveries === 1 && diag.cntFailedBytes === frameSize; +} + +/** Buffer mode: sequence gaps are detected on frames consumed via addData. */ +function testBufferModeSeqGap() { + const frame0 = encodeNetworkFrame(0); + const frame5 = encodeNetworkFrame(5); // skips seq 1-4 + const data = new Uint8Array(frame0.length + frame5.length); + data.set(frame0, 0); + data.set(frame5, frame0.length); + + const reader = new AccumulatingReader(ProfileNetworkConfig, getMessageInfo, 1024); + reader.addData(data); + + let validCount = 0; + let r; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + const diag = reader.diagnostics; + return validCount === 2 && diag.cntSeqGaps === 1 && diag.cntCrcFailures === 0; +} + +/** + * Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + * triggers a resync scan to the next start byte instead of discarding the buffer. + */ +function testSensorBufferUnknownMsgIdResync() { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info) return false; + + const data = new Uint8Array(4 + info.size); // zeroed payload is fine for framing + data[0] = 0x70; + data[1] = 0xFF; // unknown msg_id + data[2] = 0x70; + data[3] = msgId & 0xFF; + + const reader = new BufferReader(ProfileSensorConfig, data, getMessageInfo); + let sawSync = false; + let validCount = 0; + let r; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount === 1; +} + +/** + * IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + * following valid frame is still delivered. + */ +function testIpcBufferUnknownMsgId() { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info) return false; + + const data = new Uint8Array(2 + info.size); + data[0] = 0xFF; // unknown msg_id + data[1] = msgId & 0xFF; + + const reader = new BufferReader(ProfileIPCConfig, data, getMessageInfo); + let sawSync = false; + let validCount = 0; + let r; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount === 1; +} + +/** + * Split sweep: two back-to-back frames delivered intact when the byte stream is + * split into two addData chunks at every possible offset. + */ +function testSplitSweepAllBoundaries() { + const [buffer] = encodeStandardFrames(2); + const total = buffer.length; + + for (let split = 1; split < total; split++) { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + let r; + + reader.addData(buffer.slice(0, split)); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + reader.addData(buffer.slice(split)); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + if (validCount !== 2) return false; + if (reader.hasPartial()) return false; + } + return true; +} + +/** Streaming: two back-to-back frames are both decoded byte-by-byte. */ +function testStreamingTwoFrames() { + const [buffer] = encodeStandardFrames(2); + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + for (const b of buffer) { + const r = reader.pushByte(b); + if (r.valid) validCount++; + } + return validCount === 2; +} + + function main() { console.log('\n========================================'); console.log('NEGATIVE TESTS - JavaScript Parser'); @@ -763,6 +992,8 @@ function main() { // Define test matrix const tests = [ + ['Buffer mode: CRC failure counters', testBufferModeCrcCounters], + ['Buffer mode: Sequence gap counted', testBufferModeSeqGap], ['Buffer mode: recovers after CRC failure', testBufferModeRecoversAfterCrcFailure], ['Buffer reader: skips CRC-failed frame', testBufferReaderSkipsCrcFailure], ['Bulk profile: Corrupted CRC', testBulkProfileCorruptedCrc], @@ -771,8 +1002,14 @@ function main() { ['Corrupted CRC detection', testCorruptedCrc], ['Corrupted length field detection', testCorruptedLength], ['Cross-package message rejection', testCrossPackageRejection], + ['Diagnostics: CRC failure counter', testDiagnosticCrcFailure], + ['Diagnostics: Length error counter', testDiagnosticLenError], + ['Diagnostics: Reset diagnostics', testDiagnosticReset], + ['Diagnostics: Sequence gap counter', testDiagnosticSeqGap], + ['Diagnostics: Sync recovery counter', testDiagnosticSyncRecovery], ['Invalid message ID rejection', testInvalidMsgId], ['Invalid start bytes detection', testInvalidStartBytes], + ['IPC buffer: unknown msg_id advances one byte', testIpcBufferUnknownMsgId], ['Minimal profile: Truncated frame', testMinimalProfileTruncatedFrame], ['Multiple frames: CRC error then valid frame', testCrcErrorThenValidFrame], ['Multiple frames: Corrupted middle frame', testMultipleCorruptedFrames], @@ -782,9 +1019,12 @@ function main() { ['Network profile: Corrupted pkg_id', testNetworkCorruptedPkgId], ['Network profile: SysId/CompId corruption', testNetworkSysIdCompId], ['Partial frame across buffer boundary', testPartialFrameBoundary], + ['Sensor buffer: unknown msg_id resync', testSensorBufferUnknownMsgIdResync], + ['Split sweep: two frames at every boundary', testSplitSweepAllBoundaries], ['Stream mode: recovers after garbage prefix', testStreamRecoversAfterGarbage], ['Streaming: Corrupted CRC detection', testStreamingCorruptedCrc], ['Streaming: Garbage data handling', testStreamingGarbage], + ['Streaming: two frames byte-by-byte', testStreamingTwoFrames], ['Truncated frame detection', testTruncatedFrame], ['Zero-length buffer handling', testZeroLengthBuffer], ]; diff --git a/tests/py/test_negative.py b/tests/py/test_negative.py index 522c2a1e..5df8659d 100644 --- a/tests/py/test_negative.py +++ b/tests/py/test_negative.py @@ -29,6 +29,7 @@ AccumulatingReader, PROFILE_STANDARD_CONFIG, PROFILE_SENSOR_CONFIG, + PROFILE_IPC_CONFIG, PROFILE_BULK_CONFIG, PROFILE_NETWORK_CONFIG, FrameMsgStatus, @@ -649,6 +650,146 @@ def test_diagnostic_reset(): ) +def test_buffer_mode_crc_counters(): + """Buffer mode: a CRC-failed frame increments cnt_crc_failures, cnt_failed_bytes + and cnt_sync_recoveries — same counter semantics as stream mode.""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_STANDARD_CONFIG, capacity=1024) + writer.write(msg) + frame_size = writer.size() + writer.write(msg) + total = writer.size() + + buffer = bytearray(writer.data()[:total]) + buffer[frame_size - 1] ^= 0xFF # corrupt frame 1's CRC + + reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) + reader.add_data(bytes(buffer)) + valid_count = 0 + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + + diag = reader.diagnostics + return ( + valid_count == 1 + and diag.cnt_crc_failures == 1 + and diag.cnt_sync_recoveries == 1 + and diag.cnt_failed_bytes == frame_size + ) + + +def test_buffer_mode_seq_gap(): + """Buffer mode: sequence gaps are detected on frames consumed via add_data, + matching stream-mode behaviour.""" + msg = _make_test_msg() + + def encode(seq): + w = BufferWriter(PROFILE_NETWORK_CONFIG, capacity=1024) + w.write(msg, seq=seq, sys_id=1, comp_id=1) + return bytes(w.data()[:w.size()]) + + data = encode(0) + encode(5) # skips seq 1-4 → one gap + + reader = AccumulatingReader(PROFILE_NETWORK_CONFIG, get_message_info=get_message_info, buffer_size=1024) + reader.add_data(data) + valid_count = 0 + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + + diag = reader.diagnostics + return valid_count == 2 and diag.cnt_seq_gaps == 1 and diag.cnt_crc_failures == 0 + + +def test_sensor_buffer_unknown_msg_id_resync(): + """Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + triggers a resync scan to the next start byte instead of discarding the buffer.""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_SENSOR_CONFIG, capacity=1024) + writer.write(msg) + frame = bytes(writer.data()[:writer.size()]) + + start1 = PROFILE_SENSOR_CONFIG.computed_start_byte1() + data = bytes([start1, 0xFF]) + frame # 0xFF is not a known msg_id + + reader = BufferReader(PROFILE_SENSOR_CONFIG, buffer=data, get_message_info=get_message_info) + saw_sync = False + valid_count = 0 + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + elif result.status == FrameMsgStatus.SYNC_RECOVERY: + saw_sync = True + + return saw_sync and valid_count == 1 + + +def test_ipc_buffer_unknown_msg_id(): + """IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + following valid frame is still delivered.""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_IPC_CONFIG, capacity=1024) + writer.write(msg) + frame = bytes(writer.data()[:writer.size()]) + + data = bytes([0xFF]) + frame # 0xFF is not a known msg_id + + reader = BufferReader(PROFILE_IPC_CONFIG, buffer=data, get_message_info=get_message_info) + saw_sync = False + valid_count = 0 + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + elif result.status == FrameMsgStatus.SYNC_RECOVERY: + saw_sync = True + + return saw_sync and valid_count == 1 + + +def test_split_sweep_all_boundaries(): + """Split sweep: two back-to-back frames delivered intact when the byte stream + is split into two add_data chunks at every possible offset.""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_STANDARD_CONFIG, capacity=1024) + writer.write(msg) + writer.write(msg) + data = bytes(writer.data()[:writer.size()]) + total = len(data) + + for split in range(1, total): + reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) + valid_count = 0 + for chunk in (data[:split], data[split:]): + reader.add_data(chunk) + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + if valid_count != 2: + return False + if reader.has_partial(): + return False + return True + + +def test_streaming_two_frames(): + """Streaming: two back-to-back frames are both decoded byte-by-byte.""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_STANDARD_CONFIG, capacity=1024) + writer.write(msg) + writer.write(msg) + data = bytes(writer.data()[:writer.size()]) + + reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) + valid_count = 0 + for b in data: + result = reader.push_byte(b) + if result.valid: + valid_count += 1 + + return valid_count == 2 + + def test_status_waiting_for_start(): """FrameMsgStatus: push_byte returns WAITING_FOR_START when no start byte seen yet.""" reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) @@ -1078,6 +1219,8 @@ def main(): # Define test matrix tests = [ + ("Buffer mode: CRC failure counters", test_buffer_mode_crc_counters), + ("Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap), ("Buffer mode: invalid result carries diagnostics", test_buffer_mode_invalid_result_has_diagnostics), ("Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure), ("Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure), @@ -1094,12 +1237,15 @@ def main(): ("Diagnostics: Sync recovery counter", test_diagnostic_sync_recovery), ("Invalid message ID rejection", test_invalid_msg_id), ("Invalid start bytes detection", test_invalid_start_bytes), + ("IPC buffer: unknown msg_id advances one byte", test_ipc_buffer_unknown_msg_id), ("Minimal profile: Truncated frame", test_minimal_profile_truncated_frame), ("Multiple frames: CRC error then valid frame", test_crc_error_then_valid_frame), ("Multiple frames: Corrupted middle frame", test_multiple_corrupted_frames), ("Network profile: Corrupted pkg_id byte", test_network_corrupted_pkg_id), ("Network profile: SysId/CompId corruption", test_network_sysid_compid), ("Partial frame across buffer boundary", test_partial_frame_boundary), + ("Sensor buffer: unknown msg_id resync", test_sensor_buffer_unknown_msg_id_resync), + ("Split sweep: two frames at every boundary", test_split_sweep_all_boundaries), ("Split-buffer: CRC error status preserved", test_split_buffer_crc_error_status), ("TryNext drain: CRC/resync + valid", test_try_next_drain_contract), ("TryNext partial pending contract", test_try_next_partial_pending_contract), @@ -1110,6 +1256,7 @@ def main(): ("Stream mode: recovers after garbage prefix", test_stream_recovers_after_garbage), ("Streaming: Corrupted CRC detection", test_streaming_corrupted_crc), ("Streaming: Garbage data handling", test_streaming_garbage), + ("Streaming: two frames byte-by-byte", test_streaming_two_frames), ("Truncated frame detection", test_truncated_frame), ("Zero-length buffer handling", test_zero_length_buffer), ] diff --git a/tests/run_tests.py b/tests/run_tests.py index ea6bbd94..67aaf15a 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -785,7 +785,7 @@ def _compile_language(self, lang: Language) -> bool: base_runners = ["test_standard", "test_extended", "test_variable_flag", "test_profiling", "test_profiling_generated", "test_negative", "test_wire_evolution", "test_wire_evolution_interop"] - sdk_runners = ["test_streaming"] if lang.id == "c" else ["test_sdk_units", "test_sdk_subscribe"] + sdk_runners = ["test_streaming"] if lang.id == "c" else ["test_sdk_units", "test_sdk_subscribe", "test_sdk_headers_compile"] for runner in base_runners + sdk_runners: source = test_dir / f"{runner}{lang.source_ext}" @@ -797,8 +797,8 @@ def _compile_language(self, lang: Language) -> bool: cmd = f'{_cc()} {_cflags()} -I"{gen_dir}" -o "{output}" "{source}" {_ldflags()} -lm' else: include_dir = test_dir / "include" - if runner == "test_sdk_subscribe": - # SDK subscribe test needs the C++ SDK boilerplate headers + if runner in ("test_sdk_subscribe", "test_sdk_headers_compile"): + # These tests need the C++ SDK boilerplate headers sdk_dir = self.project_root / "src" / "struct_frame" / "boilerplate" / "cpp" cmd = f'{_cxx()} -std=c++20 {_cxxflags()} -I"{gen_dir}" -I"{include_dir}" -I"{sdk_dir}" -o "{output}" "{source}" {_ldflags()}' else: @@ -1800,6 +1800,7 @@ def _record(test_name: str, lang_id: str, success: bool, for runner, test_row in [ ("test_sdk_units", "test_sdk_units"), ("test_sdk_subscribe", "test_sdk_subscribe"), + ("test_sdk_headers_compile", "test_sdk_subscribe"), ]: exe = build_dir / f"{runner}{cpp_lang.exe_ext}" if exe.exists(): diff --git a/tests/rust/src/main.rs b/tests/rust/src/main.rs index f7bbb860..6309fb7e 100644 --- a/tests/rust/src/main.rs +++ b/tests/rust/src/main.rs @@ -1175,7 +1175,9 @@ fn run_wire_evolution_interop_tests() -> ! { gmi: &dyn Fn(u16) -> Option, ) -> Option { let mut r = BufferReader::new(config, buf); - r.next(gmi) + // next() surfaces invalid events (CrcFailure/SyncRecovery) for drain loops; + // these interop scenarios only care whether a VALID frame was produced. + r.next(gmi).filter(|f| f.valid) } let profiles: [(ProfileConfig, &str); 3] = [ diff --git a/tests/rust/src/test_negative.rs b/tests/rust/src/test_negative.rs index 3e351309..2f4a14d3 100644 --- a/tests/rust/src/test_negative.rs +++ b/tests/rust/src/test_negative.rs @@ -7,12 +7,17 @@ // - Malformed data use struct_frame_sdk::serialization_test::*; +use struct_frame_sdk::pkg_test_messages::{ + PackageTestMessage, get_message_info as pkg_get_message_info, +}; +use struct_frame_sdk::pkg_test_a::get_message_info as pkg_a_get_message_info; use struct_frame_sdk::get_message_info; use struct_frame_sdk::FrameMsgStatus; use struct_frame_sdk::{ encode_message_crc, encode_message_minimal, encode_with_crc, AccumulatingReader, BufferWriter, BufferReader, PROFILE_STANDARD_CONFIG, PROFILE_BULK_CONFIG, PROFILE_SENSOR_CONFIG, PROFILE_NETWORK_CONFIG, + PROFILE_IPC_CONFIG, }; // ============================================================================ @@ -60,10 +65,12 @@ fn test_corrupted_crc() -> bool { data[frame_size - 1] ^= 0xFF; data[frame_size - 2] ^= 0xFF; - // Try to parse - should fail + // Parse - the CRC-failed frame is surfaced as an invalid CrcFailure event let mut reader = BufferReader::new(PROFILE_STANDARD_CONFIG, data); - let result = reader.next(&get_message_info); - result.is_none() // Expect parsing to fail + match reader.next(&get_message_info) { + Some(f) => !f.valid && f.status == FrameMsgStatus::CrcFailure && f.frame_size > 0, + None => false, + } } /// Test: Parser rejects truncated frame @@ -102,10 +109,12 @@ fn test_invalid_start_bytes() -> bool { data[0] = 0xDE; data[1] = 0xAD; - // Try to parse - should fail + // Parse - garbage start bytes are surfaced as a SyncRecovery skip event let mut reader = BufferReader::new(PROFILE_STANDARD_CONFIG, data); - let result = reader.next(&get_message_info); - result.is_none() // Expect parsing to fail + match reader.next(&get_message_info) { + Some(f) => !f.valid && f.status == FrameMsgStatus::SyncRecovery && f.frame_size > 0, + None => false, + } } /// Test: Parser handles zero-length buffer @@ -206,13 +215,22 @@ fn test_multiple_corrupted_frames() -> bool { // First should be valid let result1 = reader.next(&get_message_info); - if result1.is_none() { - return false; + match result1 { + Some(ref f) if f.valid => {} + _ => return false, } - // Second should be invalid - let result2 = reader.next(&get_message_info); - result2.is_none() // Expect failure on second frame + // Second is surfaced as an invalid CrcFailure event + match reader.next(&get_message_info) { + Some(f) if !f.valid && f.status == FrameMsgStatus::CrcFailure => {} + _ => return false, + } + + // Third frame after the corrupt one is still delivered + match reader.next(&get_message_info) { + Some(f) => f.valid, + None => false, + } } /// Test: Bulk profile with corrupted CRC @@ -231,10 +249,12 @@ fn test_bulk_profile_corrupted_crc() -> bool { data[frame_size - 1] ^= 0xFF; data[frame_size - 2] ^= 0xFF; - // Try to parse - should fail + // Parse - the CRC-failed frame is surfaced as an invalid CrcFailure event let mut reader = BufferReader::new(PROFILE_BULK_CONFIG, data); - let result = reader.next(&get_message_info); - result.is_none() // Expect parsing to fail + match reader.next(&get_message_info) { + Some(f) => !f.valid && f.status == FrameMsgStatus::CrcFailure && f.frame_size > 0, + None => false, + } } /// Test: AccumulatingReader handles a frame fed in two separate add_data chunks @@ -302,8 +322,10 @@ fn test_invalid_msg_id() -> bool { buf[3] = 0xFF; let mut reader = BufferReader::new(PROFILE_STANDARD_CONFIG, buf[..frame_size].to_vec()); - let result = reader.next(&get_message_info); - result.is_none() // Expect failure: CRC mismatch due to wrong magic values + match reader.next(&get_message_info) { + Some(f) => !f.valid && f.status == FrameMsgStatus::CrcFailure, + None => false, + } } /// Test: Minimal profile (no CRC) rejects a truncated frame. @@ -359,8 +381,10 @@ fn test_network_sysid_compid() -> bool { buf[3] ^= 0xFF; let mut reader = BufferReader::new(PROFILE_NETWORK_CONFIG, buf[..frame_size].to_vec()); - let result = reader.next(&get_message_info); - result.is_none() // Expect failure: corrupted sys_id invalidates CRC + match reader.next(&get_message_info) { + Some(f) => !f.valid && f.status == FrameMsgStatus::CrcFailure, + None => false, + } } /// Test: BufferReader advances past a CRC-failed frame and decodes the next valid frame. @@ -379,13 +403,17 @@ fn test_buffer_reader_skips_crc_failure() -> bool { let mut reader = BufferReader::new(PROFILE_STANDARD_CONFIG, data[..total].to_vec()); - let result1 = reader.next(&get_message_info); - if result1.is_some() { - return false; // First frame must fail (CRC corrupted) + // First frame is surfaced as an invalid CrcFailure event + match reader.next(&get_message_info) { + Some(f) if !f.valid && f.status == FrameMsgStatus::CrcFailure => {} + _ => return false, } - let result2 = reader.next(&get_message_info); - result2.is_some() // Second frame must succeed after skipping the bad one + // Second frame must succeed after skipping the bad one + match reader.next(&get_message_info) { + Some(f) => f.valid, + None => false, + } } /// Test: AccumulatingReader buffer mode recovers after a CRC failure in add_data path. @@ -465,20 +493,22 @@ fn test_crc_error_then_valid_frame() -> bool { let mut reader = BufferReader::new(PROFILE_STANDARD_CONFIG, data[..total].to_vec()); // Frame 1 must decode successfully - let result1 = reader.next(&get_message_info); - if result1.is_none() { - return false; + match reader.next(&get_message_info) { + Some(f) if f.valid => {} + _ => return false, } - // Frame 2 must fail (CRC error) — reader must advance to frame 3 - let result2 = reader.next(&get_message_info); - if result2.is_some() { - return false; + // Frame 2 is surfaced as an invalid CrcFailure event — status must be preserved + match reader.next(&get_message_info) { + Some(f) if !f.valid && f.status == FrameMsgStatus::CrcFailure => {} + _ => return false, } // Reader must decode frame 3 after advancing past the CRC error - let result3 = reader.next(&get_message_info); - result3.is_some() + match reader.next(&get_message_info) { + Some(f) => f.valid, + None => false, + } } /// Test: AccumulatingReader correctly recovers after a CRC-failed frame that was @@ -643,6 +673,361 @@ fn run_test(name: &str, func: fn() -> bool) -> bool { passed } + +// ============================================================================ +// Helpers for the diagnostics / buffer-mode scenarios +// ============================================================================ + +/// Encode one Network-profile frame with the given sequence number. +fn encode_network_frame(seq: u8) -> Vec { + let msg = create_test_message(); + let mut payload = vec![0u8; BasicTypesMessage::MAX_SIZE]; + let payload_len = msg.pack(&mut payload); + let mut buf = vec![0u8; 1024]; + let frame_size = encode_with_crc( + &PROFILE_NETWORK_CONFIG, + &mut buf, + seq, + 1, + 1, + (BasicTypesMessage::MSG_ID >> 8) as u8, + (BasicTypesMessage::MSG_ID & 0xFF) as u8, + &payload[..payload_len], + BasicTypesMessage::MAGIC1, + BasicTypesMessage::MAGIC2, + ); + buf.truncate(frame_size); + buf +} + +/// Encode two back-to-back Standard-profile frames; returns (data, first frame size). +fn encode_two_standard_frames() -> (Vec, usize) { + let msg = create_test_message(); + let mut writer = BufferWriter::new(PROFILE_STANDARD_CONFIG, 2048); + let first = writer.write_crc(&msg, 0); + writer.write_crc(&msg, 0); + (writer.data().to_vec(), first) +} + +/// Diagnostics: cnt_crc_failures increments on a CRC failure. +fn test_diagnostic_crc_failure() -> bool { + let msg = create_test_message(); + let mut writer = BufferWriter::new(PROFILE_STANDARD_CONFIG, 1024); + writer.write_crc(&msg, 0); + let mut data = writer.data().to_vec(); + let frame_size = data.len(); + if frame_size < 4 { + return false; + } + data[frame_size - 1] ^= 0xFF; + data[frame_size - 2] ^= 0xFF; + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + for b in &data { + reader.push_byte(*b, &get_message_info); + } + + let diag = reader.diagnostics(); + diag.cnt_crc_failures == 1 && diag.cnt_sync_recoveries >= 1 +} + +/// Diagnostics: cnt_sync_recoveries increments when garbage bytes are fed. +/// (The unified Rust reader classifies data only once a full header's worth of +/// bytes is buffered, so feed a whole overhead-sized garbage chunk.) +fn test_diagnostic_sync_recovery() -> bool { + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + reader.add_data(&[0x90, 0xAB, 0x00, 0x00, 0x00, 0x00]); // bad start2 -> resync + while reader.try_next(&get_message_info).is_some() {} + reader.diagnostics().cnt_sync_recoveries >= 1 +} + +/// Diagnostics: cnt_len_errors increments when the header length field is out of +/// the [min_size, size] range. A zero length completes a (short, CRC-failing) +/// frame immediately, so the reader records the out-of-range length. +fn test_diagnostic_len_error() -> bool { + let msg = create_test_message(); + let mut writer = BufferWriter::new(PROFILE_STANDARD_CONFIG, 1024); + writer.write_crc(&msg, 0); + let mut data = writer.data().to_vec(); + if data.len() < 5 { + return false; + } + + // ProfileStandard header: [0x90][0x71][LEN][MSG_ID]... Zero out the length — + // out of range as long as the message's min_size is non-zero. + let info = match get_message_info(BasicTypesMessage::MSG_ID) { + Some(i) => i, + None => return false, + }; + if info.min_size == 0 { + return false; + } + data[2] = 0; + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + reader.add_data(&data); + while reader.try_next(&get_message_info).is_some() {} + + reader.diagnostics().cnt_len_errors >= 1 +} + +/// Diagnostics: cnt_seq_gaps increments when a sequence number is skipped. +fn test_diagnostic_seq_gap() -> bool { + let frame0 = encode_network_frame(0); + let frame5 = encode_network_frame(5); // skips seq 1-4 + + let mut reader = AccumulatingReader::new(PROFILE_NETWORK_CONFIG, 1024); + let mut valid_count = 0; + for b in frame0.iter().chain(frame5.iter()) { + if let Some(f) = reader.push_byte(*b, &get_message_info) { + if f.valid { + valid_count += 1; + } + } + } + + let diag = reader.diagnostics(); + valid_count == 2 && diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0 +} + +/// Diagnostics: reset_diagnostics() clears all counters. +fn test_diagnostic_reset() -> bool { + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + reader.add_data(&[0x90, 0xAB, 0x00, 0x00, 0x00, 0x00]); // bad start2 -> resync + while reader.try_next(&get_message_info).is_some() {} + if reader.diagnostics().cnt_sync_recoveries < 1 { + return false; + } + + reader.reset_diagnostics(); + let diag = reader.diagnostics(); + diag.cnt_crc_failures == 0 + && diag.cnt_sync_recoveries == 0 + && diag.cnt_failed_bytes == 0 + && diag.cnt_len_errors == 0 + && diag.cnt_seq_gaps == 0 +} + +/// Buffer mode: a CRC-failed frame increments cnt_crc_failures, cnt_failed_bytes +/// and cnt_sync_recoveries — same counter semantics as stream mode. +fn test_buffer_mode_crc_counters() -> bool { + let (mut data, frame_size) = encode_two_standard_frames(); + data[frame_size - 1] ^= 0xFF; // corrupt frame 1's CRC + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + reader.add_data(&data); + + let mut valid_count = 0; + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } + } + + let diag = reader.diagnostics(); + valid_count == 1 + && diag.cnt_crc_failures == 1 + && diag.cnt_sync_recoveries == 1 + && diag.cnt_failed_bytes == frame_size as u32 +} + +/// Buffer mode: sequence gaps are detected on frames consumed via add_data. +fn test_buffer_mode_seq_gap() -> bool { + let mut data = encode_network_frame(0); + data.extend_from_slice(&encode_network_frame(5)); // skips seq 1-4 + + let mut reader = AccumulatingReader::new(PROFILE_NETWORK_CONFIG, 1024); + reader.add_data(&data); + + let mut valid_count = 0; + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } + } + + let diag = reader.diagnostics(); + valid_count == 2 && diag.cnt_seq_gaps == 1 && diag.cnt_crc_failures == 0 +} + +/// Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte +/// triggers a resync scan to the next start byte instead of discarding the buffer. +fn test_sensor_buffer_unknown_msg_id_resync() -> bool { + let info = match get_message_info(BasicTypesMessage::MSG_ID) { + Some(i) => i, + None => return false, + }; + + // [0x70][0xFF (unknown)] then a valid sensor frame [0x70][msg_id][payload@size] + let mut data = vec![0u8; 4 + info.size]; // zeroed payload is fine for framing + data[0] = 0x70; + data[1] = 0xFF; + data[2] = 0x70; + data[3] = (BasicTypesMessage::MSG_ID & 0xFF) as u8; + + let mut reader = BufferReader::new(PROFILE_SENSOR_CONFIG, data); + let mut saw_sync = false; + let mut valid_count = 0; + while let Some(f) = reader.next(&get_message_info) { + if f.valid { + valid_count += 1; + } else if f.status == FrameMsgStatus::SyncRecovery { + saw_sync = true; + } + } + saw_sync && valid_count == 1 +} + +/// IPC (no start bytes) buffer: an unknown msg_id advances one byte and the +/// following valid frame is still delivered. +fn test_ipc_buffer_unknown_msg_id() -> bool { + let info = match get_message_info(BasicTypesMessage::MSG_ID) { + Some(i) => i, + None => return false, + }; + + let mut data = vec![0u8; 2 + info.size]; + data[0] = 0xFF; // unknown msg_id + data[1] = (BasicTypesMessage::MSG_ID & 0xFF) as u8; + + let mut reader = BufferReader::new(PROFILE_IPC_CONFIG, data); + let mut saw_sync = false; + let mut valid_count = 0; + while let Some(f) = reader.next(&get_message_info) { + if f.valid { + valid_count += 1; + } else if f.status == FrameMsgStatus::SyncRecovery { + saw_sync = true; + } + } + saw_sync && valid_count == 1 +} + +/// Split sweep: two back-to-back frames delivered intact when the byte stream is +/// split into two add_data chunks at every possible offset. +fn test_split_sweep_all_boundaries() -> bool { + let (data, _) = encode_two_standard_frames(); + let total = data.len(); + + for split in 1..total { + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + let mut valid_count = 0; + + reader.add_data(&data[..split]); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } + } + reader.add_data(&data[split..]); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } + } + + if valid_count != 2 { + return false; + } + if reader.has_partial() { + return false; + } + } + true +} + +/// Streaming: two back-to-back frames are both decoded byte-by-byte. +fn test_streaming_two_frames() -> bool { + let (data, _) = encode_two_standard_frames(); + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + let mut valid_count = 0; + for b in &data { + if let Some(f) = reader.push_byte(*b, &get_message_info) { + if f.valid { + valid_count += 1; + } + } + } + valid_count == 2 +} + +// ============================================================================ +// Package / cross-package corruption scenarios (parity with C/C++/TS/JS/C#) +// ============================================================================ + +/// Encode one Bulk-profile PackageTestMessage frame. +fn encode_bulk_pkg_frame() -> Vec { + let msg = PackageTestMessage::default(); + let mut buf = vec![0u8; 1024]; + let frame_size = encode_message_crc(&PROFILE_BULK_CONFIG, &mut buf, &msg, 0); + buf.truncate(frame_size); + buf +} + +/// Bulk profile: corrupting the pkg_id byte invalidates the CRC. +fn test_bulk_corrupted_pkg_id() -> bool { + let mut data = encode_bulk_pkg_frame(); + if data.len() < 7 { + return false; + } + // Bulk layout: [0x90][0x74][LEN_LO][LEN_HI][PKG_ID][MSG_ID]... + data[4] ^= 0xFF; + + let mut reader = BufferReader::new(PROFILE_BULK_CONFIG, data); + match reader.next(&pkg_get_message_info) { + Some(f) => !f.valid, + None => false, + } +} + +/// Bulk profile: corrupting the msg_id low byte invalidates the CRC. +fn test_bulk_corrupted_msg_id_low_byte() -> bool { + let mut data = encode_bulk_pkg_frame(); + if data.len() < 7 { + return false; + } + data[5] ^= 0xFF; + + let mut reader = BufferReader::new(PROFILE_BULK_CONFIG, data); + match reader.next(&pkg_get_message_info) { + Some(f) => !f.valid, + None => false, + } +} + +/// Cross-package rejection: a frame from one package fails CRC validation when +/// decoded with another package's message info (different pkg_id / magic bytes). +fn test_cross_package_rejection() -> bool { + let data = encode_bulk_pkg_frame(); + + let mut reader = BufferReader::new(PROFILE_BULK_CONFIG, data); + match reader.next(&pkg_a_get_message_info) { + Some(f) => !f.valid, + None => false, + } +} + +/// Network profile: corrupting the pkg_id byte invalidates the CRC. +fn test_network_corrupted_pkg_id() -> bool { + let msg = PackageTestMessage::default(); + let mut buf = vec![0u8; 1024]; + let frame_size = encode_message_crc(&PROFILE_NETWORK_CONFIG, &mut buf, &msg, 0); + if frame_size < 10 { + return false; + } + buf.truncate(frame_size); + // Network layout: [0x90][0x78][SEQ][SYS][COMP][LEN_LO][LEN_HI][PKG_ID][MSG_ID]... + buf[7] ^= 0xFF; + + let mut reader = BufferReader::new(PROFILE_NETWORK_CONFIG, buf); + match reader.next(&pkg_get_message_info) { + Some(f) => !f.valid, + None => false, + } +} + + fn main() { println!("\n========================================"); println!("NEGATIVE TESTS - Rust Parser"); @@ -653,24 +1038,39 @@ fn main() { println!("{:<50} {:>6}", "==================================================", "======"); let tests: &[(&str, fn() -> bool)] = &[ + ("Buffer mode: CRC failure counters", test_buffer_mode_crc_counters), + ("Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap), ("Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure), ("Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure), ("Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc), + ("Bulk profile: Corrupted pkg_id byte", test_bulk_corrupted_pkg_id), + ("Bulk profile: Corrupted msg_id low byte", test_bulk_corrupted_msg_id_low_byte), ("Corrupted CRC detection", test_corrupted_crc), ("Corrupted length field detection", test_corrupted_length), + ("Cross-package rejection (pkgid mismatch)", test_cross_package_rejection), + ("Diagnostics: CRC failure counter", test_diagnostic_crc_failure), + ("Diagnostics: Length error counter", test_diagnostic_len_error), + ("Diagnostics: Reset diagnostics", test_diagnostic_reset), + ("Diagnostics: Sequence gap counter", test_diagnostic_seq_gap), + ("Diagnostics: Sync recovery counter", test_diagnostic_sync_recovery), ("Invalid message ID rejection", test_invalid_msg_id), ("Invalid start bytes detection", test_invalid_start_bytes), + ("IPC buffer: unknown msg_id advances one byte", test_ipc_buffer_unknown_msg_id), ("Minimal profile: Truncated frame", test_minimal_profile_truncated_frame), ("Multiple frames: CRC error then valid frame", test_crc_error_then_valid_frame), ("Multiple frames: Corrupted middle frame", test_multiple_corrupted_frames), + ("Network profile: Corrupted pkg_id byte", test_network_corrupted_pkg_id), ("Network profile: SysId/CompId corruption", test_network_sysid_compid), ("Partial frame across buffer boundary", test_partial_frame_boundary), + ("Sensor buffer: unknown msg_id resync", test_sensor_buffer_unknown_msg_id_resync), + ("Split sweep: two frames at every boundary", test_split_sweep_all_boundaries), ("Split-buffer: CRC error status preserved", test_split_buffer_crc_error_status), ("TryNext drain: CRC/resync + valid", test_try_next_drain_contract), ("TryNext partial pending contract", test_try_next_partial_pending_contract), ("Stream mode: recovers after garbage prefix", test_stream_recovers_after_garbage), ("Streaming: Corrupted CRC detection", test_streaming_corrupted_crc), ("Streaming: Garbage data handling", test_streaming_garbage), + ("Streaming: two frames byte-by-byte", test_streaming_two_frames), ("Truncated frame detection", test_truncated_frame), ("Zero-length buffer handling", test_zero_length_buffer), ]; diff --git a/tests/ts/test_negative.ts b/tests/ts/test_negative.ts index f8e35286..3a3b854b 100644 --- a/tests/ts/test_negative.ts +++ b/tests/ts/test_negative.ts @@ -14,6 +14,7 @@ import { AccumulatingReader, ProfileStandardConfig, ProfileSensorConfig, + ProfileIPCConfig, ProfileBulkConfig, ProfileNetworkConfig, } from '../generated/ts/frame-profiles'; @@ -756,6 +757,234 @@ function testTryNextPartialPendingContract(): boolean { return !reader.hasMore(); } + +/** Helper: encode `count` Standard-profile frames; returns [buffer, per-frame sizes]. */ +function encodeStandardFrames(count: number): [Uint8Array, number[]] { + const writer = new BufferWriter(ProfileStandardConfig, 2048); + const sizes: number[] = []; + let prev = 0; + for (let i = 0; i < count; i++) { + writer.write(createTestMessage()); + sizes.push(writer.size - prev); + prev = writer.size; + } + return [writer.data().slice(0, writer.size), sizes]; +} + +/** Helper: encode one Network-profile frame with the given sequence number. */ +function encodeNetworkFrame(seq: number): Uint8Array { + const writer = new BufferWriter(ProfileNetworkConfig, 1024); + writer.write(createTestMessage(), { seq, sysId: 1, compId: 1 }); + return writer.data().slice(0, writer.size); +} + +/** Diagnostics: cntCrcFailures increments on a stream-mode CRC failure. */ +function testDiagnosticCrcFailure(): boolean { + const [buffer, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 4) return false; + + buffer[frameSize - 1] ^= 0xFF; + buffer[frameSize - 2] ^= 0xFF; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + for (let i = 0; i < frameSize; i++) { + reader.pushByte(buffer[i]); + } + + const diag = reader.diagnostics; + return diag.cntCrcFailures === 1 && diag.cntSyncRecoveries >= 1; +} + +/** Diagnostics: cntSyncRecoveries increments when garbage bytes are fed. */ +function testDiagnosticSyncRecovery(): boolean { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); // valid start1 + reader.pushByte(0xAB); // invalid start2 -> sync recovery + return reader.diagnostics.cntSyncRecoveries >= 1; +} + +/** + * Diagnostics: cntLenErrors increments when the header length field is out of the + * [minSize, size] range. Feeding just the header is enough — the check fires at + * header completion. + */ +function testDiagnosticLenError(): boolean { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info || info.size + 1 > 255) return false; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); + reader.pushByte(0x71); + reader.pushByte((info.size + 1) & 0xFF); // out of range + reader.pushByte(msgId & 0xFF); + + return reader.diagnostics.cntLenErrors === 1; +} + +/** Diagnostics: cntSeqGaps increments when a sequence number is skipped. */ +function testDiagnosticSeqGap(): boolean { + const frame0 = encodeNetworkFrame(0); + const frame5 = encodeNetworkFrame(5); // skips seq 1-4 + + const reader = new AccumulatingReader(ProfileNetworkConfig, getMessageInfo, 1024); + for (const b of frame0) reader.pushByte(b); + for (const b of frame5) reader.pushByte(b); + + const diag = reader.diagnostics; + return diag.cntSeqGaps === 1 && diag.cntCrcFailures === 0; +} + +/** Diagnostics: resetDiagnostics() clears all counters. */ +function testDiagnosticReset(): boolean { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.pushByte(0x90); + reader.pushByte(0xAB); + if (reader.diagnostics.cntSyncRecoveries < 1) return false; + + reader.resetDiagnostics(); + const diag = reader.diagnostics; + return diag.cntCrcFailures === 0 && diag.cntSyncRecoveries === 0 && + diag.cntFailedBytes === 0 && diag.cntLenErrors === 0 && diag.cntSeqGaps === 0; +} + +/** + * Buffer mode: a CRC-failed frame increments cntCrcFailures, cntFailedBytes and + * cntSyncRecoveries — same counter semantics as stream mode. + */ +function testBufferModeCrcCounters(): boolean { + const [buffer, sizes] = encodeStandardFrames(2); + const frameSize = sizes[0]; + buffer[frameSize - 1] ^= 0xFF; // corrupt frame 1's CRC + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + reader.addData(buffer); + + let validCount = 0; + let r: any; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + const diag = reader.diagnostics; + return validCount === 1 && diag.cntCrcFailures === 1 && + diag.cntSyncRecoveries === 1 && diag.cntFailedBytes === frameSize; +} + +/** Buffer mode: sequence gaps are detected on frames consumed via addData. */ +function testBufferModeSeqGap(): boolean { + const frame0 = encodeNetworkFrame(0); + const frame5 = encodeNetworkFrame(5); // skips seq 1-4 + const data = new Uint8Array(frame0.length + frame5.length); + data.set(frame0, 0); + data.set(frame5, frame0.length); + + const reader = new AccumulatingReader(ProfileNetworkConfig, getMessageInfo, 1024); + reader.addData(data); + + let validCount = 0; + let r: any; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + const diag = reader.diagnostics; + return validCount === 2 && diag.cntSeqGaps === 1 && diag.cntCrcFailures === 0; +} + +/** + * Sensor (minimal) profile buffer: an unknown msg_id after a valid start byte + * triggers a resync scan to the next start byte instead of discarding the buffer. + */ +function testSensorBufferUnknownMsgIdResync(): boolean { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info) return false; + + const data = new Uint8Array(4 + info.size); // zeroed payload is fine for framing + data[0] = 0x70; + data[1] = 0xFF; // unknown msg_id + data[2] = 0x70; + data[3] = msgId & 0xFF; + + const reader = new BufferReader(ProfileSensorConfig, data, getMessageInfo); + let sawSync = false; + let validCount = 0; + let r: any; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount === 1; +} + +/** + * IPC (no start bytes) buffer: an unknown msg_id advances one byte and the + * following valid frame is still delivered. + */ +function testIpcBufferUnknownMsgId(): boolean { + const msgId = BasicTypesMessage._msgid; + const info = getMessageInfo(msgId); + if (!info) return false; + + const data = new Uint8Array(2 + info.size); + data[0] = 0xFF; // unknown msg_id + data[1] = msgId & 0xFF; + + const reader = new BufferReader(ProfileIPCConfig, data, getMessageInfo); + let sawSync = false; + let validCount = 0; + let r: any; + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + return sawSync && validCount === 1; +} + +/** + * Split sweep: two back-to-back frames delivered intact when the byte stream is + * split into two addData chunks at every possible offset. + */ +function testSplitSweepAllBoundaries(): boolean { + const [buffer] = encodeStandardFrames(2); + const total = buffer.length; + + for (let split = 1; split < total; split++) { + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + let r: any; + + reader.addData(buffer.slice(0, split)); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + reader.addData(buffer.slice(split)); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + + if (validCount !== 2) return false; + if (reader.hasPartial()) return false; + } + return true; +} + +/** Streaming: two back-to-back frames are both decoded byte-by-byte. */ +function testStreamingTwoFrames(): boolean { + const [buffer] = encodeStandardFrames(2); + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + for (const b of buffer) { + const r = reader.pushByte(b); + if (r.valid) validCount++; + } + return validCount === 2; +} + + function main(): number { console.log('\n========================================'); console.log('NEGATIVE TESTS - TypeScript Parser'); @@ -763,6 +992,8 @@ function main(): number { // Define test matrix const tests: Array<[string, () => boolean]> = [ + ['Buffer mode: CRC failure counters', testBufferModeCrcCounters], + ['Buffer mode: Sequence gap counted', testBufferModeSeqGap], ['Buffer mode: recovers after CRC failure', testBufferModeRecoversAfterCrcFailure], ['Buffer reader: skips CRC-failed frame', testBufferReaderSkipsCrcFailure], ['Bulk profile: Corrupted CRC', testBulkProfileCorruptedCrc], @@ -771,8 +1002,14 @@ function main(): number { ['Corrupted CRC detection', testCorruptedCrc], ['Corrupted length field detection', testCorruptedLength], ['Cross-package message rejection', testCrossPackageRejection], + ['Diagnostics: CRC failure counter', testDiagnosticCrcFailure], + ['Diagnostics: Length error counter', testDiagnosticLenError], + ['Diagnostics: Reset diagnostics', testDiagnosticReset], + ['Diagnostics: Sequence gap counter', testDiagnosticSeqGap], + ['Diagnostics: Sync recovery counter', testDiagnosticSyncRecovery], ['Invalid message ID rejection', testInvalidMsgId], ['Invalid start bytes detection', testInvalidStartBytes], + ['IPC buffer: unknown msg_id advances one byte', testIpcBufferUnknownMsgId], ['Minimal profile: Truncated frame', testMinimalProfileTruncatedFrame], ['Multiple frames: CRC error then valid frame', testCrcErrorThenValidFrame], ['Multiple frames: Corrupted middle frame', testMultipleCorruptedFrames], @@ -782,9 +1019,12 @@ function main(): number { ['Network profile: Corrupted pkg_id', testNetworkCorruptedPkgId], ['Network profile: SysId/CompId corruption', testNetworkSysIdCompId], ['Partial frame across buffer boundary', testPartialFrameBoundary], + ['Sensor buffer: unknown msg_id resync', testSensorBufferUnknownMsgIdResync], + ['Split sweep: two frames at every boundary', testSplitSweepAllBoundaries], ['Stream mode: recovers after garbage prefix', testStreamRecoversAfterGarbage], ['Streaming: Corrupted CRC detection', testStreamingCorruptedCrc], ['Streaming: Garbage data handling', testStreamingGarbage], + ['Streaming: two frames byte-by-byte', testStreamingTwoFrames], ['Truncated frame detection', testTruncatedFrame], ['Zero-length buffer handling', testZeroLengthBuffer], ]; diff --git a/tests/ts/test_request_response_sdk.ts b/tests/ts/test_request_response_sdk.ts index 7bc7fd28..830a7cce 100644 --- a/tests/ts/test_request_response_sdk.ts +++ b/tests/ts/test_request_response_sdk.ts @@ -227,6 +227,42 @@ async function testSubscriptionRemovedAfterTimeout(): Promise { assert('cleanup: subscription removed after timeout', handlers.length === 0); } + +/** + * Two concurrent predicate-less requests on the same response msg_id must BOTH + * resolve from a single response. Regression test for the dispatch-skip bug: + * request()'s one-shot handler unsubscribes during dispatch, which used to + * splice the live handler array and skip the next handler for the same message. + */ +async function testConcurrentRequestsSameResponse(): Promise { + const transport = new MockTransport(); + const sdk = makeSdk(transport); + + sdk.registerCodec({ + getMsgId: () => BasicTypesMessage._msgid, + deserialize: (data) => BasicTypesMessage.deserialize(Buffer.from(data)), + }); + + // A single response arrives after both requests are parked. + injectAfter(transport, encodeMsg(7, true), 40); + + const [resultA, resultB] = await Promise.all([ + sdk.request( + new BasicTypesMessage({ regularInt: 1 }), + BasicTypesMessage._msgid, + { timeout: 2 }, + ), + sdk.request( + new BasicTypesMessage({ regularInt: 2 }), + BasicTypesMessage._msgid, + { timeout: 2 }, + ), + ]); + + assert('same-response: request A resolved', (resultA as any).regularInt === 7); + assert('same-response: request B resolved (no dispatch skip)', (resultB as any).regularInt === 7); +} + // ============================================================================= // Main // ============================================================================= @@ -242,6 +278,7 @@ async function main(): Promise { await testTimeout(); await testMatchPredicateFiltersResponses(); await testConcurrentRequestsWithMatch(); + await testConcurrentRequestsSameResponse(); await testSubscriptionRemovedAfterSuccess(); await testSubscriptionRemovedAfterTimeout(); From 37c6a0c6cab83d53510050b7199170e7b00e643b Mon Sep 17 00:00:00 2001 From: Rijesh Augustine Date: Mon, 6 Jul 2026 16:08:56 -0600 Subject: [PATCH 2/6] more fixes --- .../content/docs/reference/test-coverage.md | 4 +- .../boilerplate/c/frame_profiles.h | 92 +++++++++++-- .../boilerplate/cpp/frame_profiles.hpp | 54 ++++++++ .../struct_frame_sdk/network_transports.hpp | 63 ++++++--- .../cpp/struct_frame_sdk/transport.hpp | 15 ++- .../Framework/Framing/AccumulatingReader.cs | 34 +++-- .../csharp/Framework/Sdk/StructFrameSdk.cs | 124 ++++++++++------- .../csharp/Framework/Sdk/Transport.cs | 45 +++++-- .../Sdk/Transports/SerialTransport.cs | 26 ++-- .../boilerplate/js/frame-profiles.js | 114 +++++++++++----- .../boilerplate/py/frame_profiles.py | 78 +++++++++-- .../py/struct_frame_sdk/serial_transport.py | 4 +- .../py/struct_frame_sdk/tcp_transport.py | 4 +- .../py/struct_frame_sdk/transport.py | 30 ++++- .../py/struct_frame_sdk/udp_transport.py | 4 +- .../struct_frame_sdk/websocket_transport.py | 4 +- .../boilerplate/rust/frame_profiles.rs | 48 +++++++ .../boilerplate/ts/frame-profiles.ts | 125 +++++++++++++----- tests/NEGATIVE_TESTS.md | 22 +-- tests/c/test_negative.c | 91 +++++++++++++ tests/coverage_spec.py | 6 +- tests/cpp/test_negative.cpp | 82 ++++++++++++ tests/csharp/TestBaseTransport.cs | 34 +++++ tests/csharp/TestNegative.cs | 91 +++++++++++++ tests/js/test_negative.js | 84 ++++++++++++ tests/py/test_negative.py | 78 +++++++++++ tests/py/test_sdk.py | 48 +++++++ tests/rust/src/test_negative.rs | 94 +++++++++++++ tests/ts/test_negative.ts | 84 ++++++++++++ 29 files changed, 1372 insertions(+), 210 deletions(-) diff --git a/docs/src/content/docs/reference/test-coverage.md b/docs/src/content/docs/reference/test-coverage.md index 6656b649..f6d3a6de 100644 --- a/docs/src/content/docs/reference/test-coverage.md +++ b/docs/src/content/docs/reference/test-coverage.md @@ -226,12 +226,14 @@ Test files: `tests/{c,cpp,py,ts,js,csharp,rust}/test_negative.*` See `tests/NEGATIVE_TESTS.md` for full scenario descriptions. -The 31 scenarios in the table below are registered in every language's `test_negative.*` file, covering corruption handling, the `tryNext` drain contract, diagnostic counters (unified semantics in buffer and stream mode), minimal-profile resync, and a chunk-boundary split sweep. All seven languages additionally carry the four package-corruption scenarios (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) for 35 scenarios each; Python (40) adds status-machine and buffer-mode diagnostic extras. +The 33 scenarios in the table below are registered in every language's `test_negative.*` file, covering corruption handling, the `tryNext` drain contract, diagnostic counters (unified semantics in buffer and stream mode), minimal-profile resync, and a chunk-boundary split sweep. All seven languages additionally carry the four package-corruption scenarios (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption) for 37 scenarios each; Python (42) adds status-machine and buffer-mode diagnostic extras. | Error Scenario (test name) | C | C++ | Python | TS | JS | C# | Rust | |--------|--------|--------|--------|--------|--------|--------|--------| | Buffer mode: CRC failure counters | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Buffer mode: Sequence gap counted | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Buffer mode: garbage prefix partial recovers | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| Buffer mode: oversized length recovers | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Buffer mode: recovers after CRC failure | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Buffer reader: skips CRC-failed frame | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Bulk profile: Corrupted CRC | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | diff --git a/src/struct_frame/boilerplate/c/frame_profiles.h b/src/struct_frame/boilerplate/c/frame_profiles.h index 6505a15b..5ee76329 100644 --- a/src/struct_frame/boilerplate/c/frame_profiles.h +++ b/src/struct_frame/boilerplate/c/frame_profiles.h @@ -681,6 +681,12 @@ typedef struct accumulating_reader { size_t bytes_appended_to_internal; /* bytes copied from current buffer into internal buffer this add_data() call */ accumulating_reader_state_t state; + /* Cached per-profile constants so the per-byte push path doesn't recompute + * them on every call. */ + uint8_t cached_start_byte1; + uint8_t cached_start_byte2; + uint8_t cached_header_size; + const uint8_t* current_buffer; size_t current_size; size_t current_offset; @@ -706,6 +712,17 @@ static inline void accumulating_reader_init( reader->expected_frame_size = 0; reader->bytes_appended_to_internal = 0; reader->state = ACC_STATE_IDLE; + + /* Precompute the per-profile constants used on every push_byte() call. */ + reader->cached_start_byte1 = config->header.start_byte1; + reader->cached_start_byte2 = config->header.start_byte2; + if (config->header.header_type == HEADER_TINY && config->header.encodes_payload_type) { + reader->cached_start_byte1 = get_tiny_start_byte(config->payload.payload_type); + } + if (config->header.header_type == HEADER_BASIC && config->header.encodes_payload_type) { + reader->cached_start_byte2 = get_basic_second_start_byte(config->payload.payload_type); + } + reader->cached_header_size = profile_header_size(config); reader->current_buffer = NULL; reader->current_size = 0; reader->current_offset = 0; @@ -803,6 +820,20 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r /* First, try to complete a partial message from the internal buffer. * partial_len = bytes that came from a PREVIOUS add_data call (before * bytes_appended_to_internal were added this call). */ + if (reader->internal_data_len > 0 && reader->current_offset == 0 && + reader->bytes_appended_to_internal == 0 && + reader->current_size > reader->current_offset) { + /* Full-wedge escape: new data arrived but nothing could be appended + * because the internal buffer is full. The buffered bytes can never + * complete — discard them so parsing continues from the current + * buffer instead of stalling forever. */ + reader->diagnostics.cnt_sync_recoveries++; + reader->diagnostics.cnt_failed_bytes += (uint32_t)reader->internal_data_len; + reader->internal_data_len = 0; + reader->expected_frame_size = 0; + /* fall through to current-buffer parsing below */ + } + if (reader->internal_data_len > 0 && reader->current_offset == 0) { size_t partial_len = reader->internal_data_len - reader->bytes_appended_to_internal; result = _acc_parse_buffer(reader, reader->internal_buffer, reader->internal_data_len); @@ -831,6 +862,41 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r return result; } + /* Garbage prefix saved as a partial (WAITING_FOR_START), or a frame + * that can never complete because its claimed size exceeds the + * internal buffer (still collecting with the buffer full): resync + * inside the internal buffer instead of waiting forever. */ + if (result.status == FRAME_MSG_STATUS_WAITING_FOR_START || + reader->internal_data_len >= reader->buffer_size) { + size_t discard = reader->internal_data_len; + if (reader->config->header.num_start_bytes >= 1) { + if (reader->internal_data_len > 1) { + const void* p = memchr(reader->internal_buffer + 1, (int)sb1, + reader->internal_data_len - 1); + if (p) discard = (size_t)((const uint8_t*)p - reader->internal_buffer); + } + } else { + /* No start bytes (e.g. IPC): advance one byte and retry. */ + discard = 1; + } + reader->diagnostics.cnt_sync_recoveries++; + reader->diagnostics.cnt_failed_bytes += (uint32_t)discard; + size_t keep = reader->internal_data_len - discard; + if (keep > 0) { + memmove(reader->internal_buffer, reader->internal_buffer + discard, keep); + } + /* Only the portion of the discard that reached into this cycle's + * appended bytes reduces the appended count. */ + if (discard > partial_len) { + reader->bytes_appended_to_internal -= (discard - partial_len); + } + reader->internal_data_len = keep; + result.valid = false; + result.status = FRAME_MSG_STATUS_SYNC_RECOVERY; + result.frame_size = discard; + return result; + } + /* Still not enough data for a complete frame — wait for next add_data() */ return result; } @@ -895,6 +961,11 @@ static inline frame_msg_info_t accumulating_reader_next(accumulating_reader_t* r reader->internal_data_len = remaining; reader->bytes_appended_to_internal = 0; reader->current_offset = reader->current_size; + } else if (remaining >= reader->buffer_size) { + /* Partial too large to buffer — discard with diagnostics. */ + reader->diagnostics.cnt_sync_recoveries++; + reader->diagnostics.cnt_failed_bytes += (uint32_t)remaining; + reader->current_offset = reader->current_size; } return result; @@ -904,20 +975,13 @@ static inline frame_msg_info_t accumulating_reader_push_byte(accumulating_reader { frame_msg_info_t result = {false, 0, 0, NULL, FRAME_MSG_STATUS_NONE, 0, NULL}; result.diagnostics = &reader->diagnostics; - - uint8_t header_size = profile_header_size(reader->config); - uint8_t start_byte1 = reader->config->header.start_byte1; - uint8_t start_byte2 = reader->config->header.start_byte2; - - /* For tiny headers, start byte encodes payload type */ - if (reader->config->header.header_type == HEADER_TINY && reader->config->header.encodes_payload_type) { - start_byte1 = get_tiny_start_byte(reader->config->payload.payload_type); - } - /* For basic headers, second start byte encodes payload type */ - if (reader->config->header.header_type == HEADER_BASIC && reader->config->header.encodes_payload_type) { - start_byte2 = get_basic_second_start_byte(reader->config->payload.payload_type); - } - + + /* Use the constants precomputed at init instead of re-deriving them on + * every pushed byte. */ + uint8_t header_size = reader->cached_header_size; + uint8_t start_byte1 = reader->cached_start_byte1; + uint8_t start_byte2 = reader->cached_start_byte2; + if (reader->state == ACC_STATE_IDLE || reader->state == ACC_STATE_BUFFER_MODE) { reader->state = ACC_STATE_LOOKING_FOR_START1; reader->internal_data_len = 0; diff --git a/src/struct_frame/boilerplate/cpp/frame_profiles.hpp b/src/struct_frame/boilerplate/cpp/frame_profiles.hpp index 260e4187..94c91787 100644 --- a/src/struct_frame/boilerplate/cpp/frame_profiles.hpp +++ b/src/struct_frame/boilerplate/cpp/frame_profiles.hpp @@ -815,6 +815,19 @@ class AccumulatingReader { return with_diagnostics(FrameMsgInfo()); } + // Full-wedge escape: new data arrived but nothing could be appended because + // the internal buffer is full. The buffered bytes can never complete — + // discard them so parsing continues from the current buffer instead of + // stalling forever. + if (internal_data_len_ > 0 && current_offset_ == 0 && + bytes_appended_to_internal_ == 0 && current_size_ > current_offset_) { + diagnostics_.cnt_sync_recoveries++; + diagnostics_.cnt_failed_bytes += static_cast(internal_data_len_); + internal_data_len_ = 0; + expected_frame_size_ = 0; + // fall through to current-buffer parsing below + } + // First, try to complete a partial message from the internal buffer. // partial_len = bytes in internal buffer that came from a PREVIOUS add_data call // (i.e., before bytes_appended_to_internal_ were added this call). @@ -847,6 +860,42 @@ class AccumulatingReader { return with_diagnostics(result); } + // Garbage prefix saved as a partial (WaitingForStart), or a frame that + // can never complete because its claimed size exceeds the internal + // buffer (still collecting with the buffer full): resync inside the + // internal buffer instead of waiting forever. + if (result.status == FrameMsgStatus::WaitingForStart || + internal_data_len_ >= BufferSize) { + size_t discard = internal_data_len_; + if constexpr (Config::num_start_bytes >= 1) { + if (internal_data_len_ > 1) { + const void* p = std::memchr(internal_buffer_ + 1, + static_cast(Config::computed_start_byte1()), + internal_data_len_ - 1); + if (p) discard = static_cast(static_cast(p) - internal_buffer_); + } + } else { + // No start bytes (e.g. IPC): advance one byte and retry. + discard = 1; + } + diagnostics_.cnt_sync_recoveries++; + diagnostics_.cnt_failed_bytes += static_cast(discard); + size_t keep = internal_data_len_ - discard; + if (keep > 0) { + std::memmove(internal_buffer_, internal_buffer_ + discard, keep); + } + // Only the portion of the discard that reached into this cycle's + // appended bytes reduces the appended count. + if (discard > partial_len) { + bytes_appended_to_internal_ -= (discard - partial_len); + } + internal_data_len_ = keep; + FrameMsgInfo r; + r.status = FrameMsgStatus::SyncRecovery; + r.frame_size = discard; + return with_diagnostics(r); + } + // Still not enough data for a complete message — wait for next add_data() return with_diagnostics(FrameMsgInfo()); } @@ -910,6 +959,11 @@ class AccumulatingReader { internal_data_len_ = remaining; bytes_appended_to_internal_ = 0; current_offset_ = current_size_; + } else if (remaining >= BufferSize) { + // Partial too large to buffer — discard with diagnostics. + diagnostics_.cnt_sync_recoveries++; + diagnostics_.cnt_failed_bytes += static_cast(remaining); + current_offset_ = current_size_; } return with_diagnostics(FrameMsgInfo()); diff --git a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/network_transports.hpp b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/network_transports.hpp index 683c6845..f587f0b6 100644 --- a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/network_transports.hpp +++ b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/network_transports.hpp @@ -9,6 +9,33 @@ #include #include +namespace structframe { +namespace sdk { +namespace detail { + +/** + * Stop the io_context and reap its thread safely. + * + * When called from within an ASIO handler (i.e. on the io thread itself — + * e.g. a subscriber callback that decides to disconnect), joining would + * deadlock/throw, so the thread is detached instead; the stopped context + * makes run() return promptly on its own. + */ +inline void stop_and_reap_io_thread(asio::io_context& ctx, std::thread& io_thread) { + ctx.stop(); + if (io_thread.joinable()) { + if (io_thread.get_id() == std::this_thread::get_id()) { + io_thread.detach(); + } else { + io_thread.join(); + } + } +} + +} // namespace detail +} // namespace sdk +} // namespace structframe + namespace structframe { namespace sdk { @@ -98,13 +125,10 @@ class UdpTransport : public BaseTransport { void Disconnect() { connected_ = false; - io_context_.stop(); + detail::stop_and_reap_io_thread(io_context_, io_thread_); if (socket_.is_open()) { socket_.close(); } - if (io_thread_.joinable()) { - io_thread_.join(); - } io_context_.restart(); } @@ -159,6 +183,11 @@ class TcpTransport : public BaseTransport { } else { const std::string err = "TCP receive error: " + error.message(); HandleError(err.c_str()); + // A fatal receive error ends the receive loop — report the + // link as closed instead of dying silently. + if (connected_ && error != asio::error::operation_aborted) { + HandleClose(); + } } } } @@ -204,15 +233,12 @@ class TcpTransport : public BaseTransport { void Disconnect() { connected_ = false; - io_context_.stop(); + detail::stop_and_reap_io_thread(io_context_, io_thread_); if (socket_.is_open()) { asio::error_code ec; socket_.shutdown(asio::ip::tcp::socket::shutdown_both, ec); socket_.close(); } - if (io_thread_.joinable()) { - io_thread_.join(); - } io_context_.restart(); } @@ -233,9 +259,12 @@ class TcpTransport : public BaseTransport { }; /** - * Serial Transport using ASIO serial port + * Serial Transport using ASIO serial port. + * (Named AsioSerialTransportConfig to avoid clashing with the embedded + * SerialTransportConfig in serial_transport.hpp when both are included + * via the sdk.hpp umbrella header.) */ -struct SerialTransportConfig : public TransportConfig { +struct AsioSerialTransportConfig : public TransportConfig { std::string port; uint32_t baudRate = 115200; size_t bufferSize = 4096; @@ -250,7 +279,7 @@ class AsioSerialTransport : public BaseTransport { asio::serial_port serial_port_; std::thread io_thread_; std::vector receive_buffer_; - SerialTransportConfig serial_config_; + AsioSerialTransportConfig serial_config_; void startReceive() { serial_port_.async_read_some( @@ -264,13 +293,18 @@ class AsioSerialTransport : public BaseTransport { } else if (error) { const std::string err = "Serial receive error: " + error.message(); HandleError(err.c_str()); + // A fatal receive error ends the receive loop — report the + // link as closed instead of dying silently. + if (connected_ && error != asio::error::operation_aborted) { + HandleClose(); + } } } ); } public: - AsioSerialTransport(const SerialTransportConfig& config) + AsioSerialTransport(const AsioSerialTransportConfig& config) : BaseTransport(config), serial_port_(io_context_), receive_buffer_(config.bufferSize), @@ -312,13 +346,10 @@ class AsioSerialTransport : public BaseTransport { void Disconnect() { connected_ = false; - io_context_.stop(); + detail::stop_and_reap_io_thread(io_context_, io_thread_); if (serial_port_.is_open()) { serial_port_.close(); } - if (io_thread_.joinable()) { - io_thread_.join(); - } io_context_.restart(); } diff --git a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/transport.hpp b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/transport.hpp index fd5f6c88..44cfc3fd 100644 --- a/src/struct_frame/boilerplate/cpp/struct_frame_sdk/transport.hpp +++ b/src/struct_frame/boilerplate/cpp/struct_frame_sdk/transport.hpp @@ -3,6 +3,7 @@ #pragma once +#include #include #include @@ -36,7 +37,9 @@ using CloseCallbackFn = void (*)(void*); */ class BaseTransport { protected: - bool connected_ = false; + // Atomic: written by Connect()/Disconnect() on the user thread and read by + // receive handlers on the transport's I/O thread. + std::atomic connected_{false}; DataCallbackFn data_callback_ = nullptr; void* data_user_data_ = nullptr; ErrorCallbackFn error_callback_ = nullptr; @@ -73,16 +76,20 @@ class BaseTransport { /** * Attempt to reconnect after an error or close event. - * BaseTransport provides no implementation — concrete transports must + * + * Virtual so that HandleError()/HandleClose() (which live in this base + * class) actually reach a concrete transport's implementation — with a + * non-virtual method the auto_reconnect config was a silent no-op. + * The default implementation does nothing; concrete transports must * override this method to support auto-reconnect. */ - void AttemptReconnect() {} + virtual void AttemptReconnect() {} public: BaseTransport(const TransportConfig& config = TransportConfig()) : config_(config) {} - ~BaseTransport() = default; + virtual ~BaseTransport() = default; void OnData(DataCallbackFn callback, void* user_data) { data_callback_ = callback; diff --git a/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs b/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs index 112cb94c..857d5667 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs @@ -154,21 +154,35 @@ public FrameMsgInfo Next() // --- Try to complete a partial message from the internal buffer --- if (_internalDataLen > 0) { - if (_bytesAppendedToInternal == 0) + if (_bytesAppendedToInternal == 0 && _currentSize > _currentOffset) { - // Leftover from a previous call where the internal buffer was already full - // and nothing new could be appended. Discard to avoid an infinite wedge. + // Full-wedge escape: new data arrived but nothing could be appended + // because the internal buffer is full. The buffered bytes can never + // complete — discard them so parsing continues from the current + // buffer instead of stalling forever. + _diagnostics.CntSyncRecoveries++; + _diagnostics.CntFailedBytes += _internalDataLen; _internalDataLen = 0; } + else if (_bytesAppendedToInternal == 0) + { + // Partial was saved from the current buffer this cycle (or no new + // data arrived) — wait for more data before deciding anything. + return StatusResult(FrameMsgStatus.Collecting); + } else { var result = _parser.Parse(_internalBuffer, 0, _internalDataLen); - if (result.Status == FrameMsgStatus.WaitingForStart) + if (result.Status == FrameMsgStatus.WaitingForStart || + (result.Status == FrameMsgStatus.Collecting && _internalDataLen >= _bufferSize)) { - // Garbage at the start of the internal buffer. Byte 0 is not a valid - // start, so scan forward for the next start-byte candidate and discard - // everything before it — at least one byte, guaranteeing forward progress. + // Garbage at the start of the internal buffer (byte 0 is not a + // valid start), or a frame that can never complete because its + // claimed size exceeds the internal buffer (still collecting with + // the buffer full). Scan forward for the next start-byte candidate + // and discard everything before it — at least one byte, + // guaranteeing forward progress. int searchLen = _internalDataLen - 1; int found = (searchLen > 0 && _config.NumStartBytes > 0) ? Array.IndexOf(_internalBuffer, _config.ComputedStartByte1, 1, searchLen) @@ -176,13 +190,17 @@ public FrameMsgInfo Next() // No start bytes (e.g. IPC profile): discard one byte at a time so a // single unknown msg_id doesn't drop the rest of the buffered data. int discard = found > 0 ? found : (_config.NumStartBytes == 0 ? 1 : _internalDataLen); + int internalPrior = _internalDataLen - _bytesAppendedToInternal; _diagnostics.CntSyncRecoveries++; _diagnostics.CntFailedBytes += discard; int keep = _internalDataLen - discard; if (keep > 0) Array.Copy(_internalBuffer, discard, _internalBuffer, 0, keep); _internalDataLen = keep; - _bytesAppendedToInternal = Math.Max(0, _bytesAppendedToInternal - discard); + // Only the portion of the discard that reached into this cycle's + // appended bytes reduces the appended count. + if (discard > internalPrior) + _bytesAppendedToInternal -= (discard - internalPrior); return StatusResult(FrameMsgStatus.SyncRecovery, discard); } diff --git a/src/struct_frame/boilerplate/csharp/Framework/Sdk/StructFrameSdk.cs b/src/struct_frame/boilerplate/csharp/Framework/Sdk/StructFrameSdk.cs index 39792c6c..75e95aa1 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Sdk/StructFrameSdk.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Sdk/StructFrameSdk.cs @@ -5,6 +5,7 @@ #nullable enable using System; +using System.Buffers; using System.Collections.Generic; using System.Runtime.InteropServices; using System.Threading; @@ -373,19 +374,29 @@ public async Task SendAsync(T message, byte seq = 0, byte sysId = /// public async Task SendRawAsync(IStructFrameMessage message, byte seq = 0, byte sysId = 0, byte compId = 0) { - byte[] buffer = new byte[_profile.MaxPayload + _profile.Overhead]; - int bytesWritten = _encoder.Encode(buffer, 0, message, seq, sysId, compId); - if (bytesWritten == 0) + // Pooled: MaxPayload + Overhead is ~64KB for Bulk/Network profiles, which + // would otherwise be a fresh large allocation on every send. + byte[] buffer = ArrayPool.Shared.Rent(_profile.MaxPayload + _profile.Overhead); + try { - throw new InvalidOperationException("Failed to encode message - buffer too small or payload exceeds max size"); - } + int bytesWritten = _encoder.Encode(buffer, 0, message, seq, sysId, compId); + if (bytesWritten == 0) + { + throw new InvalidOperationException("Failed to encode message - buffer too small or payload exceeds max size"); + } - // buffer is a fresh, non-reused allocation; hand a view of the written bytes to - // the transport directly. The strict-ordering queue makes its own owned copy. - var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); + // The non-queued path awaits the transport's SendAsync to completion before + // the buffer is returned to the pool; the strict-ordering queue makes its + // own owned copy synchronously before enqueueing. + var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); - Log($"Sent message ID {message.GetMsgId()}, {bytesWritten} bytes total"); - return result; + Log($"Sent message ID {message.GetMsgId()}, {bytesWritten} bytes total"); + return result; + } + finally + { + ArrayPool.Shared.Return(buffer); + } } /// @@ -395,16 +406,23 @@ public async Task SendRawAsync(IStructFrameMessage message, byte seq /// public async Task SendRawAsync(ushort msgId, ReadOnlyMemory payload, byte seq = 0, byte sysId = 0, byte compId = 0, byte pkgId = 0) { - byte[] buffer = new byte[_profile.MaxPayload + _profile.Overhead]; - var info = _getMessageInfo(msgId); - int bytesWritten = _encoder.EncodeRaw(buffer, 0, msgId, payload.Span, seq, sysId, compId, pkgId, info); - if (bytesWritten == 0) + byte[] buffer = ArrayPool.Shared.Rent(_profile.MaxPayload + _profile.Overhead); + try + { + var info = _getMessageInfo(msgId); + int bytesWritten = _encoder.EncodeRaw(buffer, 0, msgId, payload.Span, seq, sysId, compId, pkgId, info); + if (bytesWritten == 0) + { + throw new InvalidOperationException("Failed to encode raw payload — buffer too small or payload exceeds max size"); + } + var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); + Log($"Sent raw message ID {msgId}, {bytesWritten} bytes total"); + return result; + } + finally { - throw new InvalidOperationException("Failed to encode raw payload — buffer too small or payload exceeds max size"); + ArrayPool.Shared.Return(buffer); } - var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); - Log($"Sent raw message ID {msgId}, {bytesWritten} bytes total"); - return result; } /// @@ -417,27 +435,34 @@ public async Task ReencodeAsync(FrameMsgInfo frame) throw new InvalidOperationException("Cannot re-encode an empty frame"); } - byte[] buffer = new byte[_profile.MaxPayload + _profile.Overhead]; - var info = _getMessageInfo(frame.MsgId); - int bytesWritten = _encoder.EncodeRaw( - buffer, - 0, - frame.MsgId, - frame.GetPayloadSpan(), - frame.Seq, - frame.SysId, - frame.CompId, - frame.PkgId, - info); + byte[] buffer = ArrayPool.Shared.Rent(_profile.MaxPayload + _profile.Overhead); + try + { + var info = _getMessageInfo(frame.MsgId); + int bytesWritten = _encoder.EncodeRaw( + buffer, + 0, + frame.MsgId, + frame.GetPayloadSpan(), + frame.Seq, + frame.SysId, + frame.CompId, + frame.PkgId, + info); + + if (bytesWritten == 0) + { + throw new InvalidOperationException("Failed to re-encode frame - buffer too small or payload exceeds max size"); + } - if (bytesWritten == 0) + var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); + Log($"Re-encoded and sent frame ID {frame.MsgId}, {bytesWritten} bytes total"); + return result; + } + finally { - throw new InvalidOperationException("Failed to re-encode frame - buffer too small or payload exceeds max size"); + ArrayPool.Shared.Return(buffer); } - - var result = await SendFramedBytesAsync(buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); - Log($"Re-encoded and sent frame ID {frame.MsgId}, {bytesWritten} bytes total"); - return result; } /// @@ -497,6 +522,24 @@ private void ProcessFrame(FrameMsgInfo frame) { Log($"Received message ID {frame.MsgId}, {frame.MsgLen} bytes payload, valid={frame.Valid}"); + IMessageHandler[]? handlersCopy = null; + lock (_handlersLock) + { + // COW array: safe to read the reference under the lock and iterate outside it + // without copying — Subscribe/Unsubscribe never mutate an array in place. + if (_messageHandlers.TryGetValue(frame.MsgId, out var handlers) && handlers.Length > 0) + handlersCopy = handlers; + } + + // Nobody is listening for this frame: skip the per-frame clone entirely. + bool hasTap = FrameReceived != null; + bool hasTyped = frame.Valid && handlersCopy != null; + bool hasUnhandled = frame.Valid && handlersCopy == null && UnhandledMessage != null; + if (!hasTap && !hasTyped && !hasUnhandled) + { + return; + } + // Clone frame bytes before dispatch so callback consumers never observe // transport buffer reuse from subsequent reads. FrameMsgInfo dispatchFrame = CloneFrameForDispatch(frame); @@ -510,15 +553,6 @@ private void ProcessFrame(FrameMsgInfo frame) return; } - IMessageHandler[]? handlersCopy = null; - lock (_handlersLock) - { - // COW array: safe to read the reference under the lock and iterate outside it - // without copying — Subscribe/Unsubscribe never mutate an array in place. - if (_messageHandlers.TryGetValue(dispatchFrame.MsgId, out var handlers) && handlers.Length > 0) - handlersCopy = handlers; - } - if (handlersCopy != null) { foreach (var handler in handlersCopy) diff --git a/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transport.cs b/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transport.cs index 632bf501..9eb75008 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transport.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transport.cs @@ -106,10 +106,12 @@ public interface IBufferReceiveTransport /// public abstract class BaseTransport : ITransport, IBufferReceiveTransport, IDisposable { - protected bool _connected; + // Volatile: written on connect/disconnect paths and read from receive threads. + protected volatile bool _connected; protected TransportConfig _config; protected int _reconnectAttempts; private readonly SemaphoreSlim _sendSemaphore = new SemaphoreSlim(1, 1); + private int _reconnectInProgress; private bool _disposed; public event EventHandler? DataReceived; @@ -225,25 +227,48 @@ protected void OnConnectionClosed() } } + /// + /// Reconnect loop: keeps retrying (honouring ReconnectDelayMs and + /// MaxReconnectAttempts, 0 = infinite) until the transport reconnects. + /// Retries are driven by this loop directly — the previous implementation + /// relied on OnErrorOccurred to re-trigger the next attempt, which never + /// fired after a close because its gate requires _connected, so + /// "infinite" reconnects actually stopped after a single attempt. + /// Guarded so concurrent error/close events start at most one loop. + /// protected async Task AttemptReconnectAsync() { - if (_config.MaxReconnectAttempts > 0 && - _reconnectAttempts >= _config.MaxReconnectAttempts) + if (Interlocked.Exchange(ref _reconnectInProgress, 1) == 1) { return; } - _reconnectAttempts++; - await Task.Delay(_config.ReconnectDelayMs); - try { - await ConnectAsync(); - _reconnectAttempts = 0; + while (!_connected && !_disposed && + (_config.MaxReconnectAttempts == 0 || + _reconnectAttempts < _config.MaxReconnectAttempts)) + { + _reconnectAttempts++; + await Task.Delay(_config.ReconnectDelayMs).ConfigureAwait(false); + + try + { + await ConnectAsync().ConfigureAwait(false); + _reconnectAttempts = 0; + return; + } + catch (Exception ex) + { + // Report the failed attempt without re-triggering another + // reconnect loop through OnErrorOccurred. + ErrorOccurred?.Invoke(this, ex); + } + } } - catch (Exception ex) + finally { - OnErrorOccurred(ex); + Interlocked.Exchange(ref _reconnectInProgress, 0); } } diff --git a/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transports/SerialTransport.cs b/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transports/SerialTransport.cs index 0796636a..131eb475 100644 --- a/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transports/SerialTransport.cs +++ b/src/struct_frame/boilerplate/csharp/Framework/Sdk/Transports/SerialTransport.cs @@ -129,23 +129,6 @@ await Task.Run(() => } } - private async Task RunReceiveLoopAsync() - { - try - { - if (_readCts != null) - await ReadLoopAsync(_readCts.Token); - } - catch (Exception ex) - { - if (_connected) - { - OnErrorOccurred(ex); - OnConnectionClosed(); - } - } - } - private async Task ReadLoopAsync(System.Threading.CancellationToken cancellationToken) { byte[] buffer = new byte[4096]; @@ -195,6 +178,15 @@ await Task.Factory.StartNew(() => break; } } + + // The read loop ended without an explicit disconnect (e.g. the + // cable was unplugged and Read threw IOException). Surface the + // dead link — firing ConnectionClosed and auto-reconnect — + // instead of leaving IsConnected stuck at true. + if (!cancellationToken.IsCancellationRequested && _connected) + { + OnConnectionClosed(); + } }, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default).ConfigureAwait(false); } } diff --git a/src/struct_frame/boilerplate/js/frame-profiles.js b/src/struct_frame/boilerplate/js/frame-profiles.js index 48447d46..31ad2f70 100644 --- a/src/struct_frame/boilerplate/js/frame-profiles.js +++ b/src/struct_frame/boilerplate/js/frame-profiles.js @@ -560,8 +560,11 @@ class AccumulatingReader { this.config = config; this.getMessageInfo = getMessageInfo; this.bufferSize = bufferSize; + this.headerSize = profileHeaderSize(config); + this.footerSize = profileFooterSize(config); this.internalBuffer = new Uint8Array(bufferSize); this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; this._state = AccumulatingReaderState.IDLE; this.currentBuffer = null; @@ -581,20 +584,17 @@ class AccumulatingReader { this.currentSize = buffer.length; this.currentOffset = 0; this._state = AccumulatingReaderState.BUFFER_MODE; - // If we have partial data in internal buffer, try to complete it + this.bytesAppendedToInternal = 0; + // If we have partial data in internal buffer, append as much as fits to + // try to complete it (the remainder stays in the current buffer). if (this.internalDataLen > 0) { const spaceAvailable = this.bufferSize - this.internalDataLen; - if (buffer.length <= spaceAvailable) { - this.internalBuffer.set(buffer, this.internalDataLen); - this.internalDataLen += buffer.length; - } - else { - // Partial data won't fit: discard it and start fresh from the new buffer - this._diagnostics.cntFailedBytes += this.internalDataLen; - this._diagnostics.cntSyncRecoveries++; - this.internalDataLen = 0; - this.expectedFrameSize = 0; + const bytesToCopy = Math.min(buffer.length, spaceAvailable); + if (bytesToCopy > 0) { + this.internalBuffer.set(buffer.subarray(0, bytesToCopy), this.internalDataLen); + this.internalDataLen += bytesToCopy; } + this.bytesAppendedToInternal = bytesToCopy; } } /** @@ -604,39 +604,86 @@ class AccumulatingReader { if (this._state !== AccumulatingReaderState.BUFFER_MODE) { return this.withDiagnostics((0, frame_base_1.createFrameMsgInfo)()); } + // Full-wedge escape: new data arrived but nothing could be appended + // because the internal buffer is full. The buffered bytes can never + // complete — discard them so parsing continues from the current buffer + // instead of stalling forever. + if (this.internalDataLen > 0 && this.currentOffset === 0 && + this.bytesAppendedToInternal === 0 && this.currentSize > this.currentOffset) { + this._diagnostics.cntSyncRecoveries++; + this._diagnostics.cntFailedBytes += this.internalDataLen; + this.internalDataLen = 0; + this.expectedFrameSize = 0; + // fall through to current-buffer parsing below + } // First, try to complete a partial message from the internal buffer if (this.internalDataLen > 0 && this.currentOffset === 0) { const internalBytes = this.internalBuffer.subarray(0, this.internalDataLen); const result = this.parseBuffer(internalBytes); + // Bytes already in the internal buffer before this addData() appended to it + const partialLen = this.internalDataLen - this.bytesAppendedToInternal; if (result.valid) { result.msgData = result.msgData.slice(); // own copy — internalBuffer is reused this.recordFrameDiagnostics(this.internalBuffer, true); - const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); - const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; + const frameSize = this.headerSize + result.msgLen + this.footerSize; const bytesFromCurrent = frameSize > partialLen ? frameSize - partialLen : 0; this.currentOffset = bytesFromCurrent; this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; return this.withDiagnostics(result); } - else { - if (result.status === frame_base_1.FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { - // A complete-but-CRC-failed frame was assembled across chunks. Surface it - // (status=CrcFailure, frameSize set) and advance past it, mirroring the - // valid-frame path so subsequent frames are read from the current buffer. - this._diagnostics.cntCrcFailures++; - this._diagnostics.cntFailedBytes += result.frameSize; - this._diagnostics.cntSyncRecoveries++; - this.recordFrameDiagnostics(this.internalBuffer, false); - const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; - const bytesFromCurrent = result.frameSize > partialLen ? result.frameSize - partialLen : 0; - this.currentOffset = bytesFromCurrent; - this.internalDataLen = 0; - this.expectedFrameSize = 0; - return this.withDiagnostics(result); + if (result.status === frame_base_1.FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { + // A complete-but-CRC-failed frame was assembled across chunks. Surface it + // (status=CrcFailure, frameSize set) and advance past it, mirroring the + // valid-frame path so subsequent frames are read from the current buffer. + this._diagnostics.cntCrcFailures++; + this._diagnostics.cntFailedBytes += result.frameSize; + this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(this.internalBuffer, false); + const bytesFromCurrent = result.frameSize > partialLen ? result.frameSize - partialLen : 0; + this.currentOffset = bytesFromCurrent; + this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; + this.expectedFrameSize = 0; + return this.withDiagnostics(result); + } + // Garbage prefix saved as a partial (WaitingForStart), or a frame + // that can never complete because its claimed size exceeds the + // internal buffer (still collecting with the buffer full): resync + // inside the internal buffer instead of waiting forever. + if (result.status === frame_base_1.FrameMsgStatus.WaitingForStart || + this.internalDataLen >= this.bufferSize) { + let discard = this.internalDataLen; + if (this.config.header.numStartBytes >= 1) { + const nxt = internalBytes.indexOf(this.config.startByte1, 1); + if (nxt !== -1) { + discard = nxt; + } } - return this.withDiagnostics((0, frame_base_1.createFrameMsgInfo)()); + else { + // No start bytes (e.g. IPC): advance one byte and retry. + discard = 1; + } + this._diagnostics.cntSyncRecoveries++; + this._diagnostics.cntFailedBytes += discard; + const keep = this.internalDataLen - discard; + if (keep > 0) { + this.internalBuffer.copyWithin(0, discard, this.internalDataLen); + } + // Only the portion of the discard that reached into this cycle's + // appended bytes reduces the appended count. + if (discard > partialLen) { + this.bytesAppendedToInternal -= (discard - partialLen); + } + this.internalDataLen = keep; + const r = (0, frame_base_1.createFrameMsgInfo)(); + r.status = frame_base_1.FrameMsgStatus.SyncRecovery; + r.frameSize = discard; + return this.withDiagnostics(r); } + // Still not enough data for a complete message — wait for next addData() + return this.withDiagnostics((0, frame_base_1.createFrameMsgInfo)()); } // Parse from current buffer if (this.currentBuffer === null || this.currentOffset >= this.currentSize) { @@ -646,7 +693,7 @@ class AccumulatingReader { const result = this.parseBuffer(remaining); if (result.valid) { this.recordFrameDiagnostics(remaining, true); - const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); + const frameSize = this.headerSize + result.msgLen + this.footerSize; this.currentOffset += frameSize; return this.withDiagnostics(result); } @@ -795,8 +842,8 @@ class AccumulatingReader { return this.withDiagnostics(r); } this.internalBuffer[this.internalDataLen++] = byte; - const headerSize = profileHeaderSize(this.config); - const footerSize = profileFooterSize(this.config); + const headerSize = this.headerSize; + const footerSize = this.footerSize; if (this.internalDataLen >= headerSize) { if (!this.config.payload.hasLength && !this.config.payload.hasCrc) { const msgId = this.internalBuffer[headerSize - 1]; @@ -1037,6 +1084,7 @@ class AccumulatingReader { /** Reset the reader, clearing any partial message data. */ reset() { this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; this._state = AccumulatingReaderState.IDLE; this.currentBuffer = null; @@ -1080,7 +1128,7 @@ class AccumulatingReader { else { payloadLen = frame[lenOffset] | (frame[lenOffset + 1] << 8); } - const headerSize = profileHeaderSize(config); + const headerSize = this.headerSize; let fullMsgId = 0; if (config.payload.hasPkgId) { fullMsgId = frame[headerSize - 2] << 8; diff --git a/src/struct_frame/boilerplate/py/frame_profiles.py b/src/struct_frame/boilerplate/py/frame_profiles.py index 1a5084f2..27e3967c 100644 --- a/src/struct_frame/boilerplate/py/frame_profiles.py +++ b/src/struct_frame/boilerplate/py/frame_profiles.py @@ -742,21 +742,24 @@ def __init__(self, config: ProfileConfig, buffer: bytes, """ self._config = config self._buffer = buffer + # memoryview: O(1) zero-copy slicing in next() — a plain bytes slice + # would copy the whole remaining buffer per frame (O(n^2) overall). + self._view = memoryview(buffer) self._size = len(buffer) self._offset = 0 self._get_message_info = get_message_info - + def next(self) -> FrameMsgInfo: """ Parse the next frame in the buffer. - + Returns: FrameMsgInfo with valid=True if successful, valid=False if no more frames. """ if self._offset >= self._size: return FrameMsgInfo() - - remaining = self._buffer[self._offset:] + + remaining = self._view[self._offset:] if self._config.has_crc or self._config.has_length: result = _frame_format_parse_with_crc(self._config, remaining, self._get_message_info) @@ -958,6 +961,11 @@ def __init__(self, config: ProfileConfig, self._get_message_info = get_message_info self._buffer_size = buffer_size + # Cached per-profile constants so the per-byte push path doesn't + # recompute them on every call. + self._start_byte1 = config.computed_start_byte1() + self._start_byte2 = config.computed_start_byte2() + # Internal buffer for partial messages self._internal_buffer = bytearray(buffer_size) self._internal_data_len = 0 @@ -970,6 +978,7 @@ def __init__(self, config: ProfileConfig, # Buffer mode state self._current_buffer: Optional[bytes] = None + self._current_view: Optional[memoryview] = None self._current_size = 0 self._current_offset = 0 @@ -994,10 +1003,13 @@ def add_data(self, buffer: bytes): buffer: New data to process """ self._current_buffer = buffer + # memoryview: O(1) zero-copy slicing in next() — a plain bytes slice + # would copy the whole remaining buffer per frame (O(n^2) overall). + self._current_view = memoryview(buffer) self._current_size = len(buffer) self._current_offset = 0 self._state = AccumulatingReaderState.BUFFER_MODE - + # If we have partial data in internal buffer, try to complete it self._bytes_appended_to_internal = 0 if self._internal_data_len > 0: @@ -1017,6 +1029,19 @@ def next(self) -> FrameMsgInfo: if self._state != AccumulatingReaderState.BUFFER_MODE: return self._with_diag(FrameMsgInfo()) + # Full-wedge escape: new data arrived but nothing could be appended + # because the internal buffer is full. The buffered bytes can never + # complete — discard them so parsing continues from the current buffer + # instead of stalling forever. + if (self._internal_data_len > 0 and self._current_offset == 0 + and self._bytes_appended_to_internal == 0 + and self._current_size > self._current_offset): + self._diag.cnt_sync_recoveries += 1 + self._diag.cnt_failed_bytes += self._internal_data_len + self._internal_data_len = 0 + self._expected_frame_size = 0 + # fall through to current-buffer parsing below + # First, try to complete a partial message from the internal buffer if self._internal_data_len > 0 and self._current_offset == 0: internal_bytes = bytes(self._internal_buffer[:self._internal_data_len]) @@ -1048,6 +1073,35 @@ def next(self) -> FrameMsgInfo: self._expected_frame_size = 0 return self._with_diag(result) + # Garbage prefix saved as a partial (WAITING_FOR_START), or a frame + # that can never complete because its claimed size exceeds the + # internal buffer (still collecting with the buffer full): resync + # inside the internal buffer instead of waiting forever. + if (result.status == FrameMsgStatus.WAITING_FOR_START + or self._internal_data_len >= self._buffer_size): + discard = self._internal_data_len + if self._config.num_start_bytes >= 1: + start1 = bytes([self._config.computed_start_byte1()]) + nxt = internal_bytes.find(start1, 1) + if nxt != -1: + discard = nxt + else: + # No start bytes (e.g. IPC): advance one byte and retry. + discard = 1 + self._diag.cnt_sync_recoveries += 1 + self._diag.cnt_failed_bytes += discard + keep = self._internal_data_len - discard + if keep > 0: + self._internal_buffer[:keep] = self._internal_buffer[discard:self._internal_data_len] + # Only the portion of the discard that reached into this cycle's + # appended bytes reduces the appended count. + if discard > partial_len: + self._bytes_appended_to_internal -= (discard - partial_len) + self._internal_data_len = keep + r = FrameMsgInfo(status=FrameMsgStatus.SYNC_RECOVERY) + r.frame_size = discard + return self._with_diag(r) + # Still not enough data for a complete message — wait for next add_data() return self._with_diag(FrameMsgInfo()) @@ -1055,7 +1109,7 @@ def next(self) -> FrameMsgInfo: if self._current_buffer is None or self._current_offset >= self._current_size: return self._with_diag(FrameMsgInfo()) - remaining = self._current_buffer[self._current_offset:] + remaining = self._current_view[self._current_offset:] result = self._parse_buffer(remaining) if result.valid: @@ -1098,6 +1152,11 @@ def next(self) -> FrameMsgInfo: self._internal_buffer[:remaining_len] = remaining self._internal_data_len = remaining_len self._current_offset = self._current_size + elif remaining_len >= self._buffer_size: + # Partial too large to buffer — discard with diagnostics. + self._diag.cnt_sync_recoveries += 1 + self._diag.cnt_failed_bytes += remaining_len + self._current_offset = self._current_size return self._with_diag(FrameMsgInfo()) @@ -1187,7 +1246,7 @@ def _handle_looking_for_start1(self, byte: int) -> FrameMsgInfo: else: self._state = AccumulatingReaderState.COLLECTING_HEADER else: - if byte == self._config.computed_start_byte1(): + if byte == self._start_byte1: self._internal_buffer[0] = byte self._internal_data_len = 1 @@ -1202,11 +1261,11 @@ def _handle_looking_for_start1(self, byte: int) -> FrameMsgInfo: def _handle_looking_for_start2(self, byte: int) -> FrameMsgInfo: """Handle LOOKING_FOR_START2 state""" - if byte == self._config.computed_start_byte2(): + if byte == self._start_byte2: self._internal_buffer[self._internal_data_len] = byte self._internal_data_len += 1 self._state = AccumulatingReaderState.COLLECTING_HEADER - elif byte == self._config.computed_start_byte1(): + elif byte == self._start_byte1: # Might be start of new frame - restart self._internal_buffer[0] = byte self._internal_data_len = 1 @@ -1494,6 +1553,7 @@ def reset(self): self._expected_frame_size = 0 self._state = AccumulatingReaderState.IDLE self._current_buffer = None + self._current_view = None self._current_size = 0 self._current_offset = 0 self._last_seq = None diff --git a/src/struct_frame/boilerplate/py/struct_frame_sdk/serial_transport.py b/src/struct_frame/boilerplate/py/struct_frame_sdk/serial_transport.py index 88743c8e..b85ffeb4 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_sdk/serial_transport.py +++ b/src/struct_frame/boilerplate/py/struct_frame_sdk/serial_transport.py @@ -77,8 +77,8 @@ def disconnect(self) -> None: self.receive_thread = None self.connected = False - def send(self, data: bytes) -> int: - """Send data via serial port""" + def _send_impl(self, data: bytes) -> int: + """Send data via serial port (serialized by BaseTransport.send).""" if not self.serial_port or not self.connected or not self.serial_port.is_open: raise RuntimeError('Serial port not connected') diff --git a/src/struct_frame/boilerplate/py/struct_frame_sdk/tcp_transport.py b/src/struct_frame/boilerplate/py/struct_frame_sdk/tcp_transport.py index 3756d18a..7fb08d85 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_sdk/tcp_transport.py +++ b/src/struct_frame/boilerplate/py/struct_frame_sdk/tcp_transport.py @@ -43,8 +43,8 @@ def _close_socket(self) -> None: self.socket.close() self.socket = None - def send(self, data: bytes) -> int: - """Send data via TCP""" + def _send_impl(self, data: bytes) -> int: + """Send data via TCP (serialized by BaseTransport.send).""" if not self.socket or not self.connected: raise RuntimeError('TCP socket not connected') diff --git a/src/struct_frame/boilerplate/py/struct_frame_sdk/transport.py b/src/struct_frame/boilerplate/py/struct_frame_sdk/transport.py index de5275e5..34beb36d 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_sdk/transport.py +++ b/src/struct_frame/boilerplate/py/struct_frame_sdk/transport.py @@ -46,7 +46,13 @@ def disconnect(self) -> None: @abstractmethod def send(self, data: bytes) -> int: - """Send data through the transport and return bytes written""" + """Send data through the transport and return bytes written. + + Thread-safe: concurrent calls are serialized so bytes from different + frames never interleave on the wire. Message *ordering* across + concurrent callers is not guaranteed; callers that need ordering must + await/return from each send before issuing the next. + """ pass @abstractmethod @@ -80,6 +86,28 @@ def __init__(self, config: Optional[TransportConfig] = None): self.error_callback: Optional[Callable[[Exception], None]] = None self.close_callback: Optional[Callable[[], None]] = None self.reconnect_attempts = 0 + # Serializes sends so concurrent callers cannot interleave the bytes of + # different frames on the wire (mirrors the C# BaseTransport semaphore). + self._send_lock = threading.Lock() + + def send(self, data: bytes) -> int: + """Send data through the transport, serialized with the send lock. + + Concrete transports implement the actual write in ``_send_impl``; this + template method holds ``_send_lock`` for the duration so two threads + cannot interleave frame bytes on the underlying stream. + """ + with self._send_lock: + return self._send_impl(data) + + @abstractmethod + def _send_impl(self, data: bytes) -> int: + """Perform the actual write. Called while holding ``_send_lock``. + + Concrete transports override this instead of ``send`` so every send is + serialized by the base class. + """ + raise NotImplementedError def set_data_callback(self, callback: Callable[[bytes], None]) -> None: self.data_callback = callback diff --git a/src/struct_frame/boilerplate/py/struct_frame_sdk/udp_transport.py b/src/struct_frame/boilerplate/py/struct_frame_sdk/udp_transport.py index 833f27d3..37441217 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_sdk/udp_transport.py +++ b/src/struct_frame/boilerplate/py/struct_frame_sdk/udp_transport.py @@ -37,8 +37,8 @@ def connect(self) -> None: self._handle_error(e) raise - def send(self, data: bytes) -> int: - """Send data via UDP""" + def _send_impl(self, data: bytes) -> int: + """Send data via UDP (serialized by BaseTransport.send).""" if not self.socket or not self.connected: raise RuntimeError('UDP socket not connected') diff --git a/src/struct_frame/boilerplate/py/struct_frame_sdk/websocket_transport.py b/src/struct_frame/boilerplate/py/struct_frame_sdk/websocket_transport.py index 16b99e20..8df5322c 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_sdk/websocket_transport.py +++ b/src/struct_frame/boilerplate/py/struct_frame_sdk/websocket_transport.py @@ -72,8 +72,8 @@ def disconnect(self) -> None: self.ws_thread = None self.connected = False - def send(self, data: bytes) -> int: - """Send data via WebSocket""" + def _send_impl(self, data: bytes) -> int: + """Send data via WebSocket (serialized by BaseTransport.send).""" if not self.ws or not self.connected: raise RuntimeError('WebSocket not connected') diff --git a/src/struct_frame/boilerplate/rust/frame_profiles.rs b/src/struct_frame/boilerplate/rust/frame_profiles.rs index 54a151e5..5b19fe32 100644 --- a/src/struct_frame/boilerplate/rust/frame_profiles.rs +++ b/src/struct_frame/boilerplate/rust/frame_profiles.rs @@ -971,6 +971,46 @@ impl AccumulatingReader { self.maybe_compact(); } + /// When the buffer is full but parsing still cannot make a decision (e.g. a + /// corrupted length field claims more bytes than the reader's capacity), the + /// pending frame can never complete. Force a resync past the stuck prefix so + /// the reader does not wedge permanently: drain to the next start-byte + /// candidate after the head (at least one byte). + fn forced_resync_if_full(&mut self) -> Option { + if self.buf().len() < self.capacity { + return None; + } + let buf = self.buf(); + let drain = match self.config.header.header_type { + HeaderType::None => 1, + HeaderType::Tiny => { + let sb = self.config.computed_start_byte1(); + buf[1..] + .iter() + .position(|&b| b == sb) + .map(|p| p + 1) + .unwrap_or(buf.len()) + } + HeaderType::Basic => { + let sb2 = get_basic_second_start_byte(self.config.payload.payload_type as u8); + Self::scan_for_basic_start(&buf[1..], sb2) + .map(|p| p + 1) + .unwrap_or(buf.len()) + } + } + .max(1); + + self.diagnostics.cnt_sync_recoveries += 1; + self.diagnostics.cnt_failed_bytes += drain as u32; + self.partial_pending = false; + self.drain_head(drain); + + let mut sync = FrameMsgInfo::invalid(); + sync.status = FrameMsgStatus::SyncRecovery; + sync.frame_size = drain; + Some(sync) + } + /// Increment cnt_len_errors when the frame at the buffer head carries a length /// field outside the [min_size, size] range for its message — matching the /// stream-mode counter semantics of the other language parsers. @@ -1273,12 +1313,20 @@ impl AccumulatingReader { } if result.status == FrameMsgStatus::Collecting { + // A frame whose claimed size exceeds the reader capacity can never + // complete — resync past the stuck prefix instead of wedging. + if let Some(sync) = self.forced_resync_if_full() { + return Some(sync); + } self.partial_pending = true; return None; } let bytes_to_drain = self.bytes_to_drain_for_resync(get_message_info); if bytes_to_drain == 0 { + if let Some(sync) = self.forced_resync_if_full() { + return Some(sync); + } self.partial_pending = true; return None; } diff --git a/src/struct_frame/boilerplate/ts/frame-profiles.ts b/src/struct_frame/boilerplate/ts/frame-profiles.ts index 3bd8047d..43233992 100644 --- a/src/struct_frame/boilerplate/ts/frame-profiles.ts +++ b/src/struct_frame/boilerplate/ts/frame-profiles.ts @@ -765,10 +765,18 @@ export class AccumulatingReader { private config: FrameProfileConfig; private getMessageInfo?: GetMessageInfo; private bufferSize: number; + // Cached per-profile constants so the per-byte push path doesn't recompute + // them on every call. + private readonly headerSize: number; + private readonly footerSize: number; // Internal buffer for partial messages private internalBuffer: Uint8Array; private internalDataLen: number; + // How many bytes from the current buffer were appended into the internal + // buffer by addData(). Used to advance currentOffset correctly after a + // cross-chunk frame completes. + private bytesAppendedToInternal: number; private expectedFrameSize: number; private _state: AccumulatingReaderState; @@ -789,9 +797,12 @@ export class AccumulatingReader { this.config = config; this.getMessageInfo = getMessageInfo; this.bufferSize = bufferSize; + this.headerSize = profileHeaderSize(config); + this.footerSize = profileFooterSize(config); this.internalBuffer = new Uint8Array(bufferSize); this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; this._state = AccumulatingReaderState.IDLE; @@ -815,20 +826,18 @@ export class AccumulatingReader { this.currentSize = buffer.length; this.currentOffset = 0; this._state = AccumulatingReaderState.BUFFER_MODE; + this.bytesAppendedToInternal = 0; - // If we have partial data in internal buffer, try to complete it + // If we have partial data in internal buffer, append as much as fits to + // try to complete it (the remainder stays in the current buffer). if (this.internalDataLen > 0) { const spaceAvailable = this.bufferSize - this.internalDataLen; - if (buffer.length <= spaceAvailable) { - this.internalBuffer.set(buffer, this.internalDataLen); - this.internalDataLen += buffer.length; - } else { - // Partial data won't fit: discard it and start fresh from the new buffer - this._diagnostics.cntFailedBytes += this.internalDataLen; - this._diagnostics.cntSyncRecoveries++; - this.internalDataLen = 0; - this.expectedFrameSize = 0; + const bytesToCopy = Math.min(buffer.length, spaceAvailable); + if (bytesToCopy > 0) { + this.internalBuffer.set(buffer.subarray(0, bytesToCopy), this.internalDataLen); + this.internalDataLen += bytesToCopy; } + this.bytesAppendedToInternal = bytesToCopy; } } @@ -840,41 +849,92 @@ export class AccumulatingReader { return this.withDiagnostics(createFrameMsgInfo()); } + // Full-wedge escape: new data arrived but nothing could be appended + // because the internal buffer is full. The buffered bytes can never + // complete — discard them so parsing continues from the current buffer + // instead of stalling forever. + if (this.internalDataLen > 0 && this.currentOffset === 0 && + this.bytesAppendedToInternal === 0 && this.currentSize > this.currentOffset) { + this._diagnostics.cntSyncRecoveries++; + this._diagnostics.cntFailedBytes += this.internalDataLen; + this.internalDataLen = 0; + this.expectedFrameSize = 0; + // fall through to current-buffer parsing below + } + // First, try to complete a partial message from the internal buffer if (this.internalDataLen > 0 && this.currentOffset === 0) { const internalBytes = this.internalBuffer.subarray(0, this.internalDataLen); const result = this.parseBuffer(internalBytes); + // Bytes already in the internal buffer before this addData() appended to it + const partialLen = this.internalDataLen - this.bytesAppendedToInternal; if (result.valid) { result.msgData = result.msgData.slice(); // own copy — internalBuffer is reused this.recordFrameDiagnostics(this.internalBuffer, true); - const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); - const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; + const frameSize = this.headerSize + result.msgLen + this.footerSize; const bytesFromCurrent = frameSize > partialLen ? frameSize - partialLen : 0; this.currentOffset = bytesFromCurrent; this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; return this.withDiagnostics(result); - } else { - if (result.status === FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { - // A complete-but-CRC-failed frame was assembled across chunks. Surface it - // (status=CrcFailure, frameSize set) and advance past it, mirroring the - // valid-frame path so subsequent frames are read from the current buffer. - this._diagnostics.cntCrcFailures++; - this._diagnostics.cntFailedBytes += result.frameSize!; - this._diagnostics.cntSyncRecoveries++; - this.recordFrameDiagnostics(this.internalBuffer, false); - const partialLen = this.internalDataLen > this.currentSize ? this.internalDataLen - this.currentSize : 0; - const bytesFromCurrent = result.frameSize! > partialLen ? result.frameSize! - partialLen : 0; - this.currentOffset = bytesFromCurrent; - this.internalDataLen = 0; - this.expectedFrameSize = 0; - return this.withDiagnostics(result); + } + + if (result.status === FrameMsgStatus.CrcFailure && (result.frameSize ?? 0) > 0) { + // A complete-but-CRC-failed frame was assembled across chunks. Surface it + // (status=CrcFailure, frameSize set) and advance past it, mirroring the + // valid-frame path so subsequent frames are read from the current buffer. + this._diagnostics.cntCrcFailures++; + this._diagnostics.cntFailedBytes += result.frameSize!; + this._diagnostics.cntSyncRecoveries++; + this.recordFrameDiagnostics(this.internalBuffer, false); + const bytesFromCurrent = result.frameSize! > partialLen ? result.frameSize! - partialLen : 0; + this.currentOffset = bytesFromCurrent; + this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; + this.expectedFrameSize = 0; + return this.withDiagnostics(result); + } + + // Garbage prefix saved as a partial (WaitingForStart), or a frame + // that can never complete because its claimed size exceeds the + // internal buffer (still collecting with the buffer full): resync + // inside the internal buffer instead of waiting forever. + if (result.status === FrameMsgStatus.WaitingForStart || + this.internalDataLen >= this.bufferSize) { + let discard = this.internalDataLen; + if (this.config.header.numStartBytes >= 1) { + const nxt = internalBytes.indexOf(this.config.startByte1, 1); + if (nxt !== -1) { + discard = nxt; + } + } else { + // No start bytes (e.g. IPC): advance one byte and retry. + discard = 1; } - return this.withDiagnostics(createFrameMsgInfo()); + this._diagnostics.cntSyncRecoveries++; + this._diagnostics.cntFailedBytes += discard; + const keep = this.internalDataLen - discard; + if (keep > 0) { + this.internalBuffer.copyWithin(0, discard, this.internalDataLen); + } + // Only the portion of the discard that reached into this cycle's + // appended bytes reduces the appended count. + if (discard > partialLen) { + this.bytesAppendedToInternal -= (discard - partialLen); + } + this.internalDataLen = keep; + const r = createFrameMsgInfo(); + r.status = FrameMsgStatus.SyncRecovery; + r.frameSize = discard; + return this.withDiagnostics(r); } + + // Still not enough data for a complete message — wait for next addData() + return this.withDiagnostics(createFrameMsgInfo()); } // Parse from current buffer @@ -887,7 +947,7 @@ export class AccumulatingReader { if (result.valid) { this.recordFrameDiagnostics(remaining, true); - const frameSize = profileHeaderSize(this.config) + result.msgLen + profileFooterSize(this.config); + const frameSize = this.headerSize + result.msgLen + this.footerSize; this.currentOffset += frameSize; return this.withDiagnostics(result); } @@ -1042,8 +1102,8 @@ export class AccumulatingReader { } this.internalBuffer[this.internalDataLen++] = byte; - const headerSize = profileHeaderSize(this.config); - const footerSize = profileFooterSize(this.config); + const headerSize = this.headerSize; + const footerSize = this.footerSize; if (this.internalDataLen >= headerSize) { if (!this.config.payload.hasLength && !this.config.payload.hasCrc) { @@ -1285,6 +1345,7 @@ export class AccumulatingReader { /** Reset the reader, clearing any partial message data. */ reset(): void { this.internalDataLen = 0; + this.bytesAppendedToInternal = 0; this.expectedFrameSize = 0; this._state = AccumulatingReaderState.IDLE; this.currentBuffer = null; @@ -1330,7 +1391,7 @@ export class AccumulatingReader { payloadLen = frame[lenOffset] | (frame[lenOffset + 1] << 8); } - const headerSize = profileHeaderSize(config); + const headerSize = this.headerSize; let fullMsgId = 0; if (config.payload.hasPkgId) { fullMsgId = frame[headerSize - 2] << 8; diff --git a/tests/NEGATIVE_TESTS.md b/tests/NEGATIVE_TESTS.md index 355d6720..f45debd6 100644 --- a/tests/NEGATIVE_TESTS.md +++ b/tests/NEGATIVE_TESTS.md @@ -16,51 +16,51 @@ Negative tests are critical for ensuring robust error handling. They verify that ## Test Files -All seven language implementations now share **31 identical test scenarios** (same names, +All seven language implementations now share **33 identical test scenarios** (same names, same behaviour), including a common `tryNext` drain contract, partial-pending checks, diagnostic-counter assertions, minimal-profile resync scenarios, and a chunk-boundary split sweep. All languages also carry the four package-corruption scenarios (bulk `pkg_id`/`msg_id` corruption, cross-package rejection, network `pkg_id` corruption), so -every language runs **35 scenarios**; Python adds 5 status-machine/diagnostic extras (40). +every language runs **37 scenarios**; Python adds 5 status-machine/diagnostic extras (42). ### C Tests (`tests/c/test_negative.c`) -- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios +- **37 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios - Tests buffer reader and accumulating reader (buffer mode) APIs - Uses ProfileStandard, ProfileSensor, ProfileBulk, and ProfileNetwork configurations ### C++ Tests (`tests/cpp/test_negative.cpp`) -- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios +- **37 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios - Tests both BufferReader and AccumulatingReader APIs - Tests multiple frame profiles (Standard, Sensor, Bulk, Network) ### Python Tests (`tests/py/test_negative.py`) -- **40 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios + 5 Python-specific status-machine/diagnostic extras +- **42 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios + 5 Python-specific status-machine/diagnostic extras - Tests both buffer and streaming modes - Uses ProfileStandardReader, ProfileSensorReader, and ProfileNetworkReader ### TypeScript Tests (`tests/ts/test_negative.ts`) -- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios +- **37 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### JavaScript Tests (`tests/js/test_negative.js`) -- **35 test cases** identical to TypeScript +- **37 test cases** identical to TypeScript - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### C# Tests (`tests/csharp/TestNegative.cs`) -- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios +- **37 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios - Tests ProfileStandardWriter/Reader and AccumulatingReader - Tests multiple profiles (Standard, Sensor, Bulk, Network) ### Rust Tests (`tests/rust/src/test_negative.rs`) -- **35 test cases**: the 31 uniform scenarios + 4 package-corruption scenarios +- **37 test cases**: the 33 uniform scenarios + 4 package-corruption scenarios - Tests BufferReader and AccumulatingReader APIs - Tests multiple profiles (Standard, Sensor, Bulk, Network) ## Uniform Test Scenarios -All seven languages implement the following 31 scenarios with identical names: +All seven languages implement the following 33 scenarios with identical names: 1. **Buffer mode: recovers after CRC failure** – buffer-mode accumulating reader resyncs and returns the next valid frame after a CRC-failed frame 2. **Buffer reader: skips CRC-failed frame** – `BufferReader` advances past a CRC-failed frame instead of stalling on it @@ -93,6 +93,8 @@ All seven languages implement the following 31 scenarios with identical names: 29. **Sensor buffer: unknown msg_id resync** – on the Tiny-header (Sensor) profile an unknown msg_id triggers a scan to the next start byte instead of discarding the rest of the buffer 30. **Split sweep: two frames at every boundary** – two back-to-back frames are delivered intact when the stream is split into two `add_data` chunks at *every* possible offset 31. **Streaming: two frames byte-by-byte** – two back-to-back frames are both decoded in byte-at-a-time mode +32. **Buffer mode: garbage prefix partial recovers** – a garbage tail that looks like a truncated frame start is saved as a partial; the reader resyncs inside its internal buffer and keeps delivering subsequent frames (livelock regression) +33. **Buffer mode: oversized length recovers** – a corrupted length field claiming more bytes than the reader’s internal buffer can hold does not wedge the reader permanently (livelock regression) ### `tryNext` Contract (Unified) diff --git a/tests/c/test_negative.c b/tests/c/test_negative.c index 756d3bf4..62ac449f 100644 --- a/tests/c/test_negative.c +++ b/tests/c/test_negative.c @@ -1361,6 +1361,95 @@ bool test_streaming_two_frames(void) { return valid_count == 2; } +/** + * Buffer mode: a garbage tail that looks like a truncated frame start is saved + * as a partial; the reader must resync inside the internal buffer and keep + * delivering the frames that follow (livelock regression test). + */ +bool test_buffer_mode_garbage_prefix_recovers(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + if (frame_size < 6) return false; + + uint8_t chunk1[2064]; + memcpy(chunk1, buffer, frame_size); + chunk1[frame_size] = 0x90; /* looks like a truncated frame start */ + chunk1[frame_size + 1] = 0xFF; + size_t chunk1_len = frame_size + 2; + + uint8_t internal_buffer[1024]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + int valid_count = 0; + bool saw_sync = false; + frame_msg_info_t f; + + accumulating_reader_add_data(&reader, chunk1, chunk1_len); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + else if (f.status == FRAME_MSG_STATUS_SYNC_RECOVERY) saw_sync = true; + } + + for (int i = 0; i < 3; i++) { + accumulating_reader_add_data(&reader, buffer, frame_size); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + else if (f.status == FRAME_MSG_STATUS_SYNC_RECOVERY) saw_sync = true; + } + } + + return valid_count == 4 && saw_sync; +} + +/** + * Buffer mode: a corrupted length field claiming more bytes than the reader's + * internal buffer can hold must not wedge the reader permanently + * (livelock regression test). + */ +bool test_buffer_mode_oversized_length_recovers(void) { + uint8_t buffer[2048]; + buffer_writer_t writer; + buffer_writer_init(&writer, &PROFILE_STANDARD_CONFIG, buffer, sizeof(buffer)); + size_t frame_size = encode_standard_frame(&writer); + if (frame_size < 6) return false; + + /* Bogus header claiming a 255-byte payload — the total (261) exceeds the + * 256-byte reader buffer, so this frame can never complete. */ + uint8_t bogus[4] = {0x90, 0x71, 0xFF, + (uint8_t)(SERIALIZATION_TEST_BASIC_TYPES_MESSAGE_MSG_ID & 0xFF)}; + + uint8_t internal_buffer[256]; + accumulating_reader_t reader; + accumulating_reader_init(&reader, &PROFILE_STANDARD_CONFIG, internal_buffer, sizeof(internal_buffer), get_message_info); + + frame_msg_info_t f; + accumulating_reader_add_data(&reader, bogus, sizeof(bogus)); + while (accumulating_reader_try_next(&reader, &f)) {} + + int valid_count = 0; + size_t rounds = 256 / frame_size + 3; + for (size_t i = 0; i < rounds; i++) { + accumulating_reader_add_data(&reader, buffer, frame_size); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) valid_count++; + } + } + if (valid_count < 1) return false; + + /* The reader must keep delivering fresh frames after recovery. */ + int probe_valid = 0; + for (int i = 0; i < 3; i++) { + accumulating_reader_add_data(&reader, buffer, frame_size); + while (accumulating_reader_try_next(&reader, &f)) { + if (f.valid) probe_valid++; + } + } + return probe_valid >= 1; +} + // Test function pointer type typedef bool (*TestFunc)(void); @@ -1379,6 +1468,8 @@ int main(void) { TestCase tests[] = { {"Buffer mode: CRC failure counters", test_buffer_mode_crc_counters}, {"Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap}, + {"Buffer mode: garbage prefix partial recovers", test_buffer_mode_garbage_prefix_recovers}, + {"Buffer mode: oversized length recovers", test_buffer_mode_oversized_length_recovers}, {"Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure}, {"Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure}, {"Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc}, diff --git a/tests/coverage_spec.py b/tests/coverage_spec.py index f59a7178..3c9cfbc2 100644 --- a/tests/coverage_spec.py +++ b/tests/coverage_spec.py @@ -404,14 +404,14 @@ def _full(label, symbol, notes=""): "intro": ( "Test files: `tests/{c,cpp,py,ts,js,csharp,rust}/test_negative.*`\n\n" "See `tests/NEGATIVE_TESTS.md` for full scenario descriptions.\n\n" - "The 31 scenarios in the table below are registered in every " + "The 33 scenarios in the table below are registered in every " "language's `test_negative.*` file, covering corruption handling, " "the `tryNext` drain contract, diagnostic counters (unified " "semantics in buffer and stream mode), minimal-profile resync, and " "a chunk-boundary split sweep. All seven languages additionally " "carry the four package-corruption scenarios (bulk " "`pkg_id`/`msg_id` corruption, cross-package rejection, network " - "`pkg_id` corruption) for 35 scenarios each; Python (40) adds " + "`pkg_id` corruption) for 37 scenarios each; Python (42) adds " "status-machine and buffer-mode diagnostic extras." ), "tables": [ @@ -422,6 +422,8 @@ def _full(label, symbol, notes=""): "rows": [_full(s, "✅") for s in ( "Buffer mode: CRC failure counters", "Buffer mode: Sequence gap counted", + "Buffer mode: garbage prefix partial recovers", + "Buffer mode: oversized length recovers", "Buffer mode: recovers after CRC failure", "Buffer reader: skips CRC-failed frame", "Bulk profile: Corrupted CRC", diff --git a/tests/cpp/test_negative.cpp b/tests/cpp/test_negative.cpp index 5cabafc6..8d995a0c 100644 --- a/tests/cpp/test_negative.cpp +++ b/tests/cpp/test_negative.cpp @@ -1045,6 +1045,86 @@ bool test_streaming_two_frames() { return valid_count == 2; } +/** + * Buffer mode: a garbage tail that looks like a truncated frame start is saved + * as a partial; the reader must resync inside the internal buffer and keep + * delivering the frames that follow (livelock regression test). + */ +bool test_buffer_mode_garbage_prefix_recovers() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + if (frame_size < 6) return false; + + std::vector chunk1(buffer.begin(), buffer.begin() + frame_size); + chunk1.push_back(0x90); // looks like a truncated frame start + chunk1.push_back(0xFF); + + AccumulatingReader reader(get_message_info); + + int valid_count = 0; + bool saw_sync = false; + FrameMsgInfo f; + + reader.add_data(chunk1.data(), chunk1.size()); + while (reader.try_next(f)) { + if (f.valid) valid_count++; + else if (f.status == FrameMsgStatus::SyncRecovery) saw_sync = true; + } + + for (int i = 0; i < 3; i++) { + reader.add_data(buffer.data(), frame_size); + while (reader.try_next(f)) { + if (f.valid) valid_count++; + else if (f.status == FrameMsgStatus::SyncRecovery) saw_sync = true; + } + } + + return valid_count == 4 && saw_sync; +} + +/** + * Buffer mode: a corrupted length field claiming more bytes than the reader's + * internal buffer can hold must not wedge the reader permanently + * (livelock regression test). + */ +bool test_buffer_mode_oversized_length_recovers() { + std::vector buffer(2048); + BufferWriter writer(buffer.data(), buffer.size()); + size_t frame_size = encode_standard_frame(writer); + if (frame_size < 6) return false; + + // Bogus header claiming a 255-byte payload — the total (261) exceeds the + // 256-byte reader buffer, so this frame can never complete. + uint8_t bogus[4] = {0x90, 0x71, 0xFF, static_cast(standard_message0_id() & 0xFF)}; + + AccumulatingReader reader(get_message_info); + + FrameMsgInfo f; + reader.add_data(bogus, sizeof(bogus)); + while (reader.try_next(f)) {} + + int valid_count = 0; + size_t rounds = 256 / frame_size + 3; + for (size_t i = 0; i < rounds; i++) { + reader.add_data(buffer.data(), frame_size); + while (reader.try_next(f)) { + if (f.valid) valid_count++; + } + } + if (valid_count < 1) return false; + + // The reader must keep delivering fresh frames after recovery. + int probe_valid = 0; + for (int i = 0; i < 3; i++) { + reader.add_data(buffer.data(), frame_size); + while (reader.try_next(f)) { + if (f.valid) probe_valid++; + } + } + return probe_valid >= 1; +} + // Test function pointer type typedef bool (*TestFunc)(); @@ -1063,6 +1143,8 @@ int main() { TestCase tests[] = { {"Buffer mode: CRC failure counters", test_buffer_mode_crc_counters}, {"Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap}, + {"Buffer mode: garbage prefix partial recovers", test_buffer_mode_garbage_prefix_recovers}, + {"Buffer mode: oversized length recovers", test_buffer_mode_oversized_length_recovers}, {"Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure}, {"Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure}, {"Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc}, diff --git a/tests/csharp/TestBaseTransport.cs b/tests/csharp/TestBaseTransport.cs index 4d33acbb..d93fe87b 100644 --- a/tests/csharp/TestBaseTransport.cs +++ b/tests/csharp/TestBaseTransport.cs @@ -183,6 +183,39 @@ static async Task TestAutoReconnectHonored() attemptsTriggered <= 2 && attemptsTriggered >= 1); } + // ------------------------------------------------------------------------- + // F3b. AutoReconnect keeps retrying after failed attempts until it succeeds. + // Regression test: the old implementation relied on OnErrorOccurred to + // re-trigger the next attempt, whose gate requires _connected -- so after a + // close, "infinite" reconnects actually stopped after a single attempt. + // ------------------------------------------------------------------------- + static async Task TestAutoReconnectRetriesUntilSuccess() + { + var transport = new InstrumentedTransport(new TransportConfig + { + AutoReconnect = true, + ReconnectDelayMs = 10, + MaxReconnectAttempts = 0, // infinite + }); + + await transport.ConnectAsync(); // initial connect succeeds + transport.FailNextConnect = 2; // next two reconnect attempts fail + int baseline = transport.ConnectCalls; + + transport.ForceClose(); // link drops -> reconnect loop starts + + // Wait for the loop to work through 2 failures + 1 success. + for (int i = 0; i < 100 && !transport.IsConnected; i++) + { + await Task.Delay(10); + } + + int attempts = transport.ConnectCalls - baseline; + Assert("base-reconnect: retries past failed attempts (got " + attempts + ")", + attempts == 3); + Assert("base-reconnect: eventually reconnects", transport.IsConnected); + } + // ------------------------------------------------------------------------- // F4. Dispose is idempotent and releases the semaphore. // ------------------------------------------------------------------------- @@ -221,6 +254,7 @@ public static int Main(string[] args) TestReceiveMemorySliceLegacyEvent(); TestReceiveMemoryFullBufferLegacyEvent(); TestAutoReconnectHonored().GetAwaiter().GetResult(); + TestAutoReconnectRetriesUntilSuccess().GetAwaiter().GetResult(); TestDisposeIdempotent().GetAwaiter().GetResult(); Console.WriteLine(); diff --git a/tests/csharp/TestNegative.cs b/tests/csharp/TestNegative.cs index 71c98275..48186926 100644 --- a/tests/csharp/TestNegative.cs +++ b/tests/csharp/TestNegative.cs @@ -1106,6 +1106,95 @@ private static bool TestStreamingTwoFrames() } + + /** + * Buffer mode: a garbage tail that looks like a truncated frame start is + * saved as a partial; the reader must resync inside the internal buffer and + * keep delivering the frames that follow (livelock regression test). + */ + private static bool TestBufferModeGarbagePrefixRecovers() + { + byte[] buffer = new byte[2048]; + var sizes = EncodeStandardFrames(buffer, 1); + int frameSize = sizes[0]; + if (frameSize < 6) return false; + + var chunk1 = new byte[frameSize + 2]; + Array.Copy(buffer, chunk1, frameSize); + chunk1[frameSize] = 0x90; // looks like a truncated frame start + chunk1[frameSize + 1] = 0xFF; + + var reader = new AccumulatingReader(1024, SerializationTestMD.GetMessageInfo); + int validCount = 0; + bool sawSync = false; + + reader.AddData(chunk1); + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + else if (f.Status == FrameMsgStatus.SyncRecovery) sawSync = true; + } + + for (int i = 0; i < 3; i++) + { + reader.AddData(buffer, 0, frameSize); + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + else if (f.Status == FrameMsgStatus.SyncRecovery) sawSync = true; + } + } + + return validCount == 4 && sawSync; + } + + /** + * Buffer mode: a corrupted length field claiming more bytes than the + * reader's internal buffer can hold must not wedge the reader permanently + * (livelock regression test). + */ + private static bool TestBufferModeOversizedLengthRecovers() + { + byte[] buffer = new byte[2048]; + var sizes = EncodeStandardFrames(buffer, 1); + int frameSize = sizes[0]; + if (frameSize < 6) return false; + + // Bogus header claiming a 255-byte payload — the total (261) exceeds + // the 256-byte reader buffer, so this frame can never complete. + var bogus = new byte[] { 0x90, 0x71, 0xFF, (byte)(BasicTypesMessage.MsgId & 0xFF) }; + + var reader = new AccumulatingReader(256, SerializationTestMD.GetMessageInfo); + + reader.AddData(bogus); + while (reader.TryNext(out _)) { /* drain */ } + + int validCount = 0; + int rounds = 256 / frameSize + 3; + for (int i = 0; i < rounds; i++) + { + reader.AddData(buffer, 0, frameSize); + while (reader.TryNext(out var f)) + { + if (f.Valid) validCount++; + } + } + if (validCount < 1) return false; + + // The reader must keep delivering fresh frames after recovery. + int probeValid = 0; + for (int i = 0; i < 3; i++) + { + reader.AddData(buffer, 0, frameSize); + while (reader.TryNext(out var f)) + { + if (f.Valid) probeValid++; + } + } + return probeValid >= 1; + } + + public static int Main(string[] args) { Console.WriteLine("\n========================================"); @@ -1117,6 +1206,8 @@ public static int Main(string[] args) { ("Buffer mode: CRC failure counters", TestBufferModeCrcCounters), ("Buffer mode: Sequence gap counted", TestBufferModeSeqGap), + ("Buffer mode: garbage prefix partial recovers", TestBufferModeGarbagePrefixRecovers), + ("Buffer mode: oversized length recovers", TestBufferModeOversizedLengthRecovers), ("Buffer mode: recovers after CRC failure", TestBufferModeRecoverAfterCrcFailure), ("Buffer reader: skips CRC-failed frame", TestBufferReaderSkipsCrcFailed), ("Bulk profile: Corrupted CRC", TestBulkProfileCorruptedCrc), diff --git a/tests/js/test_negative.js b/tests/js/test_negative.js index 44c6adc4..1444aab1 100644 --- a/tests/js/test_negative.js +++ b/tests/js/test_negative.js @@ -985,6 +985,88 @@ function testStreamingTwoFrames() { } + +/** + * Buffer mode: a garbage tail that looks like a truncated frame start is saved + * as a partial; the reader must resync inside the internal buffer and keep + * delivering the frames that follow (livelock regression test). + */ +function testBufferModeGarbagePrefixRecovers() { + const [frames, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 6) return false; + const frame = frames.slice(0, frameSize); + + const chunk1 = new Uint8Array(frameSize + 2); + chunk1.set(frame, 0); + chunk1[frameSize] = 0x90; // looks like a truncated frame start + chunk1[frameSize + 1] = 0xFF; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + let sawSync = false; + let r; + + reader.addData(chunk1); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + + for (let i = 0; i < 3; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + } + + return validCount === 4 && sawSync; +} + +/** + * Buffer mode: a corrupted length field claiming more bytes than the reader's + * internal buffer can hold must not wedge the reader permanently + * (livelock regression test). + */ +function testBufferModeOversizedLengthRecovers() { + const [frames, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 6) return false; + const frame = frames.slice(0, frameSize); + + // Bogus header claiming a 255-byte payload — the total (261) exceeds the + // 256-byte reader buffer, so this frame can never complete. + const bogus = new Uint8Array([0x90, 0x71, 0xFF, BasicTypesMessage._msgid & 0xFF]); + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 256); + let r; + + reader.addData(bogus); + while (tryNextCompat(reader) !== null) { /* drain */ } + + let validCount = 0; + const rounds = Math.floor(256 / frameSize) + 3; + for (let i = 0; i < rounds; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + } + if (validCount < 1) return false; + + // The reader must keep delivering fresh frames after recovery. + let probeValid = 0; + for (let i = 0; i < 3; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) probeValid++; + } + } + return probeValid >= 1; +} + + function main() { console.log('\n========================================'); console.log('NEGATIVE TESTS - JavaScript Parser'); @@ -994,6 +1076,8 @@ function main() { const tests = [ ['Buffer mode: CRC failure counters', testBufferModeCrcCounters], ['Buffer mode: Sequence gap counted', testBufferModeSeqGap], + ['Buffer mode: garbage prefix partial recovers', testBufferModeGarbagePrefixRecovers], + ['Buffer mode: oversized length recovers', testBufferModeOversizedLengthRecovers], ['Buffer mode: recovers after CRC failure', testBufferModeRecoversAfterCrcFailure], ['Buffer reader: skips CRC-failed frame', testBufferReaderSkipsCrcFailure], ['Bulk profile: Corrupted CRC', testBulkProfileCorruptedCrc], diff --git a/tests/py/test_negative.py b/tests/py/test_negative.py index 5df8659d..ac94b1b6 100644 --- a/tests/py/test_negative.py +++ b/tests/py/test_negative.py @@ -790,6 +790,82 @@ def test_streaming_two_frames(): return valid_count == 2 +def test_buffer_mode_garbage_prefix_recovers(): + """Buffer mode: a garbage tail that looks like a truncated frame start is saved + as a partial; the reader must resync inside the internal buffer and keep + delivering the frames that follow (livelock regression test).""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_STANDARD_CONFIG, capacity=2048) + writer.write(msg) + frame = bytes(writer.data()[:writer.size()]) + if len(frame) < 6: + return False + + chunk1 = frame + bytes([0x90, 0xFF]) # looks like a truncated frame start + + reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) + valid_count = 0 + saw_sync = False + + reader.add_data(chunk1) + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + elif result.status == FrameMsgStatus.SYNC_RECOVERY: + saw_sync = True + + for _ in range(3): + reader.add_data(frame) + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + elif result.status == FrameMsgStatus.SYNC_RECOVERY: + saw_sync = True + + return valid_count == 4 and saw_sync + + +def test_buffer_mode_oversized_length_recovers(): + """Buffer mode: a corrupted length field claiming more bytes than the reader's + internal buffer can hold must not wedge the reader permanently + (livelock regression test).""" + msg = _make_test_msg() + writer = BufferWriter(PROFILE_STANDARD_CONFIG, capacity=2048) + writer.write(msg) + frame = bytes(writer.data()[:writer.size()]) + if len(frame) < 6: + return False + + # Bogus header claiming a 255-byte payload — the total (261) exceeds the + # 256-byte reader buffer, so this frame can never complete. + bogus = bytes([0x90, 0x71, 0xFF, BasicTypesMessage.MSG_ID & 0xFF]) + + reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=256) + + reader.add_data(bogus) + while _try_next(reader) is not None: + pass + + valid_count = 0 + rounds = 256 // len(frame) + 3 + for _ in range(rounds): + reader.add_data(frame) + while (result := _try_next(reader)) is not None: + if result.valid: + valid_count += 1 + if valid_count < 1: + return False + + # The reader must keep delivering fresh frames after recovery. + probe_valid = 0 + for _ in range(3): + reader.add_data(frame) + while (result := _try_next(reader)) is not None: + if result.valid: + probe_valid += 1 + return probe_valid >= 1 + + def test_status_waiting_for_start(): """FrameMsgStatus: push_byte returns WAITING_FOR_START when no start byte seen yet.""" reader = AccumulatingReader(PROFILE_STANDARD_CONFIG, get_message_info=get_message_info, buffer_size=1024) @@ -1221,6 +1297,8 @@ def main(): tests = [ ("Buffer mode: CRC failure counters", test_buffer_mode_crc_counters), ("Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap), + ("Buffer mode: garbage prefix partial recovers", test_buffer_mode_garbage_prefix_recovers), + ("Buffer mode: oversized length recovers", test_buffer_mode_oversized_length_recovers), ("Buffer mode: invalid result carries diagnostics", test_buffer_mode_invalid_result_has_diagnostics), ("Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure), ("Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure), diff --git a/tests/py/test_sdk.py b/tests/py/test_sdk.py index 0342bbbf..5abdaab7 100644 --- a/tests/py/test_sdk.py +++ b/tests/py/test_sdk.py @@ -330,6 +330,53 @@ def _throwing(*_): run_test("handler isolation: sibling handler still fires", sibling_fired[0] is True) +def test_base_transport_serializes_concurrent_sends(): + """BaseTransport.send serializes concurrent callers so frame bytes from + different threads never interleave on the wire. + + Regression test for F12: the sync SDK/transport previously wrote directly + to the socket with no lock, so two threads sending concurrently could + interleave bytes mid-frame and corrupt the stream. Mirrors the C# + BaseTransport SemaphoreSlim guarantee. + """ + import threading + import time + from struct_frame_sdk.transport import BaseTransport + + class SlowTransport(BaseTransport): + def __init__(self): + super().__init__() + self.connected = True + self.events = [] + + def connect(self): + pass + + def disconnect(self): + pass + + def _send_impl(self, data): + self.events.append(("start", data[0])) + time.sleep(0.005) # window in which an unlocked send would interleave + self.events.append(("end", data[0])) + return len(data) + + transport = SlowTransport() + threads = [threading.Thread(target=transport.send, args=(bytes([i] * 4),)) + for i in range(8)] + for t in threads: + t.start() + for t in threads: + t.join() + + # Serialized => each 'start' is immediately followed by its own 'end'. + ev = transport.events + serialized = (len(ev) == 16 and all( + ev[i][0] == "start" and ev[i + 1] == ("end", ev[i][1]) + for i in range(0, len(ev), 2))) + run_test("transport: concurrent sends are serialized (no interleave)", serialized) + + # --------------------------------------------------------------------------- # Entry point # --------------------------------------------------------------------------- @@ -349,6 +396,7 @@ def main(): test_codec_registration_and_message_decoding() test_close_callback_clears_buffer_state() test_throwing_handler_does_not_stop_siblings() + test_base_transport_serializes_concurrent_sends() print() print("========================================") diff --git a/tests/rust/src/test_negative.rs b/tests/rust/src/test_negative.rs index 2f4a14d3..ca1c87ee 100644 --- a/tests/rust/src/test_negative.rs +++ b/tests/rust/src/test_negative.rs @@ -952,6 +952,98 @@ fn test_streaming_two_frames() -> bool { valid_count == 2 } + +/// Buffer mode: a garbage tail that looks like a truncated frame start is +/// buffered; the reader must resync past it and keep delivering the frames +/// that follow (livelock regression test). +fn test_buffer_mode_garbage_prefix_recovers() -> bool { + let msg = create_test_message(); + let mut writer = BufferWriter::new(PROFILE_STANDARD_CONFIG, 2048); + writer.write_crc(&msg, 0); + let frame = writer.data().to_vec(); + if frame.len() < 6 { + return false; + } + + let mut chunk1 = frame.clone(); + chunk1.push(0x90); // looks like a truncated frame start + chunk1.push(0xFF); + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 1024); + let mut valid_count = 0; + let mut saw_sync = false; + + reader.add_data(&chunk1); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } else if f.status == FrameMsgStatus::SyncRecovery { + saw_sync = true; + } + } + + for _ in 0..3 { + reader.add_data(&frame); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } else if f.status == FrameMsgStatus::SyncRecovery { + saw_sync = true; + } + } + } + + valid_count == 4 && saw_sync +} + +/// Buffer mode: a corrupted length field claiming more bytes than the reader's +/// capacity must not wedge the reader permanently (livelock regression test). +fn test_buffer_mode_oversized_length_recovers() -> bool { + let msg = create_test_message(); + let mut writer = BufferWriter::new(PROFILE_STANDARD_CONFIG, 2048); + writer.write_crc(&msg, 0); + let frame = writer.data().to_vec(); + if frame.len() < 6 { + return false; + } + + // Bogus header claiming a 255-byte payload — the total (261) exceeds the + // 256-byte reader capacity, so this frame can never complete. + let bogus = [0x90u8, 0x71, 0xFF, (BasicTypesMessage::MSG_ID & 0xFF) as u8]; + + let mut reader = AccumulatingReader::new(PROFILE_STANDARD_CONFIG, 256); + + reader.add_data(&bogus); + while reader.try_next(&get_message_info).is_some() {} + + let mut valid_count = 0; + let rounds = 256 / frame.len() + 3; + for _ in 0..rounds { + reader.add_data(&frame); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + valid_count += 1; + } + } + } + if valid_count < 1 { + return false; + } + + // The reader must keep delivering fresh frames after recovery. + let mut probe_valid = 0; + for _ in 0..3 { + reader.add_data(&frame); + while let Some(f) = reader.try_next(&get_message_info) { + if f.valid { + probe_valid += 1; + } + } + } + probe_valid >= 1 +} + + // ============================================================================ // Package / cross-package corruption scenarios (parity with C/C++/TS/JS/C#) // ============================================================================ @@ -1040,6 +1132,8 @@ fn main() { let tests: &[(&str, fn() -> bool)] = &[ ("Buffer mode: CRC failure counters", test_buffer_mode_crc_counters), ("Buffer mode: Sequence gap counted", test_buffer_mode_seq_gap), + ("Buffer mode: garbage prefix partial recovers", test_buffer_mode_garbage_prefix_recovers), + ("Buffer mode: oversized length recovers", test_buffer_mode_oversized_length_recovers), ("Buffer mode: recovers after CRC failure", test_buffer_mode_recovers_after_crc_failure), ("Buffer reader: skips CRC-failed frame", test_buffer_reader_skips_crc_failure), ("Bulk profile: Corrupted CRC", test_bulk_profile_corrupted_crc), diff --git a/tests/ts/test_negative.ts b/tests/ts/test_negative.ts index 3a3b854b..4115018b 100644 --- a/tests/ts/test_negative.ts +++ b/tests/ts/test_negative.ts @@ -985,6 +985,88 @@ function testStreamingTwoFrames(): boolean { } + +/** + * Buffer mode: a garbage tail that looks like a truncated frame start is saved + * as a partial; the reader must resync inside the internal buffer and keep + * delivering the frames that follow (livelock regression test). + */ +function testBufferModeGarbagePrefixRecovers(): boolean { + const [frames, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 6) return false; + const frame = frames.slice(0, frameSize); + + const chunk1 = new Uint8Array(frameSize + 2); + chunk1.set(frame, 0); + chunk1[frameSize] = 0x90; // looks like a truncated frame start + chunk1[frameSize + 1] = 0xFF; + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 1024); + let validCount = 0; + let sawSync = false; + let r: any; + + reader.addData(chunk1); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + + for (let i = 0; i < 3; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + else if (r.status === FrameMsgStatus.SyncRecovery) sawSync = true; + } + } + + return validCount === 4 && sawSync; +} + +/** + * Buffer mode: a corrupted length field claiming more bytes than the reader's + * internal buffer can hold must not wedge the reader permanently + * (livelock regression test). + */ +function testBufferModeOversizedLengthRecovers(): boolean { + const [frames, sizes] = encodeStandardFrames(1); + const frameSize = sizes[0]; + if (frameSize < 6) return false; + const frame = frames.slice(0, frameSize); + + // Bogus header claiming a 255-byte payload — the total (261) exceeds the + // 256-byte reader buffer, so this frame can never complete. + const bogus = new Uint8Array([0x90, 0x71, 0xFF, BasicTypesMessage._msgid & 0xFF]); + + const reader = new AccumulatingReader(ProfileStandardConfig, getMessageInfo, 256); + let r: any; + + reader.addData(bogus); + while (tryNextCompat(reader) !== null) { /* drain */ } + + let validCount = 0; + const rounds = Math.floor(256 / frameSize) + 3; + for (let i = 0; i < rounds; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) validCount++; + } + } + if (validCount < 1) return false; + + // The reader must keep delivering fresh frames after recovery. + let probeValid = 0; + for (let i = 0; i < 3; i++) { + reader.addData(frame); + while ((r = tryNextCompat(reader)) !== null) { + if (r.valid) probeValid++; + } + } + return probeValid >= 1; +} + + function main(): number { console.log('\n========================================'); console.log('NEGATIVE TESTS - TypeScript Parser'); @@ -994,6 +1076,8 @@ function main(): number { const tests: Array<[string, () => boolean]> = [ ['Buffer mode: CRC failure counters', testBufferModeCrcCounters], ['Buffer mode: Sequence gap counted', testBufferModeSeqGap], + ['Buffer mode: garbage prefix partial recovers', testBufferModeGarbagePrefixRecovers], + ['Buffer mode: oversized length recovers', testBufferModeOversizedLengthRecovers], ['Buffer mode: recovers after CRC failure', testBufferModeRecoversAfterCrcFailure], ['Buffer reader: skips CRC-failed frame', testBufferReaderSkipsCrcFailure], ['Bulk profile: Corrupted CRC', testBulkProfileCorruptedCrc], From efa57241e69a2ed06a22fa6592de9bebc37e21ff Mon Sep 17 00:00:00 2001 From: Rijesh Augustine <7819200+rijesha@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:56:38 -0600 Subject: [PATCH 3/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run_tests.py b/tests/run_tests.py index 67aaf15a..b0321070 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -1800,7 +1800,7 @@ def _record(test_name: str, lang_id: str, success: bool, for runner, test_row in [ ("test_sdk_units", "test_sdk_units"), ("test_sdk_subscribe", "test_sdk_subscribe"), - ("test_sdk_headers_compile", "test_sdk_subscribe"), + ("test_sdk_headers_compile", "test_sdk_headers_compile"), ]: exe = build_dir / f"{runner}{cpp_lang.exe_ext}" if exe.exists(): From d3982dba2a2aa3f43e22aa69499742765ccdb90c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:15:05 +0000 Subject: [PATCH 4/6] Fix C++ compilation ODR errors by using angle-bracket includes for frame_base.hpp --- src/struct_frame/cpp_gen.py | 5 ++++- tests/cpp/test_sdk_subscribe.cpp | 6 +++--- tests/run_tests.py | 9 +++++++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/struct_frame/cpp_gen.py b/src/struct_frame/cpp_gen.py index f14185b9..487feed0 100644 --- a/src/struct_frame/cpp_gen.py +++ b/src/struct_frame/cpp_gen.py @@ -906,7 +906,10 @@ def generate(package, imported_packages=None, equality=False): # Always include frame_base.hpp for FrameMsgInfo and MessageBase # (needed for deserialize(FrameMsgInfo) overload and message base class) - yield '#include "frame_base.hpp"\n' + # Use angle-bracket form so the -I search path is used rather than the + # current file's directory; this allows the SDK's canonical copy to satisfy + # the include when generated headers are compiled alongside the SDK. + yield '#include \n' # Include generated headers for imported packages (enables language server navigation) if imported_packages: diff --git a/tests/cpp/test_sdk_subscribe.cpp b/tests/cpp/test_sdk_subscribe.cpp index 39680945..86a6e9d2 100644 --- a/tests/cpp/test_sdk_subscribe.cpp +++ b/tests/cpp/test_sdk_subscribe.cpp @@ -13,11 +13,11 @@ #include #include -// Generated frame profiles and message types +// Generated message types #include "include/standard_messages.hpp" -#include "../../generated/cpp/frame_profiles.hpp" -// StructFrameSdk (header-only) +// StructFrameSdk (header-only) – also provides the frame profile types +// (ProfileStandardConfig, FrameEncoderWithCrc, BufferParserWithCrc, etc.) #include "../../src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp" using namespace structframe; diff --git a/tests/run_tests.py b/tests/run_tests.py index b0321070..4f86ae75 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -798,9 +798,14 @@ def _compile_language(self, lang: Language) -> bool: else: include_dir = test_dir / "include" if runner in ("test_sdk_subscribe", "test_sdk_headers_compile"): - # These tests need the C++ SDK boilerplate headers + # These tests need the C++ SDK boilerplate headers. + # sdk_dir must come before gen_dir so that angle-bracket + # includes of frame_base.hpp (from generated .structframe.hpp + # files) resolve to the SDK's canonical copy rather than the + # generated copy, avoiding ODR violations when the SDK and + # generated headers are included in the same translation unit. sdk_dir = self.project_root / "src" / "struct_frame" / "boilerplate" / "cpp" - cmd = f'{_cxx()} -std=c++20 {_cxxflags()} -I"{gen_dir}" -I"{include_dir}" -I"{sdk_dir}" -o "{output}" "{source}" {_ldflags()}' + cmd = f'{_cxx()} -std=c++20 {_cxxflags()} -I"{sdk_dir}" -I"{gen_dir}" -I"{include_dir}" -o "{output}" "{source}" {_ldflags()}' else: cmd = f'{_cxx()} -std=c++20 {_cxxflags()} -I"{gen_dir}" -I"{include_dir}" -o "{output}" "{source}" {_ldflags()}' From e7bc00b68d7c79243a7f2206c712d5e1fc48c8e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:42:22 +0000 Subject: [PATCH 5/6] Fix KeyError by adding test_sdk_headers_compile to SDK_APPLICABILITY --- tests/run_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/run_tests.py b/tests/run_tests.py index 4f86ae75..54fe5786 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -1750,6 +1750,7 @@ def run_sdk_tests(self) -> bool: SDK_APPLICABILITY: Dict[str, List[str]] = { "test_streaming": ["c", "rust"], "test_sdk_units": ["cpp"], + "test_sdk_headers_compile": ["cpp"], "test_sdk_subscribe": ["cpp", "csharp", "rust"], "test_sdk": ["py", "ts", "js"], "test_async_sdk": ["py"], From a0c4cb3ea9e10685eee53d9dec8d00fe63d2cbcb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 00:01:25 +0000 Subject: [PATCH 6/6] Fix coverage spec: update Python test_sdk.py run_test count from 31 to 32 --- tests/gen_test_coverage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gen_test_coverage.py b/tests/gen_test_coverage.py index b762cd43..3cae93d0 100644 --- a/tests/gen_test_coverage.py +++ b/tests/gen_test_coverage.py @@ -128,7 +128,7 @@ def discover_negative_scenarios(lang: str) -> set[str]: "display": "Python test_sdk.py run_test assertions", "path": "tests/py/test_sdk.py", "pattern": r"run_test\(", - "expected": 31, + "expected": 32, }, { "display": "TypeScript test_sdk.ts assert assertions",