-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add checks to allow logging only specific datatypes #46
Conversation
You linked your team's caretaker journal. Could you also link the incident ticket (IN-299) or create an internal ticket for this problem for tracking fixes as well? |
linked the incident ticket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look fine but please add the tests
85a57be
to
c7bfb64
Compare
attr_accessor :asd | ||
|
||
def initialize | ||
@asd = 'tdlgdfaha' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random fields, I know, but could name them something nicer 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. i can do that 😄
def process(pointers, data) | ||
described_class.new(pointers: pointers, action: :prune).process(data) | ||
end | ||
end | ||
end | ||
|
||
class Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bad name to have globally defined. You don't need a global class, you can do:
let(:data) do
Class.new do
attr_accessor :some_attribute
#...
end.new
end
I personally would use an open struct instead:
let(:data) { OpenStruct.new(:name => "Rowdy", :owner => "John Smith") }
@@ -18,8 +18,11 @@ def process(data, pointer = '') | |||
when Array | |||
process_array(data, pointer) | |||
|
|||
else | |||
when String, Numeric, Symbol, Date, Time, TrueClass, FalseClass, NilClass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date
is not loaded by default.
~ irb
irb(main):001:0> Date
Traceback (most recent call last):
4: from /home/indrek/.asdf/installs/ruby/2.7.8/bin/irb:23:in `<main>'
3: from /home/indrek/.asdf/installs/ruby/2.7.8/bin/irb:23:in `load'
2: from /home/indrek/.asdf/installs/ruby/2.7.8/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>'
1: from (irb):1
NameError (uninitialized constant Date)
And if it's loaded, then there's also DateTime
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests for this as well. DateTime is subclass of Date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no further comments
f485be2
to
4519cc4
Compare
Currently, it allows logging of objects and the whitelisting does not apply to the fields of that object. This change adds data type checks and allows logging of only specific types so that data that is not whitelisted does not go through. This means it will be either masked or removed. MSG-709, IN-299
Also adding tests
Currently, it allows logging of objects and the whitelisting does not apply to the fields of that object.
This change adds data type checks and allows logging of only specific types so that data that is not whitelisted or that is blacklisted does not go through. This means it will be either masked or pruned.
MSG-709