Fix scrubber #112
Fix scrubber #112
Conversation
Uses manual recursion to enable scrubbing of entire array fields like "Cookie".
MichaelMackus
force-pushed
the
fix-scrubber
branch
from
3eadc61
to
9300095
Compare
|
Just to be clear, from what I could tell from initial usage the "scrubFields" param was incredibly broken. It seems like it just ended up scrubbing all the fields, no matter the "scrubFields" param. I've gone through the scrubber function and simply fixed some parameters & other things to ensure the "scrubFields" param is recognized. I've also updated it to manually recurse the scrubber function (by calling "array_walk" inside the function, only if the value is an array). I believe I did this so that data structures other than arrays get through the scrubber without erroring out. |
This wasn't quite working very well before, and was masking values that shouldn't have been masked. Turning on strict mode fixes this and should be fairly backward compatible.
|
Here's a link I found explaining some reasoning behind the previous commit. |
|
@rokob any input? This definitely fixes a lot of issues with the previous scrubber functionality. |
| }; | ||
| array_walk_recursive($arr, $scrubber); |
There was a problem hiding this comment.
This is confusing to me. I don't understand the reasoning behind doing a manual recursion versus array_walk_recursive()
There was a problem hiding this comment.
Oh, I just saw @MichaelMackus comment above. I think that making the code to error out when passing other types than an array is a good idea since they are not going to get scrubbed. My reasoning is that if I pass some data to be scrubbed (e.g. an object) and the library doesn't support this type, it's a pretty critical issue. We don't want the users to be passing objects and thinking they got scrubbed when they didn't.
There was a problem hiding this comment.
Argument I'm making here is that if a data type not supported by the scrubbing method is passed we should at least throw an exception and make it very obvious.
|
Improvements @MichaelMackus did are looking great. I'm going to take his work, use it as a base for my branch and expand on them. |
Branch split of #111, specifically to fix the DataBuilder::scrub method.