Skip to content

Commit deee162

Browse files
Drop SLCAN standard-ID data frames instead of aliasing them as extended
Review finding #8 (Medium). The SLCAN 't' command (11-bit standard-ID data frame) was decoded into an ordinary Frame, indistinguishable from an extended frame with a small ID because Frame carries no IDE flag - in violation of the extended-only Interface contract that the SocketCAN and python-can backends enforce. On a mixed bus a standard frame could parse as a bogus transfer or even trigger a spurious node-ID collision re-roll. Standard-ID 't' lines are now dropped alongside the already-dropped 'r'/'R' remote frames; the reference consumes only CAN_EFF_FLAG frames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CabiCDQ5DNBYq8WDKRzG4Z
1 parent 03f278e commit deee162

2 files changed

Lines changed: 11 additions & 12 deletions

File tree

src/pycyphal2/can/_media_slcan.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import logging
88

9-
from ._interface import CAN_EXT_ID_MASK, CAN_STD_ID_MASK, Frame
9+
from ._interface import CAN_EXT_ID_MASK, Frame
1010
from ._wire import DLC_TO_LENGTH, MTU_CAN_CLASSIC
1111

1212
_logger = logging.getLogger(__name__)
@@ -126,12 +126,12 @@ def _parse_line(line: bytes) -> Frame | None:
126126
command = line[:1]
127127
if command in (b"T", b"x"):
128128
return _parse_data_frame(line, id_length=8, max_payload_length=MTU_CAN_CLASSIC)
129-
if command == b"t":
130-
return _parse_data_frame(line, id_length=3, max_payload_length=MTU_CAN_CLASSIC)
131129
if command == b"D":
132130
return _parse_data_frame(line, id_length=8, max_payload_length=64)
133-
if command in (b"r", b"R"):
134-
_logger.debug("SLCAN drop unsupported frame type cmd=%r", command)
131+
if command in (b"t", b"r", b"R"):
132+
# Standard-ID (11-bit) frames: the Interface contract is extended-only and Frame carries no IDE
133+
# discriminator, so forwarding a 't' data frame would alias an extended frame with a small ID.
134+
_logger.debug("SLCAN drop standard-id frame cmd=%r", command)
135135
return None
136136
_logger.debug("SLCAN drop unknown line=%r", line)
137137
return None
@@ -155,9 +155,6 @@ def _parse_data_frame(line: bytes, *, id_length: int, max_payload_length: int) -
155155
if len(line) < expected:
156156
_logger.debug("SLCAN drop data dlc mismatch len=%d expected=%d", len(line), expected)
157157
return None
158-
if id_length == 3 and identifier > CAN_STD_ID_MASK:
159-
_logger.debug("SLCAN drop invalid standard id=%x", identifier)
160-
return None
161158
data = _parse_hex_bytes(line[header_length:expected])
162159
if data is None:
163160
_logger.debug("SLCAN drop malformed data id=%08x", identifier)

tests/can/test_media_slcan.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ def test_parse_classic_extended_frames() -> None:
5353

5454
assert parser.feed(b"T000001232ABCD\r") == [Frame(id=0x123, data=b"\xab\xcd")]
5555
assert parser.feed(b"T000001232abCd\r") == [Frame(id=0x123, data=b"\xab\xcd")]
56-
assert parser.feed(b"t1231AA\r") == [Frame(id=0x123, data=b"\xaa")]
57-
assert parser.feed(b"t7FF1AA\r") == [Frame(id=0x7FF, data=b"\xaa")]
58-
assert parser.feed(b"t7FF0\r") == [Frame(id=0x7FF, data=b"")]
56+
# Standard-ID 't' data frames are dropped: the Interface contract is extended-only, and Frame has
57+
# no IDE discriminator, so forwarding them would alias extended frames with small IDs.
58+
assert parser.feed(b"t1231AA\r") == []
59+
assert parser.feed(b"t7FF1AA\r") == []
60+
assert parser.feed(b"t7FF0\r") == []
5961
assert parser.feed(b"T000001") == []
6062
assert parser.feed(b"230\r") == [Frame(id=0x123, data=b"")]
6163
assert parser.feed(b"x1BADC0DE201AB\r") == [Frame(id=0x1BADC0DE, data=b"\x01\xab")]
@@ -68,7 +70,7 @@ def test_parse_ignores_optional_frame_suffix() -> None:
6870
assert parser.feed(b"T000001232ABCDL\r") == [Frame(id=0x123, data=b"\xab\xcd")]
6971
assert parser.feed(b"T000001232ABCD1234L\r") == [Frame(id=0x123, data=b"\xab\xcd")]
7072
assert parser.feed(b"T000001232ABCDzzzz\r") == [Frame(id=0x123, data=b"\xab\xcd")]
71-
assert parser.feed(b"t1231AAL\r") == [Frame(id=0x123, data=b"\xaa")]
73+
assert parser.feed(b"t1231AAL\r") == [] # Standard-ID frames are dropped regardless of suffix.
7274
assert parser.feed(b"T000001232ABCD1234\x03\r") == [Frame(id=0x123, data=b"\xab\xcd")]
7375
assert parser.feed(b"T10AE6EFF8000000FF000000A07071\r") == [
7476
Frame(id=0x10AE6EFF, data=b"\x00\x00\x00\xff\x00\x00\x00\xa0"),

0 commit comments

Comments
 (0)