Skip to content

Conversation

DaScheid
Copy link
Contributor

Removing database-properties from exception message, as those might contain database-credentials.
See related issue #452

@DaScheid DaScheid requested a review from a team as a code owner January 29, 2024 10:08
@liweinan liweinan linked an issue Jan 29, 2024 that may be closed by this pull request
@liweinan
Copy link
Contributor

@DaScheid instead of deleting the message item, wdyt if change the the usage of the message to:

throw BatchMessages.MESSAGES.failToObtainConnection(e, dbUrl, "<db props> masked");

@DaScheid
Copy link
Contributor Author

DaScheid commented Jan 29, 2024

Yes, using a static string of "<db props> masked" would also be an option.
Or did you mean to dynamically exclude only password-/username-properties from the error message, but leave other properties in?

@liweinan
Copy link
Contributor

liweinan commented Jan 29, 2024

@DaScheid It's safer to use a static string IMHO. If you agree could you please update your PR according to our discussion? (and please squash the commits)

@DaScheid DaScheid force-pushed the fix/JBERET-452_logging_db_creds branch from 4026b3b to c2598f9 Compare January 29, 2024 15:28
@liweinan liweinan merged commit eeef999 into jberet:main Jan 29, 2024
@liweinan
Copy link
Contributor

@DaScheid Thanks for the contribution!

@DaScheid
Copy link
Contributor Author

Thank you for the fast review @liweinan

@christf christf mentioned this pull request Jan 29, 2024
@DaScheid DaScheid deleted the fix/JBERET-452_logging_db_creds branch February 16, 2024 15:15
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.

jberet-core logging database credentials
2 participants