Add new 'redacted keys' config option to replace 'filters' #623
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
This replaces 'filters' with slightly different semantics
The new redacted keys option can do everything filters can and more
imjoehaines
commented
Feb 5, 2021
| @@ -5,24 +5,24 @@ Bugsnag\Handler should increase the memory limit by the configured amount when a | |||
| $client = require __DIR__ . '/_prelude.php'; | |||
| $client->setMemoryLimitIncrease(1024 * 1024 * 10); | |||
|
|
|||
| ini_set('memory_limit', '2M'); | |||
| Bugsnag\Handler::register($client); | |||
There was a problem hiding this comment.
This test could run OOM before installing the handler, so I've moved this to before we reduce the memory limit and bumped the limit a bit
imjoehaines
commented
Feb 5, 2021
| @@ -29,6 +29,7 @@ jobs: | |||
| with: | |||
| php-version: ${{ matrix.php-version }} | |||
| coverage: none | |||
| extensions: intl, mbstring | |||
There was a problem hiding this comment.
PHPUnit requires mbstring so we can't run the tests without it
This was referenced Feb 5, 2021
kattrali
approved these changes
Feb 8, 2021
Merged
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.
Goal
This PR adds a 'redacted keys' option, to replace the existing 'filters' option. Filters is problematic because it works with a case-insensitive string contains, which can easily cause false positives
Redacted keys has different semantics but can still express everything that filters could (and more!). Redacted keys can contain both strings and regexes, where string comparisons are case-insensitive and exact — i.e.
amatchesaandAbut notab. Regexes can be used to emulate filters, e.g./a/iwill matcha,AandabRedacted keys also attempts to match multi-byte strings by normalising strings and using multi-byte comparisons. This relies on the intl and mbstring extensions, so we fallback to
strcasecmpif they are missing (we also trystrcasecmpfirst as it's quicker than normalising + MB comparisons)Changeset
redactedKeyswith getters & setters in Client & Configurationfiltersoption is now marked as deprecatedReport::shouldFilteralso usesredactedKeysto check if keys should be filtered/redactedUtils::stringCaseEqualsmethod has been added. This does a unicode-aware, case-insensitive string match