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

Handle incorrectly UTF-8 encoded query and cookie url #3815

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

unoduetre
Copy link
Contributor

Use a different exception when this situation is detected:

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

What

Stop sentry alerting on the “invalid byte sequence in UTF-8 errors”.

As discussed with @hannako, we will use a a solution based on an external gem called rack-utf8_sanitizer instead of developing our own gem. This solution will then be used in all our apps by copy-pasting the configuration of this gem.

Why

When security scanning of our site takes place we are inundated with these errors in our slack channel. These errors are not serving any purpose since there is no issue to be solved. Therefore we should remove them to keep all our alerts actionable.

We often get exceptions in Sentry coming from a security checking bot that attempts some attacks by using strings with special characters. All of those "attacks" fail, but they produce exceptions which we would like to ignore, so they do not clutter Sentry. On the other hand we don’t want to ignore exceptions of the same class coming from other sources.

Trello card

@unoduetre unoduetre force-pushed the 2938-silently-handle-invalid-byte-sequence-in-utf-8-errors-l branch from 37135b7 to dae6678 Compare October 25, 2024 12:51
Use a different exception when this situation is detected:
* Sanitiser::Strategy::SanitisingError
This error is not sent to Sentry.
See: alphagov/govuk_app_config#402
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