[GitHub] [maven-surefire] andpab commented on a diff in pull request #387: [SUREFIRE-1939] Fix NPE in SystemUtils.toJdkHomeFromJvmExec if java home has 2 components

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

andpab commented on code in PR #387:
URL: https://github.com/apache/maven-surefire/pull/387#discussion_r1059800902


##########
surefire-booter/src/test/java/org/apache/maven/surefire/booter/SystemUtilsTest.java:
##########
@@ -98,6 +98,15 @@ public void incorrectJdkPath()
             assertThat( SystemUtils.isJava9AtLeast( incorrect.getAbsolutePath() ) ).isFalse();
         }
 
+        @Test
+        public void incorrectJdkPathShouldNotNPE()

Review Comment:
   I've backtracked the production scope callers of the SystemUtils method and the parameter can come from three possible places:
   
   1. `<jvm>` mojo property
   2. `System.getProperty( "java.home" ) + File.separator + "bin" + File.separator + "java"`
   3. JavaToolChain installFolder `+ File.separator + "bin" + File.separator + "java"`
   
   The only one that could in theory lead to the NPE is the `<jvm>` property - but only if a user enters `/` or `C:\` there. Not an actual use case.
   
   Conclusion: This is not an actual problem in production.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org