From 2bf3f1b9ddd288f5aebaad1b0b35977ac04ea5d3 Mon Sep 17 00:00:00 2001 From: Matthew Wolter Date: Sat, 11 Apr 2026 19:24:26 -0700 Subject: [PATCH] fix: BATTERY handler drops frames shorter than 3 bytes (level field guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A BATTERY frame with len(data) < 3 caused dbuf.read(2) to return short bytes; int.from_bytes(b"", ...) silently yielded 0, propagating a bogus level=0 to HA sensors. Same silent-zero class as N07 (storage fields). Option B: early-return with debug log, matching the NEW-C pattern for STATUS_RESPONSE. No BATTERY event is dispatched for malformed frames. Not in the original forensics report — discovered during G1 N07 work and logged in issues_log.md. Resolved here because no later branch touches this handler. Files changed: - src/meshcore/reader.py: add `if len(data) < 3: return` guard before the level read in the BATTERY branch - tests/unit/test_reader.py: add test_g1_battery_too_short_for_level — sends a 1-byte frame (type only), asserts no BATTERY event dispatched and debug log emitted --- src/meshcore/reader.py | 16 ++++++++++++---- tests/unit/test_reader.py | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/meshcore/reader.py b/src/meshcore/reader.py index ead8b0a..518eb5c 100644 --- a/src/meshcore/reader.py +++ b/src/meshcore/reader.py @@ -341,12 +341,20 @@ class MessageReader: await self.dispatcher.dispatch(Event(EventType.CONTACT_URI, result)) elif packet_type_value == PacketType.BATTERY.value: + # Full RESP_CODE_BATT_AND_STORAGE: 1 type + 2 level + 4 used_kb + 4 total_kb = 11 bytes. + # Minimum viable frame is 3 bytes (type + level). Shorter frames are + # malformed — dbuf.read(2) would return short bytes and + # int.from_bytes(b"", ...) silently yields 0 (same class as N07). + if len(data) < 3: + logger.debug( + "BATTERY frame too short for level field " + f"({len(data)} bytes < 3), skipping" + ) + return battery_level = int.from_bytes(dbuf.read(2), byteorder="little") result = {"level": battery_level} - # Full RESP_CODE_BATT_AND_STORAGE frame is 11 bytes: - # 1 type + 2 level + 4 used_kb + 4 total_kb. The previous - # `len(data) > 3` guard let 4-10 byte truncated frames through, - # producing silent zero values for used_kb/total_kb because + # The previous `len(data) > 3` guard let 4-10 byte truncated frames + # through, producing silent zero values for used_kb/total_kb because # io.BytesIO.read() returns short data without raising. if len(data) >= 11: # has storage info as well result["used_kb"] = int.from_bytes(dbuf.read(4), byteorder="little") diff --git a/tests/unit/test_reader.py b/tests/unit/test_reader.py index aa72b03..67fa9e6 100644 --- a/tests/unit/test_reader.py +++ b/tests/unit/test_reader.py @@ -162,6 +162,33 @@ async def test_g1_battery_short_frame_omits_storage_fields(): ) +@pytest.mark.asyncio +async def test_g1_battery_too_short_for_level(caplog): + """BATTERY frame shorter than 3 bytes must be dropped entirely (Option B). + + A 1-byte frame (just the packet-type byte 0x0c, no level bytes) would cause + dbuf.read(2) to return b"" and int.from_bytes(b"", ...) to silently yield 0. + The fix adds an early return with a debug log, matching the NEW-C pattern. + """ + dispatcher = _CapturingDispatcher() + reader = MessageReader(dispatcher) + + # 1-byte BATTERY frame: only the type byte, no level payload. + too_short = bytearray.fromhex("0c") + + with caplog.at_level(logging.DEBUG, logger="meshcore"): + await reader.handle_rx(too_short) + + battery_events = [e for e in dispatcher.events if e.type == EventType.BATTERY] + assert len(battery_events) == 0, ( + "BATTERY frame shorter than 3 bytes must not dispatch an event" + ) + debug_records = [ + r for r in caplog.records if "BATTERY frame too short" in r.message + ] + assert debug_records, "Expected a debug log about the short BATTERY frame" + + @pytest.mark.asyncio async def test_g1_status_response_short_frame_skipped(caplog): """G1/NEW-C: short STATUS_RESPONSE push frame must be skipped, not parsed with bogus zeros."""