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.

Sign up for GitHub

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

Jump to bottom

Obey safe repr for exceptions #91

Merged
merged 7 commits into from
Feb 22, 2016
Merged

Obey safe repr for exceptions #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fda5eb1
obey safe_repr settings for exceptions
sibson Feb 11, 2016
f132b7f
fix tests
sibson Feb 11, 2016
4945b93
scrub call args if name is in scrub fields
sibson Feb 11, 2016
7140a7b
fix flask test
sibson Feb 16, 2016
5456510
fix for py3k
sibson Feb 16, 2016
5d51141
Merge pull request #2 from sibson/scrub-args
sibson Feb 16, 2016
fe581dc
Merge branch 'master' into obey-safe-repr-for-exceptions
sibson Feb 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rollbar/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ def _report_exc_info(exc_info, request, extra_data, payload_data, level=None):
'frames': frames,
'exception': {
'class': cls.__name__,
'message': text(exc),
'message': _transform(exc),
}
}
}
Expand Down Expand Up @@ -864,7 +864,7 @@ def _add_locals_data(data, exc_info):
# Fill in all of the named args
for named_arg in named_args:
if named_arg in local_vars:
args.append(local_vars[named_arg])
args.append(_transform(local_vars[named_arg], key=(named_arg,)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this happen as part of the _serialize_frame_data() call below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, at that point you've lost the important context that the arg was password and only have the content left, which is insufficient for scrubbing. eg https://github.com/rollbar/pyrollbar/pull/91/files#diff-d7fa735556b7ff27de68295083f733d4R754.

I've been experimenting with an alternate solution that may be "cleaner" but has a larger impact on how rollbar is capturing and presenting data, https://github.com/rollbar/pyrollbar/pull/91/files#diff-d7fa735556b7ff27de68295083f733d4R754. I can PR against this repo if you'd like to have more discussion around it.

Copy link
Contributor

Choose a reason for hiding this comment

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

That'd be great. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/rollbar/pyrollbar/pull/95/files#diff-1fcbac3a897889dbaf8984fdfe14c99cR869


# Add any varargs
if arginfo.varargs is not None:
Expand Down
4 changes: 2 additions & 2 deletions rollbar/test/contrib/flask/test_flask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def test_uncaught(self, send_payload):

self.assertIn('body', data)
self.assertEqual(data['body']['trace']['exception']['class'], 'Exception')
self.assertStringEqual(data['body']['trace']['exception']['message'], 'Uh oh')
self.assertStringEqual(data['body']['trace']['exception']['message'], str(type(Exception('Uh oh'))))

self.assertIn('person', data)
self.assertDictEqual(data['person'],
Expand Down Expand Up @@ -115,7 +115,7 @@ def test_uncaught_json_request(self, send_payload):

self.assertIn('body', data)
self.assertEqual(data['body']['trace']['exception']['class'], 'Exception')
self.assertStringEqual(data['body']['trace']['exception']['message'], 'Uh oh')
self.assertStringEqual(data['body']['trace']['exception']['message'], str(type(Exception('Uh oh'))))

self.assertIn('person', data)
self.assertDictEqual(data['person'],
Expand Down
24 changes: 22 additions & 2 deletions rollbar/test/test_rollbar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def _raise():
self.assertIn('body', payload['data'])
self.assertIn('trace', payload['data']['body'])
self.assertIn('exception', payload['data']['body']['trace'])
self.assertEqual(payload['data']['body']['trace']['exception']['message'], 'foo')
self.assertEqual(payload['data']['body']['trace']['exception']['message'], str(type(Exception())))
self.assertEqual(payload['data']['body']['trace']['exception']['class'], 'Exception')

self.assertNotIn('args', payload['data']['body']['trace']['frames'][-1])
Expand Down Expand Up @@ -744,6 +744,27 @@ def test_modify_arg(self, send_payload):

self.assertEqual('changed', called_with_frame['args'][0])

@mock.patch('rollbar.send_payload')
def test_scrub_arg(self, send_payload):

def sensitive_call(username, password):
step1()

try:
sensitive_call('user', 'secret')
except:
rollbar.report_exc_info()

self.assertEqual(send_payload.called, True)

payload = json.loads(send_payload.call_args[0][0])
frames = payload['data']['body']['trace']['frames']
called_with_frame = frames[1]

self.assertEqual('user', called_with_frame['args'][0])
self.assertNotEqual('secret', called_with_frame['args'][1])
self.assertRegex(called_with_frame['args'][1], r'\*+')

@mock.patch('rollbar.send_payload')
def test_unicode_exc_info(self, send_payload):
message = '\u221a'
Expand All @@ -757,7 +778,6 @@ def test_unicode_exc_info(self, send_payload):
payload = json.loads(send_payload.call_args[0][0])
self.assertEqual(payload['data']['body']['trace']['exception']['message'], message)


@mock.patch('requests.post', side_effect=lambda *args, **kw: MockResponse({'status': 'OK'}, 200))
def test_serialize_and_send_payload(self, post=None):
invalid_b64 = b'CuX2JKuXuLVtJ6l1s7DeeQ=='
Expand Down