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

Guessing query strings should err on being conservative #164

Merged
merged 2 commits into from
May 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/DataBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -760,29 +760,9 @@ public function scrub(&$data, $replacement = '*')
}

if (is_array($data)) { // scrub arrays

$data = $this->scrubArray($data, $replacement);
} elseif (is_string($data)) { // scrub URLs and query strings

$query = parse_url($data, PHP_URL_QUERY);

/**
* String is not a URL but it still might be just a plain
* query string in format arg1=val1&arg2=val2
*/
if (!$query) {
parse_str($data, $parsed);
$parsedValues = array_values($parsed);

/**
* 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(array_filter($parsedValues)) > 0) {
$query = $data;
}
}

if ($query) {
$data = str_replace(
$query,
Expand All @@ -791,7 +771,6 @@ public function scrub(&$data, $replacement = '*')
);
}
}

return $data;
}

Expand Down
20 changes: 10 additions & 10 deletions tests/DataBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ private function scrubJSONNumbersProvider()
array(
'sensitive'
),
'b=%5B1023%2C1924%5D'
'b=[1023,1924]'
)
);
}
Expand Down Expand Up @@ -277,7 +277,7 @@ private function scrubRecursiveDataProvider()
'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',
'non sensitive data 6' => '?baz&foo=bar',
'sensitive data' => '456',
array(
'non sensitive data 3' => '789',
Expand All @@ -298,8 +298,8 @@ private function scrubRecursiveDataProvider()
'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',
'non sensitive data 4' => 'a=4&56',
'non sensitive data 6' => '?baz=&foo=xxxxxxxx',
'sensitive data' => '********',
array(
'non sensitive data 3' => '789',
Expand All @@ -318,7 +318,7 @@ private function scrubFlatStringDataProvider()
{
return array(
// $testData
http_build_query(
'?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'scrubit',
Expand All @@ -329,7 +329,7 @@ private function scrubFlatStringDataProvider()
'sensitive'
),
// $expected
http_build_query(
'?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'xxxxxxxx',
Expand All @@ -343,7 +343,7 @@ private function scrubRecursiveStringDataProvider()
{
return array(
// $testData
http_build_query(
'?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'scrubit',
Expand All @@ -357,7 +357,7 @@ private function scrubRecursiveStringDataProvider()
'sensitive'
),
// $expected
http_build_query(
'?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'xxxxxxxx',
Expand All @@ -380,7 +380,7 @@ private function scrubRecursiveStringRecursiveDataProvider()
array(
'non sensitive data 3' => '789',
'recursive sensitive data' => 'qwe',
'non sensitive data 3' => http_build_query(
'non sensitive data 3' => '?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'scrubit',
Expand All @@ -407,7 +407,7 @@ private function scrubRecursiveStringRecursiveDataProvider()
array(
'non sensitive data 3' => '789',
'recursive sensitive data' => '********',
'non sensitive data 3' => http_build_query(
'non sensitive data 3' => '?' . http_build_query(
array(
'arg1' => 'val 1',
'sensitive' => 'xxxxxxxx',
Expand Down