diff --git a/.github/workflows/test-package.yml b/.github/workflows/test-package.yml index e29229fa..37d8537a 100644 --- a/.github/workflows/test-package.yml +++ b/.github/workflows/test-package.yml @@ -29,6 +29,7 @@ jobs: with: php-version: ${{ matrix.php-version }} coverage: none + extensions: intl, mbstring - run: composer validate diff --git a/CHANGELOG.md b/CHANGELOG.md index 76447871..8af31da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,13 @@ Changelog * Add a "discard classes" configuration option that allows events to be discarded based on the exception class name or PHP error name [#622](https://github.com/bugsnag/bugsnag-php/pull/622) +* Add a "redacted keys" configuration option. This is similar to `filters` but allows both strings and regexes. String matching is exact but case-insensitive. Regex matching allows for partial and wildcard matching. + [#623](https://github.com/bugsnag/bugsnag-php/pull/623) + +### Deprecations + +* The `filters` configuration option is now deprecated as `redactedKeys` can express everything that filters could and more. + ## 3.25.0 (2020-11-25) ### Enhancements diff --git a/src/Client.php b/src/Client.php index 7800230e..4a2cb825 100644 --- a/src/Client.php +++ b/src/Client.php @@ -532,6 +532,8 @@ public function shouldNotify() * * Eg. ['password', 'credit_card']. * + * @deprecated Use redactedKeys instead + * * @param string[] $filters an array of metaData filters * * @return $this @@ -546,7 +548,9 @@ public function setFilters(array $filters) /** * Get the array of metaData filters. * - * @var string + * @deprecated Use redactedKeys instead + * + * @var string[] */ public function getFilters() { @@ -987,4 +991,28 @@ public function getDiscardClasses() { return $this->config->getDiscardClasses(); } + + /** + * Set the array of metadata keys that should be redacted. + * + * @param string[] $redactedKeys + * + * @return $this + */ + public function setRedactedKeys(array $redactedKeys) + { + $this->config->setRedactedKeys($redactedKeys); + + return $this; + } + + /** + * Get the array of metadata keys that should be redacted. + * + * @var string[] + */ + public function getRedactedKeys() + { + return $this->config->getRedactedKeys(); + } } diff --git a/src/Configuration.php b/src/Configuration.php index 8fdeccc3..9dd8bc7d 100644 --- a/src/Configuration.php +++ b/src/Configuration.php @@ -43,6 +43,8 @@ class Configuration /** * The strings to filter out from metaData. * + * @deprecated Use redactedKeys instead + * * @var string[] */ protected $filters = [ @@ -170,6 +172,13 @@ class Configuration */ protected $discardClasses = []; + /** + * An array of metadata keys that should be redacted. + * + * @var string[] + */ + protected $redactedKeys = []; + /** * Create a new config instance. * @@ -261,6 +270,8 @@ public function shouldNotify() * * Eg. ['password', 'credit_card']. * + * @deprecated Use redactedKeys instead + * * @param string[] $filters an array of metaData filters * * @return $this @@ -275,7 +286,9 @@ public function setFilters(array $filters) /** * Get the array of metaData filters. * - * @var string + * @deprecated Use redactedKeys instead + * + * @var string[] */ public function getFilters() { @@ -836,4 +849,28 @@ public function getDiscardClasses() { return $this->discardClasses; } + + /** + * Set the array of metadata keys that should be redacted. + * + * @param string[] $redactedKeys + * + * @return $this + */ + public function setRedactedKeys(array $redactedKeys) + { + $this->redactedKeys = $redactedKeys; + + return $this; + } + + /** + * Get the array of metadata keys that should be redacted. + * + * @var string[] + */ + public function getRedactedKeys() + { + return $this->redactedKeys; + } } diff --git a/src/Report.php b/src/Report.php index f3d470cb..8a1dfd9b 100644 --- a/src/Report.php +++ b/src/Report.php @@ -785,11 +785,21 @@ protected function cleanupObj($obj, $isMetaData) */ protected function shouldFilter($key, $isMetaData) { - if ($isMetaData) { - foreach ($this->config->getFilters() as $filter) { - if (stripos($key, $filter) !== false) { - return true; - } + if (!$isMetaData) { + return false; + } + + foreach ($this->config->getFilters() as $filter) { + if (stripos($key, $filter) !== false) { + return true; + } + } + + foreach ($this->config->getRedactedKeys() as $redactedKey) { + if (@preg_match($redactedKey, $key) === 1) { + return true; + } elseif (Utils::stringCaseEquals($redactedKey, $key)) { + return true; } } diff --git a/src/Utils.php b/src/Utils.php index 5529061e..fbac1088 100644 --- a/src/Utils.php +++ b/src/Utils.php @@ -2,6 +2,8 @@ namespace Bugsnag; +use Normalizer; + class Utils { /** @@ -40,4 +42,50 @@ public static function getBuilderName() return $builderName; } + + /** + * Check if two strings are equal, ignoring case. + * + * @param string $a + * @param string $b + * + * @return bool + */ + public static function stringCaseEquals($a, $b) + { + // Avoid unicode normalisation and MB comparison if possible + if (strcasecmp($a, $b) === 0) { + return true; + } + + // Normalise code points into their decomposed form. For example "ñ" + // can be a single code point (U+00F1) or "n" (U+006E) with a combining + // tilde (U+0303). The decomposed form will always represent this as + // U+006E and U+0303, which means we'll match strings more accurately + // and makes case-insensitive comparisons easier + if (function_exists('normalizer_is_normalized') + && function_exists('normalizer_normalize') + ) { + $form = Normalizer::NFD; + + if (!normalizer_is_normalized($a, $form)) { + $a = normalizer_normalize($a, $form); + } + + if (!normalizer_is_normalized($b, $form)) { + $b = normalizer_normalize($b, $form); + } + } + + if (function_exists('mb_stripos') && function_exists('mb_strlen')) { + // There's no MB equivalent to strcasecmp, so we have to use + // mb_stripos with a length check instead + return mb_strlen($a) === mb_strlen($b) && mb_stripos($a, $b) === 0; + } + + // If the MB extension isn't available we can still use strcasecmp + // This will still work for multi-byte strings in some cases because + // the strings were normalised + return strcasecmp($a, $b) === 0; + } } diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 8205eaa9..441e59ad 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -1197,6 +1197,20 @@ public function testDiscardClassesCanBeSet() $this->assertSame($discardClasses, $this->client->getDiscardClasses()); } + public function testRedactedKeysDefault() + { + $this->assertSame([], $this->client->getRedactedKeys()); + } + + public function testRedactedKeysCanBeSet() + { + $redactedKeys = ['password', 'password_confirmation']; + + $this->client->setRedactedKeys($redactedKeys); + + $this->assertSame($redactedKeys, $this->client->getRedactedKeys()); + } + private function getGuzzleOption($guzzle, $name) { if (GuzzleCompat::isUsingGuzzle5()) { diff --git a/tests/ConfigurationTest.php b/tests/ConfigurationTest.php index 235ac860..7c04d260 100644 --- a/tests/ConfigurationTest.php +++ b/tests/ConfigurationTest.php @@ -344,4 +344,18 @@ public function testDiscardClassesCanBeSet() $this->assertSame($discardClasses, $this->config->getDiscardClasses()); } + + public function testRedactedKeysDefault() + { + $this->assertSame([], $this->config->getRedactedKeys()); + } + + public function testRedactedKeysCanBeSet() + { + $redactedKeys = ['password', 'password_confirmation']; + + $this->config->setRedactedKeys($redactedKeys); + + $this->assertSame($redactedKeys, $this->config->getRedactedKeys()); + } } diff --git a/tests/ReportTest.php b/tests/ReportTest.php index 60cf4b9c..50b7806e 100644 --- a/tests/ReportTest.php +++ b/tests/ReportTest.php @@ -3,6 +3,7 @@ namespace Bugsnag\Tests; use BadMethodCallException; +use Bugsnag\Breadcrumbs\Breadcrumb; use Bugsnag\Configuration; use Bugsnag\Report; use Bugsnag\Stacktrace; @@ -178,6 +179,160 @@ public function testFiltersAreCaseInsensitive() ); } + /** + * @dataProvider redactedKeysProvider + * + * @param array $metadata + * @param string[] $redactedKeys + * @param array $expected + * + * @return void + */ + public function testRedactedKeys( + array $metadata, + array $redactedKeys, + array $expected + ) { + $this->config->setRedactedKeys($redactedKeys); + $this->report->setMetaData(['Testing' => $metadata]); + + $actual = $this->report->toArray()['metaData']['Testing']; + + $this->assertEquals($expected, $actual); + } + + /** + * @dataProvider redactedKeysProvider + * + * @param array $metadata + * @param string[] $redactedKeys + * @param array $expected + * + * @return void + */ + public function testRedactedKeysWithBreadcrumbMetadata( + array $metadata, + array $redactedKeys, + array $expected + ) { + $this->config->setRedactedKeys($redactedKeys); + + $breadcrumb = new Breadcrumb('abc', Breadcrumb::LOG_TYPE, ['Testing' => $metadata]); + $this->report->addBreadcrumb($breadcrumb); + + $actual = $this->report->toArray()['breadcrumbs'][0]['metaData']['Testing']; + + $this->assertEquals($expected, $actual); + } + + public function redactedKeysProvider() + { + yield [ + ['abc' => 'xyz', 'a' => 1, 'b' => 2, 'c' => 3], + ['a', 'c'], + ['abc' => 'xyz', 'a' => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]'], + ]; + + yield [ + ['abc' => 'xyz', 'a' => 1, 'b' => 2, 'C' => 3], + ['A', 'c'], + ['abc' => 'xyz', 'a' => '[FILTERED]', 'b' => 2, 'C' => '[FILTERED]'], + ]; + + yield [ + ['â' => 1, 'b' => 2, 'ñ' => 3, 'n' => 4], + ['â', 'ñ'], + ['â' => '[FILTERED]', 'b' => 2, 'ñ' => '[FILTERED]', 'n' => 4], + ]; + + yield [ + ['â' => 1, 'b' => 2, 'Ñ' => 3], + ['Â', 'ñ'], + ['â' => '[FILTERED]', 'b' => 2, 'Ñ' => '[FILTERED]'], + ]; + + // 6e cc 83 is equivalent to "\u{006E}\u{0303}" but in a way PHP 5 can + // understand. This is the character "ñ" built out of "n" and a + // combining tilde + yield [ + ["\x6e\xcc\x83" => 1, 'b' => 2, 'c' => 3, 'n' => 4], + ["\x6e\xcc\x83", 'c'], + ["\x6e\xcc\x83" => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]', 'n' => 4], + ]; + + // 4e cc 83 is equivalent to "\u{004E}\u{0303}", which is the capital + // version of the above ("N" + a combining tilde) + yield [ + ["\x6e\xcc\x83" => 1, 'b' => 2, 'c' => 3, 'n' => 4], + ["\x4e\xcc\x83", 'c'], + ["\x6e\xcc\x83" => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]', 'n' => 4], + ]; + + // This is "ñ" both as a single character and with the combining tilde + yield [ + ["\x6e\xcc\x83" => 1, 'b' => 2, 'c' => 3, 'n' => 4], + ["\xc3\xb1", 'c'], + ["\x6e\xcc\x83" => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]', 'n' => 4], + ]; + + // This is "Ñ" as a single character and "ñ" with the combining tilde + yield [ + ["\x6e\xcc\x83" => 1, 'b' => 2, 'c' => 3, 'n' => 4], + ["\xc3\x91", 'c'], + ["\x6e\xcc\x83" => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]', 'n' => 4], + ]; + + // This is "Ñ" as a single character and "ñ" with the combining tilde + yield [ + ["\xc3\x91" => 1, 'b' => 2, 'c' => 3, 'n' => 4], + ["\x6e\xcc\x83", 'c'], + ["\xc3\x91" => '[FILTERED]', 'b' => 2, 'c' => '[FILTERED]', 'n' => 4], + ]; + + yield [ + ['abc' => 1, 'xyz' => 2], + ['/^.b.$/'], + ['abc' => '[FILTERED]', 'xyz' => 2], + ]; + + yield [ + ['abc' => 1, 'xyz' => 2, 'oOo' => 3], + ['/^[a-z]{3}$/'], + ['abc' => '[FILTERED]', 'xyz' => '[FILTERED]', 'oOo' => 3], + ]; + + yield [ + ['abc' => 1, 'xyz' => 2, 'oOo' => 3, 'oOoOo' => 4], + ['/^[A-z]{3}$/'], + ['abc' => '[FILTERED]', 'xyz' => '[FILTERED]', 'oOo' => '[FILTERED]', 'oOoOo' => 4], + ]; + + yield [ + ['abc' => 1, 'xyz' => 2, 'yyy' => 3], + ['/(c|y)$/'], + ['abc' => '[FILTERED]', 'xyz' => 2, 'yyy' => '[FILTERED]'], + ]; + + yield [ + ['abc' => 1, 'xyz' => 2, 'yyy' => 3], + ['/c$/', '/y$/'], + ['abc' => '[FILTERED]', 'xyz' => 2, 'yyy' => '[FILTERED]'], + ]; + + // This doesn't match the regex but does match as a string comparison + yield [ + ['/^abc$/' => 1, 'xyz' => 2, 'oOo' => 3], + ['/^abc$/'], + ['/^abc$/' => '[FILTERED]', 'xyz' => 2, 'oOo' => 3], + ]; + + yield [ + ['/abc/' => 1, 'xyz' => 2, 'oOo' => 3], + ['/abc/'], + ['/abc/' => '[FILTERED]', 'xyz' => 2, 'oOo' => 3], + ]; + } + public function testCanGetStacktrace() { $beginningOfTest = __LINE__; diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php new file mode 100644 index 00000000..162bba47 --- /dev/null +++ b/tests/UtilsTest.php @@ -0,0 +1,111 @@ +markTestSkipped("This test requires at least PHP {$requiredVersion} to run"); + } + } + + $this->assertSame( + $expected, + Utils::stringCaseEquals($a, $b), + sprintf( + 'Expected "%s" %s "%s"', + $a, + $expected ? 'to equal' : 'not to equal', + $b + ) + ); + + $this->assertSame( + $expected, + Utils::stringCaseEquals($b, $a), + sprintf( + 'Expected "%s" %s "%s"', + $b, + $expected ? 'to equal' : 'not to equal', + $a + ) + ); + } + + public function stringCaseEqualsProvider() + { + yield ['a', 'a', true]; + yield ['a', 'A', true]; + yield ['A', 'A', true]; + + yield ['a', 'b', false]; + yield ['c', 'b', false]; + + yield ['jalapeño', 'jalapeño', true]; + yield ['JALAPEÑO', 'jalapeño', true]; + yield ['JaLaPeÑo', 'jAlApEñO', true]; + yield ['jalapeño', 'jalapeno', false]; + + // 6e cc 83 is equivalent to "\u{006E}\u{0303}" but in a way PHP 5 can + // understand. This is the character "ñ" built out of "n" and a + // combining tilde + yield ["jalape\x6e\xcc\x83o", "jalape\x6e\xcc\x83o", true]; + yield ["jalape\x6e\xcc\x83o", 'jalapeño', true]; + + // 4e cc 83 is equivalent to "\u{004E}\u{0303}", which is the capital + // version of the above ("N" + a combining tilde) + yield ["jalape\x6e\xcc\x83o", "jalape\x4e\xcc\x83o", true]; + + // This is "ñ" both as a single character and with the combining tilde + yield ["jalape\x6e\xcc\x83o", "jalape\xc3\xb1o", true]; + + // This is "Ñ" as a single character and "ñ" with the combining tilde + yield ["jalape\x6e\xcc\x83o", "jalape\xc3\x91o", true]; + + yield ["jalape\x6e\xcc\x83o", 'jalapeno', false]; + + // This test fails with a simple strcasecmp, proving that the MB string + // functions are necessary in some cases + // This requires PHP 7.3, which contains many MB String improvements: + // https://www.php.net/manual/en/migration73.new-features.php#migration73.new-features.mbstring + yield ['größer', 'gröẞer', true, '7.3.0']; + yield ['größer', 'GRÖẞER', true, '7.3.0']; + + // Tests with characters from various unicode planes + + yield ['Iñtërnâtiônàližætiøn', 'Iñtërnâtiônàližætiøn', true]; + yield ['iñtërnâtiônàližætiøn', 'IÑTËRNÂTIÔNÀLIŽÆTIØN', true, '5.6.0']; + + yield ['обичам те', 'обичам те', true]; + yield ['обичам те', 'ОБИЧАМ ТЕ', true, '5.6.0']; + yield ['ОбИчАм Те', 'оБиЧаМ тЕ', true, '5.6.0']; + yield ['обичам те', 'oбичam te', false]; + + yield ['大好きだよ', '大好きだよ', true]; + yield ['أحبك', 'أحبك', true]; + + yield ['😀😀', '😀😀', true]; + + yield ['👨‍👩‍👧‍👦🇬🇧', '👨‍👩‍👧‍👦🇬🇧', true]; + yield ['🇬🇧👨‍👩‍👧‍👦', '👨‍👩‍👧‍👦🇬🇧', false]; + + $ukFlag = "\xf0\x9f\x87\xac\xf0\x9f\x87\xa7"; + yield ['👨‍👩‍👧‍👦'.$ukFlag, '👨‍👩‍👧‍👦🇬🇧', true]; + yield [$ukFlag.'👨‍👩‍👧‍👦', '👨‍👩‍👧‍👦🇬🇧', false]; + } +} diff --git a/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt b/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt index 46ac7442..dc551117 100644 --- a/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt +++ b/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt @@ -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); + +ini_set('memory_limit', 1024 * 1024 * 5); var_dump(ini_get('memory_limit')); $client->registerCallback(function () { var_dump(ini_get('memory_limit')); }); -Bugsnag\Handler::register($client); - $a = str_repeat('a', 2147483647); echo "No OOM!\n"; ?> --EXPECTF-- -string(2) "2M" +string(7) "5242880" Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line 14 -string(8) "12582912" +string(8) "15728640" Guzzle request made (1 event)! * Method: 'POST' * URI: 'http://localhost/notify'