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

rack: return generic error instead of leaking exception msg #99

Closed
wants to merge 1 commit into from
Closed

rack: return generic error instead of leaking exception msg #99

wants to merge 1 commit into from

Conversation

davidor
Copy link

@davidor davidor commented Feb 7, 2020

Fixes #98

@davidor davidor requested a review from ioquatix February 7, 2020 12:00
@ioquatix
Copy link
Member

ioquatix commented Feb 7, 2020

Why not use a middleware to do this, if that's your desire?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.293% when pulling 45ab047 on davidor:generic-error-instead-of-exception into b8e9ca5 on socketry:master.

@davidor
Copy link
Author

davidor commented Feb 7, 2020

Thanks for the quick response @ioquatix

I can use a middleware to do this, but I thought that it might make more sense to change this to avoid leaking sensitive information.

@ioquatix ioquatix force-pushed the master branch 4 times, most recently from 77ce8ce to 98a0ae6 Compare April 8, 2020 15:12
@gottlike
Copy link

gottlike commented Apr 9, 2020

This kind of relates to: #112

Would love to see this PR merged :)

@ianks
Copy link

ianks commented Aug 22, 2020

This is important, certain errors can contain sensitive or irrelevant info, and should not be shown to users. Using a middleware for solving this is a lot to ask for, since this is going against the grain of almost all app servers...

@ioquatix
Copy link
Member

@davidor can you rebase on master so I can merge.

@davidor
Copy link
Author

davidor commented Aug 24, 2020

@ioquatix sorry, I forgot about this PR and deleted my fork some time ago. Now I can't edit this PR, so I had to open a new one. It's the same commit but rebased on master: #130

@davidor davidor closed this Aug 24, 2020
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.

Option to avoid leaking exception messages in the response body
5 participants