Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rust: read: handle padded compressed chunks properly #1299

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

james-rms
Copy link
Collaborator

@james-rms james-rms commented Dec 19, 2024

Changelog

  • rust: fixed case where a compressed chunk with padding bytes added by the compressor at the end would break decompression.

Docs

None.

Description

When MCAPs are written, the compressor may insert padding bytes at the end of a compressed block after all useful data bytes are written. This means that when the decompressor is reading messages out, it will have decompressed all message bytes out of a compressed chunk before it reaches the end of the compressed data. Right now, the rust MCAP library will see that there is more unused data in the compressed chunk, try to decompress another message, but get no output from of the decompressor when it tries. Right now this causes the reader to loop infinitely. This PR adds a file to LFS which triggers this condition and fixes it.

BeforeAfter

@james-rms james-rms force-pushed the decompression-freezes branch from 37fb262 to 849d60b Compare December 20, 2024 04:59
@james-rms james-rms changed the title add repro case rust: handle padded compressed chunks properly Dec 20, 2024
@james-rms james-rms changed the title rust: handle padded compressed chunks properly rust: read: handle padded compressed chunks properly Dec 20, 2024
@james-rms james-rms marked this pull request as ready for review December 20, 2024 05:03
Copy link

@gasmith gasmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

rust/src/sans_io/read.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants