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

fix(server): Finish decoding compressed requests correctly [INGEST-619] #1123

Merged
merged 2 commits into from
Nov 12, 2021

Conversation

jan-auer
Copy link
Member

It is required to finish decoders to flush remaining buffers and validate
checksums at the end of the payload. In this PR, we ensure this by replacing
Decoder::take with Decoder::finish. The take method is now fully internal
and only used in the streaming DecodingPayload. All other users must call
finish instead.

@jan-auer jan-auer requested a review from a team November 12, 2021 10:03
Ok(Async::NotReady) => return Ok(Async::NotReady),
Err(error) => return Err(error),
};
loop {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the largest change. To summarize the diff:

  • If the payload returns None, finish the decoder. This may return None again or another chunk of bytes; both is fine at that point
  • If the decoder doesn't yield a result, continue looping to poll the next chunk

@jan-auer jan-auer merged commit 0778635 into master Nov 12, 2021
@jan-auer jan-auer deleted the fix/finish-decoder branch November 12, 2021 10:20
@jan-auer jan-auer changed the title fix(server): Finish decoding compressed requests correctly fix(server): Finish decoding compressed requests correctly [INGEST-619] Nov 12, 2021
@github-actions
Copy link

Fails
🚫 Please consider adding a changelog entry for the next release.
Instructions and example for changelog

For changes exposed to the Python package, please add an entry to py/CHANGELOG.md. This includes, but is not limited to event normalization, PII scrubbing, and the protocol.

For changes to the Relay server, please add an entry to CHANGELOG.md under the following heading:

  1. Features: For new user-visible functionality.
  2. Bug Fixes: For user-visible bug fixes.
  3. Internal: For features and bug fixes in internal operation, especially processing mode.

To the changelog entry, please add a link to this PR (consider a more descriptive message):

- Finish decoding compressed requests correctly [INGEST-619]. ([#1123](https://github.com/getsentry/relay/pull/1123))

If none of the above apply, you can opt out by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against a49ae8c

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.

2 participants