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

Filter deep parameters, Rails 5-style. #256

Merged
merged 1 commit into from
Dec 23, 2015

Conversation

fimmtiu
Copy link
Contributor

@fimmtiu fimmtiu commented Oct 28, 2015

Rails 5 has added the ability to filter parameters out of a deeply-nested structure by specifying filters with periods in them, such that (for example) /users\.id/ will filter {users: {id: 1}} to {users: {id: '[FILTERED]'}}. I've broken Bugsnag's parameter-filtering code out into a separate class and added similar functionality to make it work with Rails 5-style filters.

For context, see Rails 5's ActionDispatch::Http::ParameterFilter::CompiledFilter:
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/http/parameter_filter.rb

Note that I haven't added such functionality to the GET parameter filtering, for two reasons: (1) it would be rather nontrivial to do in full generality, and (2) it only seems to be used in the RackRequest and Rails2Request classes, and neither Rack nor Rails 2 do deep parameter filters.

@fimmtiu
Copy link
Contributor Author

fimmtiu commented Dec 11, 2015

Ping! It's been about six weeks since I created this pull request; is anyone interested in it?

@fimmtiu fimmtiu force-pushed the rails5-deep-parameter-filtering branch from 18f8a1f to d6894b7 Compare December 11, 2015 20:37
@kattrali
Copy link
Contributor

Thank you for the contribution, @fimmtiu. I apologize for the delay. The changes look good. I've had to give some thought to which release to this in, I think it should be grouped with a few other adjustments for rails 5, which should be released together soon.

kattrali added a commit that referenced this pull request Dec 23, 2015
@kattrali kattrali merged commit d6894b7 into bugsnag:master Dec 23, 2015
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.

2 participants