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

Improve param scrubbing #57

Merged
merged 4 commits into from
Jun 4, 2015
Merged

Improve param scrubbing #57

merged 4 commits into from
Jun 4, 2015

Conversation

chanind
Copy link
Contributor

@chanind chanind commented Jun 4, 2015

This PR adds several improvements to param scrubbing:

  • regexes can be used in the scrub_fields list. ex: $config['scrub_fields'] = array('/password/i');
  • If a param has an array as a value, the array will be recursively scrubbed
  • $_GET and headers are also scrubbed before being recorded

@brianr
Copy link
Member

brianr commented Jun 4, 2015

Wow, thanks! We'll review and get this merged shortly.

$scrubbed[$k] = str_repeat('*', $count);
if ($this->_key_should_be_scrubbed($k, $potential_regex_filters)) {
$scrubbed[$k] = $this->_scrub($v);
} elseif (is_array($v)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading this correctly, the behavior here is:

  • If the key matches a scrub pattern, then scrub it, no matter what its value is (i.e. even if the value is an array)
  • Otherwise, if it's an array, recursively scrub params in that array

I think that sounds like the right thing to do, but I just thought I'd shine some light. Perhaps there should be a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's intended. test has been amended here: cdcb2f1

brianr added a commit that referenced this pull request Jun 4, 2015
@brianr brianr merged commit cf8a1db into rollbar:master Jun 4, 2015
@brianr
Copy link
Member

brianr commented Jun 4, 2015

This is up now on packagist as version 0.12.0. Thanks again!

@jeffchan jeffchan deleted the enhanced-scrubbing branch June 4, 2015 05:25
@chanind
Copy link
Contributor Author

chanind commented Jun 4, 2015

awesome thanks for the fast turnaround on this!

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.

None yet

2 participants