diff --git a/python/mcap/mcap/__init__.py b/python/mcap/mcap/__init__.py index a955fdae1..bc86c944f 100644 --- a/python/mcap/mcap/__init__.py +++ b/python/mcap/mcap/__init__.py @@ -1 +1 @@ -__version__ = "1.2.1" +__version__ = "1.2.2" diff --git a/python/mcap/mcap/stream_reader.py b/python/mcap/mcap/stream_reader.py index a51920243..c85eecddc 100644 --- a/python/mcap/mcap/stream_reader.py +++ b/python/mcap/mcap/stream_reader.py @@ -217,7 +217,7 @@ def _read_record(self, opcode: int, length: int) -> Optional[McapRecord]: return SummaryOffset.read(self._stream) # Skip unknown record types - self._stream.read(length - 9) + self._stream.read(length) __all__ = ["StreamReader"] diff --git a/python/mcap/tests/test_reader.py b/python/mcap/tests/test_reader.py index 70e0fc701..0ddee0f8e 100644 --- a/python/mcap/tests/test_reader.py +++ b/python/mcap/tests/test_reader.py @@ -1,4 +1,5 @@ """tests for the McapReader implementations.""" + # cspell:words getbuffer import json import os @@ -24,6 +25,14 @@ ) +class StrictBytesIO(BytesIO): + """A subclass of BytesIO that throws for negative or unspecified-length reads.""" + + def read(self, size: Union[int, None] = -1): + assert size is not None and size > 0 + return super().read(size) + + @pytest.fixture def pipe(): r, w = os.pipe() @@ -263,21 +272,21 @@ def test_detect_invalid_initial_magic(tmpdir: Path): def test_record_size_limit(): # create a simple small MCAP - write_stream = BytesIO() + write_stream = StrictBytesIO() writer = Writer(write_stream) writer.start("profile", "library") writer.finish() # default stream reader can read it stream_reader = StreamReader( - BytesIO(write_stream.getbuffer()), record_size_limit=100 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=100 ) records = [r for r in stream_reader.records] assert len(records) == 10 # can cause "large" records to raise an error by setting a low limit stream_reader = StreamReader( - BytesIO(write_stream.getbuffer()), record_size_limit=10 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=10 ) with pytest.raises( RecordLengthLimitExceeded, @@ -287,7 +296,7 @@ def test_record_size_limit(): # default seeking reader can read it seeking_reader = SeekingReader( - BytesIO(write_stream.getbuffer()), record_size_limit=100 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=100 ) seeking_reader.get_header() seeking_reader.get_summary() @@ -295,7 +304,7 @@ def test_record_size_limit(): # can cause "large" records to raise an error by setting a low limit seeking_reader = SeekingReader( - BytesIO(write_stream.getbuffer()), record_size_limit=10 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=10 ) with pytest.raises( RecordLengthLimitExceeded, @@ -311,16 +320,29 @@ def test_record_size_limit(): # default non-seeking reader can read it non_seeking_reader = NonSeekingReader( - BytesIO(write_stream.getbuffer()), record_size_limit=100 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=100 ) non_seeking_reader.get_header() # can cause "large" records to raise an error by setting a low limit non_seeking_reader = NonSeekingReader( - BytesIO(write_stream.getbuffer()), record_size_limit=10 + StrictBytesIO(write_stream.getbuffer()), record_size_limit=10 ) with pytest.raises( RecordLengthLimitExceeded, match="HEADER record has length 22 that exceeds limit 10", ): non_seeking_reader.get_header() + + +def test_custom_record(): + write_stream = StrictBytesIO() + writer = Writer(write_stream) + writer.start("profile", "library") + write_stream.write(b"\x80\x00\x00\x00\x00\x00\x00\x00\x00") + writer.finish() + stream_reader = StreamReader( + StrictBytesIO(write_stream.getbuffer()), record_size_limit=100 + ) + records = [r for r in stream_reader.records] + assert len(records) == 10