[GitHub] [shiro] bmarwell commented on a change in pull request #288: Bugfix, fixed Scanner issue

[GitBox <gi...@apache.org>]

bmarwell commented on a change in pull request #288:
URL: https://github.com/apache/shiro/pull/288#discussion_r599715248



##########
File path: core/src/main/java/org/apache/shiro/realm/text/TextConfigurationRealm.java
##########
@@ -211,11 +212,14 @@ 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());
-        }
-        return set;
+		try (Scanner scanner = new Scanner(s)) {
+			while (scanner.hasNextLine()) {
+				set.add(scanner.nextLine());
+			}
+		} catch (NoSuchElementException e) {
+			e.printStackTrace();
+		} 
+		return set;

Review comment:
       > how was this the exception triggered? 
   
   > Link to Apache's SonarCloud: the bug.
   
   yields: `Use try-with-resources or close this "Scanner" in a "finally" clause.`
   
   That is, you would not want to catch the exception. Just the try would be fine in this case, as this is only about closing the scanner. But as we are reading a String, this is a noop anyway:
   
   https://github.com/openjdk/jdk/blob/d7268fa3a68c2356548651a27b307d7f7158e700/src/java.base/share/classes/java/util/Scanner.java#L1171-L1175
   
   So in this case, while Sonar is right in the sense of "good habit" (as the source could change): My -1 on this as we are not "fixing" anything or making anything better. Closing the scanner is fine for me, but there was no underlying bug and the API would also never be able to change to something `Closeable`.
   
   
   




-- 
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:
users@infra.apache.org