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

Made dumpQueriesOnException = false by default as per documentation. #131

Closed
wants to merge 1 commit into from

Conversation

King-hT
Copy link

@King-hT King-hT commented Jan 11, 2019

…Added a unit test case for the same.

@King-hT
Copy link
Author

King-hT commented Jan 11, 2019

This PR is raised based on 2 things.

  1. Making the code in sync with documentation.
    image
  2. In our project, we value user data a lot. And there was a security bug reported by the team as the query had the critical user information logged in the stack trace.
    Now, this could be a common issue with others where they do not want the queries to be printed on the exception stack trace.
    For now, we have solved this by adding 'dumpQueriesOnException=false' to the host URL.

@King-hT King-hT changed the title Made dumpQueriesOnException = false by default as per documentation. … Made dumpQueriesOnException = false by default as per documentation. Jan 11, 2019
@vaintroub
Copy link
Contributor

If you value security a lot, maybe you should not show the raw SQLException text?

@King-hT
Copy link
Author

King-hT commented Jan 15, 2019

@vaintroub Thanks for the comment. I was wondering if I was missing any step as no one was even looking at my PR.

Well, you can say it as a bad design that we have in place or bad structure. But we have many classes which catch the exception and pass it ahead as our Business Exception which logs everything.
I get what you are saying, but in our case, it would be very difficult to catch SQLException everywhere and eat the exception trace.

That's where the dumpQueriesOnException comes into the picture. We have currently solved the issue by adding 'dumpQueriesOnException=false' to the host URL.

@rusher
Copy link
Collaborator

rusher commented Jan 22, 2019

Thanks for contribution. this has been merged on develop branch, with additional commit c58c2ab for tests correction

@rusher rusher closed this Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants