Skip to content

rust: reject oversized chunk compression strings before allocating - #1823

Merged
clalancette merged 1 commit into
foxglove:mainfrom
official-burak:fix/summary-overallocation
Aug 25, 2026
Merged

rust: reject oversized chunk compression strings before allocating#1823
clalancette merged 1 commit into
foxglove:mainfrom
official-burak:fix/summary-overallocation

Conversation

@official-burak

@official-burak official-burak commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Changelog

  • rust: return an error instead of allocating when a chunk header declares a compression string larger than the chunk record or the configured record length limit
  • rust: clamp summary-reader ReadRequest sizes to the remaining file length when the file size is known

Docs

None.

Description

LinearReader loads a chunk header in two steps: first the fixed prefix, then MIN_CHUNK_HEADER_SIZE + compression_len bytes. compression_len is taken from the file and was passed to consume! / insert() with no check against the chunk record length or record_length_limit. insert(n) resizes the buffer before any bytes are read, so a 191 KiB file that declared a ~1.69 GiB compression string drove peak RSS to about 1.69 GiB and then returned UnexpectedEof.

record_length_limit did not prevent this. The limit is applied to parsed record bodies, and the chunk path skipped that check until after the header (including the compression string) had been requested.

This PR:

  1. Rejects a compression-string length that exceeds the chunk record body (BadChunkLength).
  2. Applies record_length_limit to that header length (ChunkTooLarge).
  3. Caps SummaryReader read requests to the remaining file bytes when file_size is known, so the documented sans-io loop cannot allocate more than the file can contain.
  4. Caps decompressor next_read_size() by compressed_remaining.

Well-formed files are unchanged. Regression tests cover the 1.69 GiB compression-string case both with a mismatched record length and with a matching length plus a 64x file-size record limit.

Fixes #1813

@clalancette clalancette left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is a nice improvement. I've found similar problems in most of the rest of the languages, which I'll fix in a follow-up PR.

@clalancette
clalancette merged commit 68f77c8 into foxglove:main Aug 25, 2026
31 checks passed
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.

Reading a summary allocates gigabytes from a small file

2 participants