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

Don't submit non-filterable bodies #71

Merged
merged 1 commit into from
Aug 27, 2020
Merged

Don't submit non-filterable bodies #71

merged 1 commit into from
Aug 27, 2020

Conversation

JoelQ
Copy link
Contributor

@JoelQ JoelQ commented Aug 27, 2020

🧰 What's being changed?

When given allow_only or reject parameters, we want to make sure
that the request and response bodies are properly filtered. In order to
do that, we need to be able to parse them. Currently we only support
parsing JSON and form-encoded bodies. If bodies come in as other content
types and a filter is configured, we should not submit the request to
the Readme API because it might contain sensitive data.

If no filter is set, it is safe to just pass-through any bodies to the
Readme API regardless of content type.

Testing this behavior was rather annoying as I had to create new
instances of both app and middleware for each test in order to get
custom response bodies and middleware configurations desired. To make
this nicer I ended up creating a bunch of testing helpers. I also
created two test app classes based on content returned: JsonApp and
TextApp.

The checking we do around content-types has to happen for both the
request and the response so I pulled it out into a mixin that can be
included into both. Also, as part of this I created an HttpResponse
class that decorates Rack::Request and includes the mixin.

🧪 Testing

Create a simple rack app that returns text/plain content and has a reject or allow-only parameter set. When you make a request to it, you should see an error in the logs:

Monosnap 2020-08-27 10-49-18

Try various combinations of reject_params, allow_only, text response bodies, and text request bodies.

When given `allow_only` or `reject` parameters, we want to make sure
that the request and response bodies are properly filtered. In order to
do that, we need to be able to parse them. Currently we only support
parsing JSON and form-encoded bodies. If bodies come in as other content
types _and_ a filter is configured, we should not submit the request to
the Readme API because it might contain sensitive data.

If no filter is set, it is safe to just pass-through any bodies to the
Readme API regardless of content type.

Testing this behavior was rather annoying as I had to create new
instances of both app and middleware for each test in order to get
custom response bodies and middleware configurations desired. To make
this nicer I ended up creating a bunch of testing helpers. I also
created two test app classes based on content returned: `JsonApp` and
`TextApp`.

The checking we do around content-types has to happen for both the
request and the response so I pulled it out into a mixin that can be
included into both. Also, as part of this I created an `HttpResponse`
class that decorates `Rack::Request` and includes the mixin.
@JoelQ JoelQ force-pushed the jq-no-pass-through branch from 2831032 to 935062e Compare August 27, 2020 15:45
@JoelQ JoelQ merged commit 935062e into master Aug 27, 2020
@JoelQ JoelQ deleted the jq-no-pass-through branch August 27, 2020 15:53
djmango pushed a commit to djmango/metrics-sdks that referenced this pull request Sep 19, 2023
readmeio#71)

Bumps [conventional-changelog-cli](https://github.com/conventional-changelog/conventional-changelog) from 2.0.31 to 2.0.34.
- [Release notes](https://github.com/conventional-changelog/conventional-changelog/releases)
- [Commits](https://github.com/conventional-changelog/conventional-changelog/compare/[email protected]@2.0.34)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@erunion erunion added ruby Issues related to our Ruby SDK and removed area:ruby labels Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ruby Issues related to our Ruby SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants