From b9e54ed59fd4c6620a24ff03d040a0c33b508439 Mon Sep 17 00:00:00 2001 From: Andrew Ledvina Date: Mon, 1 May 2017 15:03:37 -0700 Subject: [PATCH 1/3] Fix query string guessing in scrubber [Fixes #147] We are doing a bit of guess work when given a string to scrub to decide whether we should treat it as a bare query string or just as a string. This adds some additional tests and changes the way we guess if we have a query string based on the values of the parsed string being not falsey. --- src/DataBuilder.php | 10 +++------- tests/DataBuilderTest.php | 26 ++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/DataBuilder.php b/src/DataBuilder.php index b223ca75..f996a47b 100644 --- a/src/DataBuilder.php +++ b/src/DataBuilder.php @@ -775,14 +775,10 @@ public function scrub(&$data, $replacement = '*') $parsedValues = array_values($parsed); /** - * If parsing a string results in an associative array - * with multiple elements it's valid query string (key - * recognition). - * - * Also, if it results in first key having an assigned value - * it's also a valid query string (values recognition). + * If we have at least one key/value pair (i.e. a=b) then + * we treat the whole string as a query string. */ - if (count($parsed) > 1 || $parsedValues[0]) { + if (count(array_filter($parsedValues)) > 0) { $query = $data; } } diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index cf17137a..a2584d8c 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -221,6 +221,8 @@ public function scrubDataProvider() return array_merge(array( 'flat data array' => $this->scrubFlatDataProvider(), + 'plain numbers in an array' => + $this->scrubJSONNumbersProvider(), 'recursive data array' => $this->scrubRecursiveDataProvider(), 'string encoded values' => @@ -231,7 +233,18 @@ public function scrubDataProvider() $this->scrubRecursiveStringRecursiveDataProvider() ), $this->scrubUrlDataProvider()); } - + + private function scrubJSONNumbersProvider() + { + return array( + '[1023,1924]', + array( + 'sensitive' + ), + '[1023,1924]' + ); + } + private function scrubFlatDataProvider() { return array( @@ -255,9 +268,13 @@ private function scrubRecursiveDataProvider() array( // $testData 'non sensitive data 1' => '123', 'non sensitive data 2' => '456', + 'non sensitive data 3' => '4&56', + 'non sensitive data 4' => 'a=4&56', + 'non sensitive data 6' => 'baz&foo=bar', 'sensitive data' => '456', array( 'non sensitive data 3' => '789', + 'non sensitive data 5' => '789&5=', 'recursive sensitive data' => 'qwe', 'non sensitive data 3' => 'rty', array( @@ -267,14 +284,19 @@ private function scrubRecursiveDataProvider() ), array( // $scrubFields 'sensitive data', - 'recursive sensitive data' + 'recursive sensitive data', + 'foo' ), array( // $expected 'non sensitive data 1' => '123', 'non sensitive data 2' => '456', + 'non sensitive data 3' => '4&56', + 'non sensitive data 4' => 'a=4&56=', // this is a weird edge case + 'non sensitive data 6' => 'baz=&foo=xxxxxxxx', 'sensitive data' => '********', array( 'non sensitive data 3' => '789', + 'non sensitive data 5' => '789&5=', 'recursive sensitive data' => '********', 'non sensitive data 3' => 'rty', array( From 523c0febb253cc83dec193d6d97ed53396f1e232 Mon Sep 17 00:00:00 2001 From: Andrew Ledvina Date: Mon, 1 May 2017 15:12:20 -0700 Subject: [PATCH 2/3] update version number everywhere needed, and bump to 1.0.1 --- CHANGELOG.md | 4 ++++ README.md | 2 +- src/Payload/Notifier.php | 2 +- tests/NotifierTest.php | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb3266e..4454e7e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 1.0.1 +- Bug fix related to scrubbing potential query strings +- Update notifier to send the correct version number in the payload + ## 1.0.0 Almost everything has been refactored or rewritten. The updated README has all of the current diff --git a/README.md b/README.md index 696fd4c1..b6d49473 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ Add `rollbar/rollbar` to your `composer.json`: ```json { "require": { - "rollbar/rollbar": "~1.0.0" + "rollbar/rollbar": "~1.0.1" } } ``` diff --git a/src/Payload/Notifier.php b/src/Payload/Notifier.php index 548913ee..f4b89de7 100644 --- a/src/Payload/Notifier.php +++ b/src/Payload/Notifier.php @@ -5,7 +5,7 @@ class Notifier implements \JsonSerializable { const NAME = "rollbar-php"; - const VERSION = "1.0.0-beta"; + const VERSION = "1.0.1"; public static function defaultNotifier() { diff --git a/tests/NotifierTest.php b/tests/NotifierTest.php index 779712d1..83bc2171 100644 --- a/tests/NotifierTest.php +++ b/tests/NotifierTest.php @@ -29,6 +29,6 @@ public function testEncode() { $notifier = Notifier::defaultNotifier(); $encoded = json_encode($notifier->jsonSerialize()); - $this->assertEquals('{"name":"rollbar-php","version":"1.0.0-beta"}', $encoded); + $this->assertEquals('{"name":"rollbar-php","version":"1.0.1"}', $encoded); } } From cb6af16526bc90cb9e45d41a01785be042ced4ef Mon Sep 17 00:00:00 2001 From: Andrew Ledvina Date: Mon, 1 May 2017 15:19:25 -0700 Subject: [PATCH 3/3] add another test for #147 --- tests/DataBuilderTest.php | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/DataBuilderTest.php b/tests/DataBuilderTest.php index a2584d8c..2242437e 100644 --- a/tests/DataBuilderTest.php +++ b/tests/DataBuilderTest.php @@ -221,8 +221,6 @@ public function scrubDataProvider() return array_merge(array( 'flat data array' => $this->scrubFlatDataProvider(), - 'plain numbers in an array' => - $this->scrubJSONNumbersProvider(), 'recursive data array' => $this->scrubRecursiveDataProvider(), 'string encoded values' => @@ -231,17 +229,26 @@ public function scrubDataProvider() $this->scrubRecursiveStringDataProvider(), 'string encoded recursive values in recursive array' => $this->scrubRecursiveStringRecursiveDataProvider() - ), $this->scrubUrlDataProvider()); + ), $this->scrubUrlDataProvider(), $this->scrubJSONNumbersProvider()); } private function scrubJSONNumbersProvider() { return array( - '[1023,1924]', - array( - 'sensitive' + 'plain array' => array( + '[1023,1924]', + array( + 'sensitive' + ), + '[1023,1924]' ), - '[1023,1924]' + 'param equals array' => array( + 'b=[1023,1924]', + array( + 'sensitive' + ), + 'b=%5B1023%2C1924%5D' + ) ); }