Skip to content

fix(minidump): Convert compressed minidump to stream#6151

Open
jjbayer wants to merge 14 commits into
masterfrom
fix/minidump-upload-compressed
Open

fix(minidump): Convert compressed minidump to stream#6151
jjbayer wants to merge 14 commits into
masterfrom
fix/minidump-upload-compressed

Conversation

@jjbayer

@jjbayer jjbayer commented Jun 30, 2026

Copy link
Copy Markdown
Member

When a minidump uploaded to /minidump has internal compression (not Content-Encoding), decompress its stream on the fly so that the streaming variant has feature parity with the non-streaming version.

The initial plan was to leave the stream compressed and just pass it on to the upstream, but

  1. In practice this would only make sense for zstd, because that's the compression the objectstore client currently supports,
  2. We would have to re-think a lot of size checks that currently count the number of bytes going through the stream, for accounting purposes. Concretely we would need objectstore to return the number of decompressed bytes.
  3. We would have to bypass the RequestDecompressionLayer for the upload endpoint. This is certainly possible, by splitting routes into separate Routers.
  4. There is currently no way of telling objectstore to mark an attachment as compressed without running extra an extra compression step. See FS-394.

All these problems are solvable but seem out of scope for the purpose of this PR.

Fixes INGEST-913

@linear-code

linear-code Bot commented Jul 1, 2026

Copy link
Copy Markdown

INGEST-913

@jjbayer jjbayer marked this pull request as ready for review July 1, 2026 12:27
@jjbayer jjbayer requested a review from a team as a code owner July 1, 2026 12:27

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4b2b3c6. Configure here.

Comment thread relay-server/src/endpoints/minidump.rs
Comment thread relay-server/src/endpoints/minidump.rs Outdated
Comment thread relay-server/src/endpoints/minidump.rs
Comment thread CHANGELOG.md Outdated

@tobias-wilfert tobias-wilfert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is changing relay-conventions intentional?

Comment on lines +366 to 373
let stream = match decode_stream(stream).await {
Ok(decoded) => decoded,
Err(_) => {
let _ = item.reject_err(Outcome::Invalid(DiscardReason::InvalidMinidump));
return Err(BadStoreRequest::InvalidMinidump);
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The streaming path for minidump uploads decompresses data but fails to validate it as a valid minidump before uploading, unlike the non-streaming path which does perform validation.
Severity: MEDIUM

Suggested Fix

After decompressing the stream with decode_stream in the streaming code paths (upload_stream_checked and raw_minidump_to_item), add a validation step. This should involve reading the initial bytes of the decompressed stream to check for the MDMP or PMDM magic bytes, similar to validate_minidump. If validation fails, reject the item with an InvalidMinidump reason before uploading to object storage.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: relay-server/src/endpoints/minidump.rs#L366-L373

Potential issue: In the streaming path for minidump uploads, enabled by the
`projects:relay-minidump-uploads` feature, the code decompresses the incoming data
stream but does not validate the result. The decompressed stream is passed directly to
`upload_stream` for storage without checking if it is a valid minidump (e.g., by looking
for `MDMP`/`PMDM` magic bytes). This is inconsistent with the non-streaming path, which
performs this validation. Consequently, compressed garbage data can be successfully
uploaded to object storage, leading to invalid data being persisted and causing
potential downstream processing issues.

Also affects:

  • relay-server/src/endpoints/minidump.rs:542~546

Did we get this right? 👍 / 👎 to inform future reviews.

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.

3 participants