Skip to content

Reject truncated Zstandard event logs - #274

Open
gerashegalov wants to merge 1 commit into
NVIDIA:devfrom
gerashegalov:fix/zstd-truncated-frames
Open

gerashegalov wants to merge 1 commit into
NVIDIA:devfrom
gerashegalov:fix/zstd-truncated-frames

Conversation

@gerashegalov

Copy link
Copy Markdown
Collaborator

Summary

  • reject Zstandard event logs when physical EOF arrives before the current frame completes
  • preserve bounded chunk emission and support for valid concatenated Zstandard frames
  • classify truncated frames as retryable event-log read failures so application usage is returned as incomplete
  • add reader regressions for valid concatenated, truncated single, and truncated concatenated frames plus public API coverage

Follow-up to #272.

Testing

  • python -m py_compile yarn-resource-cost/*.py
  • python -m unittest discover -s yarn-resource-cost -p 'test*.py' on Python 3.10, 3.11, and 3.12 with zstandard==0.23.0 (64 tests each)
  • python -m unittest discover -s yarn-resource-cost -p 'test*.py' with zstandard==0.25.0 (64 tests)
  • python yarn-resource-cost/package_yarn_job_cost.py --output-dir /tmp/csb-zstd-truncation-dist
  • git diff --check

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

The PR is not safe to merge until Zstandard decompression remains memory-bounded for highly compressible event logs.

Findings

  1. P1 Security Unbounded decompression allocation ▶

Summary

The PR changes Zstandard event-log reading to detect incomplete frames while retaining concatenated-frame support, and adds reader and public-API regression tests.

  • Truncated frames now produce retryable incomplete results.
  • The new decompression path loses the previous bound on in-memory output.

Reviews (1) · Last reviewed commit: "Reject truncated Zstandard event logs"

raise EventLogReadError(
"Truncated Zstandard frame", retryable=True
)
output = frame.decompress(compressed)

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.

P1 security Unbounded decompression allocation A highly compressible .zstd event-log block can expand far beyond READ_SIZE in this single decompress() call. Slicing the result afterward limits the chunks yielded, but the full result has already been allocated, so a small supplied event log can exhaust the reader’s memory. The previous reader limited decompressed output per read; this path needs an output-side bound. How this was verified: Supplied .zstd event-log bytes reach this call, which materializes the expanded output before the chunking loop runs.

@gerashegalov gerashegalov self-assigned this Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant