Don't crash on invalid UTF-8 in headers #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were creating on GET and also making pointless events when testing the queue option. This should be more readable.
Users are able to craft request content which goes to BigQuery by setting request headers. This can cause issues at the point of job serialization (Sidekiq can't serialize the args) or at the point of sending to BQ (the Google Cloud library can't build the JSON), when there's a string present that's not valid UTF-8. We could preemptively rountrip events to JSON and back again before putting them on the queue, but the Google library expects hashes, so we can't reuse that JSON. Instead, focus on the things we know can be malformed and fix those: ua, referer and path. We don't need to worry about query params as Rails will reject these out of hand - see ActionDispatch::Request::Utils.check_param_encoding). We decided to still send these requests, because we send everything, but with � in place of unparseable bytes (which is how String#scrub replaces them).
duncanjbrown
deleted the
733-recover-from-malformed-strings-in-user-agents
branch
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Users are able to craft request content which goes to BigQuery by setting request headers. These can include invalid
UTF-8. This can cause issues at the point of job serialization (Sidekiq can't serialize the args) or at the point of sending to BQ (the Google Cloud library can't build the JSON).We could preemptively roundtrip events to JSON and back again before putting them on the queue, but the Google library expects hashes so we can't reuse that JSON and it's wasteful. Instead, focus on the things we know can be malformed and fix those: ua and referer. We don't need to worry about about query params as Rails will reject those out of hand - see ActionDispatch::Request::Utils.check_param_encoding).
We decided to still send these events, because we send everything, but with � in place of unparseable bytes (which is how String#scrub replaces them).