bdemers commented on a change in pull request #288:
URL: https://github.com/apache/shiro/pull/288#discussion_r599732138
##########
File path:
core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java
##########
@@ -211,9 +215,15 @@ protected void processUserDefinitions(Map<String, String>
userDefs) {
protected static Set<String> toLines(String s) {
LinkedHashSet<String> set = new LinkedHashSet<String>();
- Scanner scanner = new Scanner(s);
- while (scanner.hasNextLine()) {
- set.add(scanner.nextLine());
+ try (Scanner scanner = new Scanner(s)) {
+ while (scanner.hasNextLine()) {
+ set.add(scanner.nextLine());
+ }
+ } catch (Exception e) {
+ if (log.isWarnEnabled()) {
+ String msg = "Unable to fetch next line, Scanner stream
corrupted.";
+ log.warn(msg, e);
+ }
Review comment:
Agree, we want the original exception thrown here, failure to parse the
config (especially config related to security) is a valid reason to throw an
exception and resulting in a failure of the application to start.
A couple of other nits, as you learn more about Java:
you don't need to check `log.isWarnEnabled()` unless you are concatenating
strings, or doing another expensive operation in that block, the log back
`warn` method will do this check for you.
But like we mentioned, we don't want to swallow this exception
@pitjazz let's close this PR for now, if there is a listing issue you want
to fix, we can continue the thread here and provide other suggestions if needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
