fix packet scan

[joe-saronic]

Running proptests on another branch, I found this pathological condition where the parser fails to find correct messages if garbage contains the expected init sequence, even if nothing else matches up.

Two unit tests added to catch regression.

Did a throughput test on some noisy data: this actually makes the parser faster since less re-scanning needs to happen. No real timing differences on non-noisy data in mega-sbf file.

┌──────────────────────────────────┬────────┬────────────┬───────────────────┐
│                                  │ ns/msg │ throughput │       found       │
├──────────────────────────────────┼────────┼────────────┼───────────────────┤
│ Old (drain-1, rescan from front) │ 192.8  │ 1200 MB/s  │ 199994 (6 misses) │
├──────────────────────────────────┼────────┼────────────┼───────────────────┤
│ Hardened (scan forward)          │ 172.5  │ 1341 MB/s  │ 200000            │
└──────────────────────────────────┴────────┴────────────┴───────────────────┘

Unit tests on main:

$ cargo test --all-features --all-targets 
   Compiling libsbf v0.17.0 (/home/joe/repos/libsbf-rs)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 4.10s
     Running unittests src/lib.rs (target/debug/deps/libsbf-0c4ff8fe653692c3)

running 20 tests
test parser::tests::test_bad_crc_false_sync_before_valid_message ... ok
test parser::tests::test_parse_datagram_bad_crc ... ok
test parser::tests::test_oversized_false_sync_before_valid_message ... FAILED
test parser::tests::test_parse_datagram_exceeds_max_udp ... ok
test parser::tests::test_parse_datagram_incomplete ... ok
test parser::tests::test_parse_datagram_no_sync ... ok
test parser::tests::test_parse_datagram_unsupported_block_bad_crc_errors ... ok
test parser::tests::test_parse_datagram_unsupported_block_returns_ok ... ok
test parser::tests::test_parse_datagram_valid ... ok
test parser::tests::test_partial_message_completes_on_second_half ... ok
test parser::tests::test_partial_message_with_noise_and_false_sync ... FAILED
test parser::tests::test_receiver_setup_parsing ... ok
test mega_test::tests::test_mega_file_roundtrip ... ok
test mega_test::tests::test_mega_file_message_integrity ... ok
test mega_test::tests::test_mega_file_all_message_types ... ok
test reader::tests::sbf_correct_parse ... ok
test parser::tests::test_receiver_setup_with_noise ... ok
test parser::tests::test_valid_message_with_noise ... ok
test parser::tests::test_multiple_messages_multiple_types ... ok
test reader::tests::test_random_data_consumption ... ok

failures:

---- parser::tests::test_oversized_false_sync_before_valid_message stdout ----

thread 'parser::tests::test_oversized_false_sync_before_valid_message' (2717935) panicked at src/parser.rs:620:22:
expected QualityInd after the false sync, got None
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- parser::tests::test_partial_message_with_noise_and_false_sync stdout ----

thread 'parser::tests::test_partial_message_with_noise_and_false_sync' (2717944) panicked at src/parser.rs:698:22:
expected QualityInd once the second half arrives, got None


failures:
    parser::tests::test_oversized_false_sync_before_valid_message
    parser::tests::test_partial_message_with_noise_and_false_sync

test result: FAILED. 18 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5.98s

error: test failed, to rerun pass `--lib`

[Copilot]

Pull request overview

This PR hardens the streaming SBF parser against pathological “false sync” cases where noise contains the $@ sync bytes and a plausible length, which previously could prevent the parser from ever reaching the next real message. The new approach scans forward for the next complete and CRC-valid message (while retaining any plausible partial candidate), improving robustness and reducing unnecessary rescanning on noisy inputs.

Changes:

• Reworked parse_message to scan forward for the next valid complete message and return an appropriate drain amount, avoiding stalls on false syncs with oversized/incomplete bodies.
• Refactored payload decoding into decode_body and simplified SbfParser::consume to a single pass per call.
• Added unit tests to cover false-sync and partial-message buffering scenarios; updated proptest regression corpus.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File	Description
src/parser.rs	Implements forward-scanning resync logic for streaming parsing and adds regression/unit tests to validate behavior under noise/false sync conditions.
proptest-regressions/parser.txt	Adds a new proptest regression seed/case to preserve the discovered pathological input.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[therishidesai]

I ran the proptest-regression from this PR against the current parser implementation and it doesn't fail.

Do you mean if the noise contains the sync bits it drops a message? We do clean up the noise data here to avoid having impossible message lengths in the proptest.

[therishidesai]

These two unit tests create 2 contrived scenarios that are also impossible to generate from the proptest:

• The proptest has a sanitize noise function which sets the length of a fake sync to 1 so it would have been impossible to ever see these two scenarios which is why I probably wasn't able to repro it.
• The one test you have that is worth digging more into is the one with a very large length. It is technically legal for the length field in SBF to be an arbitrary u16 so we should scan the whole input for that value. The old parser will correctly keep returning IncompleteData until it scans the full 1000 bytes and then it will unpack that data and return it as an InvalidCRC if the message isn't a valid SBF type or if the message is valid then it would return Unsupported. If an InvalidCRC is received we still have the old buffer and will start scanning for a new fake sync so no message is dropped. This unit test is not actually testing this though. Instead, this unit test does the following:
  • Passes a buffer with a fake-sync + large-header + sync + valid-message
  • The sync + valid-message size is shorter than the 1000 byte length in the fake-sync
  • The parser will get to the end of the buffer and return None since there is no more data
  • This scenario wouldn't happen in practice with a streaming parser since no more data coming would mean the connection to the Septentrio some how ended (e.g network failure) and that would be an error further up in the buffered reader.

I don't think this parser re-write is necessary on the basis of just correctness.

[joe-saronic]

Sounds good. I'll close this PR for now. Will investigate the longer buffer size behavior when I have the time.