Bound CI-V reassembly buffer to prevent OOM on a terminator-less stream

[patrickrb]

Summary

The shared CI-V (Icom / Xiegu) stream reassembler, CivFrameSplitter, buffered
the trailing bytes after the last frame terminator (0xFD) and carried them into
the next read via each rig's dataBuffer. Nothing bounded that buffer: a CI-V
stream that never delivers a terminator — a rig connected at the wrong baud rate,
line noise on a serial link, or a misbehaving network peer streaming garbage —
made dataBuffer grow by every received byte, without limit, until the app
eventually ran out of memory.

This affects every CI-V rig, since IcomRig, XieGuRig, and XieGu6100Rig all
feed their bytes through CivFrameSplitter.split(...) and store result.remainder
back into dataBuffer.

Root cause

CivFrameSplitter.split returned everything after the last 0xFD as the
remainder with no upper bound. Under well-formed traffic the remainder is a
command still in progress and stays tiny (a CI-V command this app exchanges is a
few to a few dozen bytes), so the growth only manifests when the terminator never
arrives — i.e. exactly the malformed-stream case a robust reassembler must
tolerate without falling over.

Fix

Bound the carried-over remainder in the one shared place it is produced
(CivFrameSplitter.boundRemainder). When the remainder exceeds a hard cap
(MAX_BUFFERED_BYTES = 1024, orders of magnitude above any real CI-V command):

• Resynchronise to the most recent FE FE preamble — the earliest point a
  valid command could still begin — and drop the unparseable bytes before it.
  Those bytes can never complete a frame (a valid one would have ended in 0xFD
  and already been split off), so this is lossless for well-formed traffic and
  only ever trims garbage.
• If no preamble is present, the buffer is pure noise: keep at most a single
  trailing 0xFE, in case a preamble is split across the read boundary.
• A final trailing-window clamp guarantees boundedness even in the pathological
  case of a lone preamble followed by a huge terminator-less run.

Behaviour is unchanged for all well-formed and normally-fragmented traffic (the
remainder never approaches the cap there), so decode/CAT interoperability is
untouched.

Testing

./gradlew :app:testDebugUnitTest --tests com.k1af.ft8af.rigs.CivFrameSplitterTest

Added pure-JVM cases to the existing CivFrameSplitterTest:

• a terminator-less stream fed across 100 callbacks keeps the remainder <= 1024
  bytes (fails before the fix — the remainder grows unbounded);
• an overflow resynchronises to the last FE FE preamble and the resynced
  command still completes on the next chunk;
• overflow with no preamble drops the noise but keeps a trailing 0xFE;
• overflow of pure noise drops everything;
• a lone preamble followed by a huge terminator-less tail is hard-capped to 1024;
• a large-but-under-cap partial command is carried through unchanged.

All existing reassembly tests continue to pass (single/multi-frame,
split-across-callbacks, no stray bytes).

Risk

Low. The change is confined to the shared pure-logic helper and only alters
behaviour once the buffer exceeds a cap far above any legitimate CI-V frame;
normal and normally-fragmented traffic is byte-for-byte identical. No native, DSP,
encoder/decoder, or protocol changes.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.81%. Comparing base (7f38704) to head (5cdd3d9).
⚠️ Report is 12 commits behind head on dev.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev     #649      +/-   ##
============================================
+ Coverage     36.73%   36.81%   +0.08%     
- Complexity      197      207      +10     
============================================
  Files           216      218       +2     
  Lines         26888    27012     +124     
  Branches       3287     3315      +28     
============================================
+ Hits           9877     9945      +68     
- Misses        16785    16832      +47     
- Partials        226      235       +9     

Flag	Coverage Δ
android	15.35% <ø> (+0.31%)	⬆️
native	9.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR hardens the CI-V stream reassembly logic (CivFrameSplitter) by bounding the carried-over remainder buffer so a terminator-less stream can’t grow memory usage unbounded and eventually OOM the app.

Changes:

• Add a hard cap (MAX_BUFFERED_BYTES = 1024) to the carried-over remainder and resynchronization logic to the most recent FE FE preamble on overflow.
• Add unit tests covering terminator-less streams, overflow resync behavior, and edge cases (no preamble, trailing 0xFE, lone preamble with huge tail).

Reviewed changes

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

File	Description
ft8af/app/src/main/java/com/k1af/ft8af/rigs/CivFrameSplitter.java	Bounds oversized reassembly remainder and resyncs to the most recent CI‑V preamble to prevent unbounded memory growth.
ft8af/app/src/test/java/com/k1af/ft8af/rigs/CivFrameSplitterTest.java	Adds coverage for malformed/terminator-less streams and overflow resynchronization edge cases.

---

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