Skip to content
This repository was archived by the owner on Apr 20, 2024. It is now read-only.

Filter out sensitive data from the HTTP headers #75

Merged
merged 2 commits into from
Oct 9, 2020

Conversation

raphaelcruzeiro
Copy link
Contributor

The HTTP headers were not being stripped of sensitive data as the request data was. This PR fixes that.

… out sensitive data from the request headers. Fixes ml-archive#74
@raphaelcruzeiro
Copy link
Contributor Author

Not really sure if this project accepts external contributions but since this bugsnag integration plays such an important part on Tangerine, I decided to share here this very small fix I made so that we wouldn't leak our user's authentication tokens on our logs ;)

@siemensikkema
Copy link
Contributor

@raphaelcruzeiro Thanks for you PR! We are always open for contributions 🙂 Your PR fixes an important bug so thanks for that! I'd like to avoid the force unwrap you're using, though. Can you find a way to avoid that? Also consider https://developer.apple.com/documentation/swift/array/3126956-reduce for a possibly more compact way to create the header dict. If you don't get to it, I'll try to see to it when I get some time but that might be a while.

…r dictonary building to use reduce(into:) so that the overall code is more idionatic Swift
@raphaelcruzeiro
Copy link
Contributor Author

@siemensikkema No worries! I've changed the code to avoid that force cast and replaced the reduce with reduce(into:) (thanks for this tip btw. I didn't know this variant of reduce and it's a way nicer way to deal with reducing stuff into a dictionary).

Copy link
Contributor

@siemensikkema siemensikkema left a comment

Choose a reason for hiding this comment

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

Looks really good! Thanks for the quick finishing touches

@raphaelcruzeiro
Copy link
Contributor Author

Glad to help ;)

@siemensikkema siemensikkema merged commit 4f02ae4 into ml-archive:master Oct 9, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants